From 5bc7680f5aeba3587df188b26dd893c219fef974 Mon Sep 17 00:00:00 2001
From: Simon Willison <swillison@gmail.com>
Date: Tue, 23 Apr 2024 18:32:23 -0700
Subject: [PATCH] Switch to tilde-encoded table names, refs #62

---
 datasette_edit_schema/__init__.py             |  7 ++--
 .../templates/edit_schema_table.html          | 16 ++++----
 tests/conftest.py                             | 15 ++++++++
 tests/test_edit_schema.py                     | 37 +++++++++++++++----
 4 files changed, 57 insertions(+), 18 deletions(-)

diff --git a/datasette_edit_schema/__init__.py b/datasette_edit_schema/__init__.py
index d92fb8d..b71726e 100644
--- a/datasette_edit_schema/__init__.py
+++ b/datasette_edit_schema/__init__.py
@@ -1,7 +1,7 @@
 from datasette import hookimpl
 from datasette.events import CreateTableEvent, AlterTableEvent, DropTableEvent
 from datasette.utils.asgi import Response, NotFound, Forbidden
-from datasette.utils import sqlite3
+from datasette.utils import sqlite3, tilde_decode, tilde_encode
 from urllib.parse import quote_plus, unquote_plus
 import sqlite_utils
 import textwrap
@@ -40,7 +40,7 @@ async def inner():
         return [
             {
                 "href": datasette.urls.path(
-                    "/-/edit-schema/{}/{}".format(database, quote_plus(table))
+                    "/-/edit-schema/{}/{}".format(database, tilde_encode(table))
                 ),
                 "label": "Edit table schema",
                 "description": "Rename the table, add and remove columns...",
@@ -307,7 +307,7 @@ def create_the_table(conn):
 
 
 async def edit_schema_table(request, datasette):
-    table = unquote_plus(request.url_vars["table"])
+    table = tilde_decode(request.url_vars["table"])
     databases = get_databases(datasette)
     database_name = request.url_vars["database"]
 
@@ -619,6 +619,7 @@ def get_columns_and_schema_and_fks_and_pks_and_indexes(conn):
                 "can_rename_table": await can_rename_table(
                     datasette, request.actor, database_name, table
                 ),
+                "tilde_encode": tilde_encode,
             },
             request=request,
         )
diff --git a/datasette_edit_schema/templates/edit_schema_table.html b/datasette_edit_schema/templates/edit_schema_table.html
index 5d2b704..0dda850 100644
--- a/datasette_edit_schema/templates/edit_schema_table.html
+++ b/datasette_edit_schema/templates/edit_schema_table.html
@@ -86,12 +86,12 @@
 {% endblock %}
 
 {% block content %}
-<h1>Edit table <a href="{{ base_url }}{{ database.name|quote_plus }}/{{ table|quote_plus }}">{{ database.name }}/{{ table }}</a></h1>
+<h1>Edit table <a href="{{ base_url }}{{ database.name|quote_plus }}/{{ tilde_encode(table) }}">{{ database.name }}/{{ table }}</a></h1>
 
 {% if can_rename_table %}
 <h2>Rename table</h2>
 
-<form action="{{ base_url }}-/edit-schema/{{ database.name|quote_plus }}/{{ table|quote_plus }}" method="post">
+<form action="{{ base_url }}-/edit-schema/{{ database.name|quote_plus }}/{{ tilde_encode(table) }}" method="post">
     <input type="hidden" name="csrftoken" value="{{ csrftoken() }}">
     <p><label>New name&nbsp; <input type="text" name="name"></label>
     <input type="hidden" name="rename_table" value="1">
@@ -99,7 +99,7 @@ <h2>Rename table</h2>
 </form>
 {% endif %}
 
-<form action="{{ base_url }}-/edit-schema/{{ database.name|quote_plus }}/{{ table|quote_plus }}" method="post">
+<form action="{{ base_url }}-/edit-schema/{{ database.name|quote_plus }}/{{ tilde_encode(table) }}" method="post">
 <h2>Change existing columns</h2>
 <ul class="sortable-columns">
 {% for column in columns %}
@@ -134,7 +134,7 @@ <h2>Change existing columns</h2>
 
 <h2>Add a column</h2>
 
-<form action="{{ base_url }}-/edit-schema/{{ database.name|quote_plus }}/{{ table|quote_plus }}" method="post">
+<form action="{{ base_url }}-/edit-schema/{{ database.name|quote_plus }}/{{ tilde_encode(table) }}" method="post">
     <input type="hidden" name="csrftoken" value="{{ csrftoken() }}">
     <input type="hidden" name="add_column" value="1">
     <p><label>Name &nbsp;<input type="text" name="name"></label>
@@ -156,7 +156,7 @@ <h2>Update foreign key relationships</h2>
 }
 </style>
 
-<form action="{{ base_url }}-/edit-schema/{{ database.name|quote_plus }}/{{ table|quote_plus }}" method="post">
+<form action="{{ base_url }}-/edit-schema/{{ database.name|quote_plus }}/{{ tilde_encode(table) }}" method="post">
     <input type="hidden" name="csrftoken" value="{{ csrftoken() }}">
     <input type="hidden" name="action" value="update_foreign_keys">
     <table class="foreign-key-options">
