From a5ede3cdd455e2bb1a1fb2f4e1b5a9855caf5179 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sun, 24 Jan 2021 21:13:05 -0800 Subject: [PATCH] Fixed bug loading database called 'test-database (1).sqlite' Closes #1181. Also now ensures that database URLs have special characters URL-quoted. --- datasette/url_builder.py | 6 ++++-- datasette/views/base.py | 3 ++- docs/changelog.rst | 10 ++++++---- tests/test_api.py | 14 +++++++------- tests/test_cli.py | 23 +++++++++++++++++++++++ tests/test_html.py | 6 +++--- tests/test_internals_urls.py | 20 ++++++++++---------- 7 files changed, 55 insertions(+), 27 deletions(-) diff --git a/datasette/url_builder.py b/datasette/url_builder.py index 3034b664..2bcda869 100644 --- a/datasette/url_builder.py +++ b/datasette/url_builder.py @@ -30,9 +30,11 @@ class Urls: def database(self, database, format=None): db = self.ds.databases[database] if self.ds.setting("hash_urls") and db.hash: - path = self.path(f"{database}-{db.hash[:HASH_LENGTH]}", format=format) + path = self.path( + f"{urllib.parse.quote(database)}-{db.hash[:HASH_LENGTH]}", format=format + ) else: - path = self.path(database, format=format) + path = self.path(urllib.parse.quote(database), format=format) return path def table(self, database, table, format=None): diff --git a/datasette/views/base.py b/datasette/views/base.py index a21b9298..ba0f7d4c 100644 --- a/datasette/views/base.py +++ b/datasette/views/base.py @@ -181,6 +181,7 @@ class DataView(BaseView): async def resolve_db_name(self, request, db_name, **kwargs): hash = None name = None + db_name = urllib.parse.unquote_plus(db_name) if db_name not in self.ds.databases and "-" in db_name: # No matching DB found, maybe it's a name-hash? name_bit, hash_bit = db_name.rsplit("-", 1) @@ -191,7 +192,7 @@ class DataView(BaseView): hash = hash_bit else: name = db_name - name = urllib.parse.unquote_plus(name) + try: db = self.ds.databases[name] except KeyError: diff --git a/docs/changelog.rst b/docs/changelog.rst index ac2ac8c9..abc2f4f9 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,12 +4,14 @@ Changelog ========= -.. _v0_54_a0: +.. _v0_54: + +0.54 (2021-01-24) +----------------- + + -0.54a0 (2020-12-19) -------------------- -**Alpha release**. Release notes in progress. - Improved support for named in-memory databases. (`#1151 `__) - New ``_internal`` in-memory database tracking attached databases, tables and columns. (`#1150 `__) diff --git a/tests/test_api.py b/tests/test_api.py index 3b4f3437..0d1bddd3 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -609,17 +609,17 @@ def test_no_files_uses_memory_database(app_client_no_files): assert response.status == 200 assert { ":memory:": { + "name": ":memory:", "hash": None, "color": "f7935d", + "path": "/%3Amemory%3A", + "tables_and_views_truncated": [], + "tables_and_views_more": False, + "tables_count": 0, + "table_rows_sum": 0, + "show_table_row_counts": False, "hidden_table_rows_sum": 0, "hidden_tables_count": 0, - "name": ":memory:", - "show_table_row_counts": False, - "path": "/:memory:", - "table_rows_sum": 0, - "tables_count": 0, - "tables_and_views_more": False, - "tables_and_views_truncated": [], "views_count": 0, "private": False, } diff --git a/tests/test_cli.py b/tests/test_cli.py index 1d806bff..c42c22ea 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -17,6 +17,7 @@ import pytest import sys import textwrap from unittest import mock +import urllib @pytest.fixture @@ -255,3 +256,25 @@ def test_serve_duplicate_database_names(ensure_eventloop, tmpdir): assert result.exit_code == 0, result.output databases = json.loads(result.output) assert {db["name"] for db in databases} == {"db", "db_2"} + + +@pytest.mark.parametrize( + "filename", ["test-database (1).sqlite", "database (1).sqlite"] +) +def test_weird_database_names(ensure_eventloop, tmpdir, filename): + # https://github.com/simonw/datasette/issues/1181 + runner = CliRunner() + db_path = str(tmpdir / filename) + sqlite3.connect(db_path).execute("vacuum") + result1 = runner.invoke(cli, [db_path, "--get", "/"]) + assert result1.exit_code == 0, result1.output + filename_no_stem = filename.rsplit(".", 1)[0] + expected_link = '{}'.format( + urllib.parse.quote(filename_no_stem), filename_no_stem + ) + assert expected_link in result1.output + # Now try hitting that database page + result2 = runner.invoke( + cli, [db_path, "--get", "/{}".format(urllib.parse.quote(filename_no_stem))] + ) + assert result2.exit_code == 0, result2.output diff --git a/tests/test_html.py b/tests/test_html.py index 08d17ca7..6c33fba7 100644 --- a/tests/test_html.py +++ b/tests/test_html.py @@ -30,7 +30,7 @@ def test_homepage(app_client_two_attached_databases): # Should be two attached databases assert [ {"href": "/fixtures", "text": "fixtures"}, - {"href": "/extra database", "text": "extra database"}, + {"href": r"/extra%20database", "text": "extra database"}, ] == [{"href": a["href"], "text": a.text.strip()} for a in soup.select("h2 a")] # The first attached database should show count text and attached tables h2 = soup.select("h2")[1] @@ -44,8 +44,8 @@ def test_homepage(app_client_two_attached_databases): {"href": a["href"], "text": a.text.strip()} for a in links_p.findAll("a") ] assert [ - {"href": "/extra database/searchable", "text": "searchable"}, - {"href": "/extra database/searchable_view", "text": "searchable_view"}, + {"href": r"/extra%20database/searchable", "text": "searchable"}, + {"href": r"/extra%20database/searchable_view", "text": "searchable_view"}, ] == table_links diff --git a/tests/test_internals_urls.py b/tests/test_internals_urls.py index fd05c1b6..e6f405b3 100644 --- a/tests/test_internals_urls.py +++ b/tests/test_internals_urls.py @@ -103,9 +103,9 @@ def test_logout(ds, base_url, expected): @pytest.mark.parametrize( "base_url,format,expected", [ - ("/", None, "/:memory:"), - ("/prefix/", None, "/prefix/:memory:"), - ("/", "json", "/:memory:.json"), + ("/", None, "/%3Amemory%3A"), + ("/prefix/", None, "/prefix/%3Amemory%3A"), + ("/", "json", "/%3Amemory%3A.json"), ], ) def test_database(ds, base_url, format, expected): @@ -118,10 +118,10 @@ def test_database(ds, base_url, format, expected): @pytest.mark.parametrize( "base_url,name,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", None, "/%3Amemory%3A/name"), + ("/prefix/", "name", None, "/prefix/%3Amemory%3A/name"), + ("/", "name", "json", "/%3Amemory%3A/name.json"), + ("/", "name.json", "json", "/%3Amemory%3A/name.json?_format=json"), ], ) def test_table_and_query(ds, base_url, name, format, expected): @@ -137,9 +137,9 @@ def test_table_and_query(ds, base_url, name, format, expected): @pytest.mark.parametrize( "base_url,format,expected", [ - ("/", None, "/:memory:/facetable/1"), - ("/prefix/", None, "/prefix/:memory:/facetable/1"), - ("/", "json", "/:memory:/facetable/1.json"), + ("/", None, "/%3Amemory%3A/facetable/1"), + ("/prefix/", None, "/prefix/%3Amemory%3A/facetable/1"), + ("/", "json", "/%3Amemory%3A/facetable/1.json"), ], ) def test_row(ds, base_url, format, expected):