Use dash encoding for table names, refs #1439

datasette/url_builder.py

@@ -1,4 +1,4 @@
-from .utils import path_with_format, HASH_LENGTH, PrefixedUrlString
+from .utils import dash_encode, path_with_format, HASH_LENGTH, PrefixedUrlString
 import urllib
 
 
@@ -38,13 +38,13 @@ def database(self, database, format=None):
         return path
 
     def table(self, database, table, format=None):
-        path = f"{self.database(database)}/{urllib.parse.quote_plus(table)}"
+        path = f"{self.database(database)}/{dash_encode(table)}"
         if format is not None:
             path = path_with_format(path=path, format=format)
         return PrefixedUrlString(path)
 
     def query(self, database, query, format=None):
-        path = f"{self.database(database)}/{urllib.parse.quote_plus(query)}"
+        path = f"{self.database(database)}/{dash_encode(query)}"
         if format is not None:
             path = path_with_format(path=path, format=format)
         return PrefixedUrlString(path)

datasette/views/base.py

@@ -17,6 +17,8 @@
     InvalidSql,
     LimitedWriter,
     call_with_supported_arguments,
+    dash_decode,
+    dash_encode,
     path_from_row_pks,
     path_with_added_args,
     path_with_removed_args,
@@ -233,21 +235,19 @@ async def async_table_exists(t):
                     return await db.table_exists(t)
 
                 table, _format = await resolve_table_and_format(
-                    table_and_format=urllib.parse.unquote_plus(
-                        kwargs["table_and_format"]
-                    ),
+                    table_and_format=dash_decode(kwargs["table_and_format"]),
                     table_exists=async_table_exists,
                     allowed_formats=self.ds.renderers.keys(),
                 )
                 kwargs["table"] = table
                 if _format:
                     kwargs["as_format"] = f".{_format}"
             elif kwargs.get("table"):
-                kwargs["table"] = urllib.parse.unquote_plus(kwargs["table"])
+                kwargs["table"] = dash_decode(kwargs["table"])
 
             should_redirect = self.ds.urls.path(f"{name}-{expected}")
             if kwargs.get("table"):
-                should_redirect += "/" + urllib.parse.quote_plus(kwargs["table"])
+                should_redirect += "/" + dash_encode(kwargs["table"])
             if kwargs.get("pk_path"):
                 should_redirect += "/" + kwargs["pk_path"]
             if kwargs.get("as_format"):
@@ -467,15 +467,15 @@ async def async_table_exists(t):
                 return await db.table_exists(t)
 
             table, _ext_format = await resolve_table_and_format(
-                table_and_format=urllib.parse.unquote_plus(args["table_and_format"]),
+                table_and_format=dash_decode(args["table_and_format"]),
                 table_exists=async_table_exists,
                 allowed_formats=self.ds.renderers.keys(),
             )
             _format = _format or _ext_format
             args["table"] = table
             del args["table_and_format"]
         elif "table" in args:
-            args["table"] = urllib.parse.unquote_plus(args["table"])
+            args["table"] = dash_decode(args["table"])
         return _format, args
 
     async def view_get(self, request, database, hash, correct_hash_provided, **kwargs):

datasette/views/table.py

@@ -143,7 +143,7 @@ async def display_columns_and_rows(
                             '<a href="{base_url}{database}/{table}/{flat_pks_quoted}">{flat_pks}</a>'.format(
                                 base_url=base_url,
                                 database=database,
-                                table=urllib.parse.quote_plus(table),
+                                table=dash_encode(table),
                                 flat_pks=str(markupsafe.escape(pk_path)),
                                 flat_pks_quoted=path_from_row_pks(row, pks, not pks),
                             )
@@ -200,8 +200,8 @@ async def display_columns_and_rows(
                         link_template.format(
                             database=database,
                             base_url=base_url,
-                            table=urllib.parse.quote_plus(other_table),
-                            link_id=urllib.parse.quote_plus(str(value)),
+                            table=dash_encode(other_table),
+                            link_id=dash_encode(str(value)),
                             id=str(markupsafe.escape(value)),
                             label=str(markupsafe.escape(label)) or "-",
                         )

tests/test_html.py

@@ -140,7 +140,7 @@ def test_database_page(app_client):
     assert queries_ul is not None
     assert [
         (
-            "/fixtures/%F0%9D%90%9C%F0%9D%90%A2%F0%9D%90%AD%F0%9D%90%A2%F0%9D%90%9E%F0%9D%90%AC",
+            "/fixtures/-F0-9D-90-9C-F0-9D-90-A2-F0-9D-90-AD-F0-9D-90-A2-F0-9D-90-9E-F0-9D-90-AC",
             "𝐜𝐢𝐭𝐢𝐞𝐬",
         ),
         ("/fixtures/from_async_hook", "from_async_hook"),
@@ -193,11 +193,11 @@ def test_row_redirects_with_url_hash(app_client_with_hash):
 
 
 def test_row_strange_table_name_with_url_hash(app_client_with_hash):
-    response = app_client_with_hash.get("/fixtures/table%2Fwith%2Fslashes.csv/3")
+    response = app_client_with_hash.get("/fixtures/table-2Fwith-2Fslashes-2Ecsv/3")
     assert response.status == 302
-    assert response.headers["Location"].endswith("/table%2Fwith%2Fslashes.csv/3")
+    assert response.headers["Location"].endswith("/table-2Fwith-2Fslashes-2Ecsv/3")
     response = app_client_with_hash.get(
-        "/fixtures/table%2Fwith%2Fslashes.csv/3", follow_redirects=True
+        "/fixtures/table-2Fwith-2Fslashes-2Ecsv/3", follow_redirects=True
     )
     assert response.status == 200
 

tests/test_internals_urls.py

@@ -121,7 +121,7 @@ def test_database(ds, base_url, format, expected):
         ("/", "name", None, "/_memory/name"),
         ("/prefix/", "name", None, "/prefix/_memory/name"),
         ("/", "name", "json", "/_memory/name.json"),
-        ("/", "name.json", "json", "/_memory/name.json?_format=json"),
+        ("/", "name.json", "json", "/_memory/name-2Ejson.json"),
     ],
 )
 def test_table_and_query(ds, base_url, name, format, expected):