@@ -180,7 +180,7 @@ <h2>{% if is_rowid_table %}Set a primary key{% else %}Change the primary key{% e
 
     <p>The primary key column uniquely identifies each row in the table.</p>
 
-    <form action="{{ base_url }}-/edit-schema/{{ database.name|quote_plus }}/{{ table|quote_plus }}" method="post">
+    <form action="{{ base_url }}-/edit-schema/{{ database.name|quote_plus }}/{{ tilde_encode(table) }}" method="post">
         <input type="hidden" name="csrftoken" value="{{ csrftoken() }}">
         <input type="hidden" name="action" value="update_primary_key">
         <label for="primary_key">Primary key column &nbsp;</label>
@@ -201,7 +201,7 @@ <h2>Table indexes</h2>
 
     <p>Indexes can speed up filter and sort operations against indexed columns.</p>
 
-    <form action="{{ base_url }}-/edit-schema/{{ database.name|quote_plus }}/{{ table|quote_plus }}" method="post">
+    <form action="{{ base_url }}-/edit-schema/{{ database.name|quote_plus }}/{{ tilde_encode(table) }}" method="post">
         <input type="hidden" name="csrftoken" value="{{ csrftoken() }}">
         {% if non_primary_key_columns %}
             <p><label for="id_add_index_column">
@@ -233,7 +233,7 @@ <h3>Existing indexes</h3>
 {% if can_drop_table %}
     <h2>Drop table</h2>
 
-    <form id="drop-table-form" action="{{ base_url }}-/edit-schema/{{ database.name|quote_plus }}/{{ table|quote_plus }}" method="post">
+    <form id="drop-table-form" action="{{ base_url }}-/edit-schema/{{ database.name|quote_plus }}/{{ tilde_encode(table) }}" method="post">
         <input type="hidden" name="csrftoken" value="{{ csrftoken() }}">
         <input type="hidden" name="drop_table" value="1">
         <input type="submit" class="button-red" value="Drop this table">
diff --git a/tests/conftest.py b/tests/conftest.py
index fd0abf5..a79fbf3 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -90,6 +90,21 @@ def db_and_path(tmpdir):
     db["has_indexes"].create_index(
         ["name"], index_name="name_unique_index", unique=True
     )
+    db["animal.name/with/slashes"].insert(
+        {
+            "id": 1,
+            "name": "Sirocco",
+            "species": 582,
+        },
+        pk="id",
+    )
+    db["table.name/with/slashes.categories"].insert(
+        {
+            "id": 582,
+            "name": "Kākāpō",
+        },
+        pk="id",
+    )
 
     return db, path
 
diff --git a/tests/test_edit_schema.py b/tests/test_edit_schema.py
index dcf086c..7919521 100644
--- a/tests/test_edit_schema.py
+++ b/tests/test_edit_schema.py
@@ -1,4 +1,5 @@
 from datasette.app import Datasette
+from datasette.utils import tilde_encode
 from datasette_edit_schema.utils import (
     potential_foreign_keys,
     get_primary_keys,
@@ -41,8 +42,9 @@ async def test_csrf_required(db_path):
         ("user_with_no_perms", False),
     ),
 )
+@pytest.mark.parametrize("table", ("creatures", "animal.name/with/slashes"))
 @pytest.mark.asyncio
-async def test_table_actions(permission_plugin, ds, actor_id, should_allow):
+async def test_table_actions(permission_plugin, ds, actor_id, should_allow, table):
     ds._rules_allow = [
         Rule(
             actor_id="user_with_edit_schema",
@@ -60,9 +62,13 @@ async def test_table_actions(permission_plugin, ds, actor_id, should_allow):
     cookies = None
     if actor_id:
         cookies = {"ds_actor": ds.sign({"a": {"id": actor_id}}, "actor")}
-    response = await ds.client.get("/data/creatures", cookies=cookies)
+    response = await ds.client.get(
+        ds.urls.table(database="data", table=table), cookies=cookies
+    )
     assert response.status_code == 200
-    fragment = '<a href="/-/edit-schema/data/creatures">Edit table schema'
+    fragment = '<a href="/-/edit-schema/data/{}">Edit table schema'.format(
+        tilde_encode(table)
+    )
     if should_allow:
         # Should have table action
         assert fragment in response.text
@@ -1065,6 +1071,19 @@ def test_potential_primary_keys_primary_key_only_table():
             "Index dropped: name_unique_index",
             [{"columns": ["name"], "name": "name_index", "unique": 0}],
         ),
+        # Test for table with surprising characters in its name
+        (
+            "animal.name/with/slashes",
+            {"add_index": "1", "add_index_column": "species"},
+            "Index added on species",
+            [
+                {
+                    "name": "idx_animal.name/with/slashes_species",
+                    "columns": ["species"],
+                    "unique": 0,
+                }
+            ],
+        ),
     ),
 )
 async def test_add_remove_index(
@@ -1072,13 +1091,17 @@ async def test_add_remove_index(
 ):
     ds = Datasette([db_path])
     cookies = {"ds_actor": ds.sign({"a": {"id": "root"}}, "actor")}
-    csrftoken = (
-        await ds.client.get("/-/edit-schema/data/{}".format(table), cookies=cookies)
-    ).cookies["ds_csrftoken"]
+    get_response = await ds.client.get(
+        "/-/edit-schema/data/{}".format(tilde_encode(table)), cookies=cookies
+    )
+    assert get_response.status_code == 200
+    csrftoken = get_response.cookies["ds_csrftoken"]
     cookies["ds_csrftoken"] = csrftoken
     post_data["csrftoken"] = csrftoken
     response = await ds.client.post(
-        "/-/edit-schema/data/{}".format(table), cookies=cookies, data=post_data
+        "/-/edit-schema/data/{}".format(tilde_encode(table)),
+        cookies=cookies,
+        data=post_data,
     )
     assert response.status_code == 302
     messages = ds.unsign(response.cookies["ds_messages"], "messages")