From b760264ae6a1836be1dfea04d12364cd4f89db0e Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sun, 6 Mar 2022 15:38:44 -0800 Subject: [PATCH] Use dash encoding for table names, refs #1439 --- datasette/url_builder.py | 6 +++--- datasette/views/base.py | 14 +++++++------- datasette/views/table.py | 6 +++--- tests/test_html.py | 8 ++++---- tests/test_internals_urls.py | 2 +- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/datasette/url_builder.py b/datasette/url_builder.py index 2bcda86942..3f54730bf1 100644 --- a/datasette/url_builder.py +++ b/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) diff --git a/datasette/views/base.py b/datasette/views/base.py index c74d6141ec..6d9a574309 100644 --- a/datasette/views/base.py +++ b/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,9 +235,7 @@ 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(), ) @@ -243,11 +243,11 @@ async def async_table_exists(t): 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,7 +467,7 @@ 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(), ) @@ -475,7 +475,7 @@ async def async_table_exists(t): 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): diff --git a/datasette/views/table.py b/datasette/views/table.py index 8ccb737155..1d81755ea0 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -143,7 +143,7 @@ async def display_columns_and_rows( '{flat_pks}'.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 "-", ) diff --git a/tests/test_html.py b/tests/test_html.py index a7ad424894..f6456aa59f 100644 --- a/tests/test_html.py +++ b/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 diff --git a/tests/test_internals_urls.py b/tests/test_internals_urls.py index e486e4c9b4..16515ad6d0 100644 --- a/tests/test_internals_urls.py +++ b/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):