diff --git a/datasette/app.py b/datasette/app.py index 06db740e..1f69c2b3 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -1139,6 +1139,7 @@ class DatasetteRouter: raw_path = scope.get("raw_path") if raw_path: path = raw_path.decode("ascii") + path = path.partition("?")[0] return await self.route_path(scope, receive, send, path) async def route_path(self, scope, receive, send, path): @@ -1192,7 +1193,9 @@ class DatasetteRouter: async def handle_404(self, request, send, exception=None): # If URL has a trailing slash, redirect to URL without it - path = request.scope.get("raw_path", request.scope["path"].encode("utf8")) + path = request.scope.get( + "raw_path", request.scope["path"].encode("utf8") + ).partition(b"?")[0] context = {} if path.endswith(b"/"): path = path.rstrip(b"/") diff --git a/datasette/utils/asgi.py b/datasette/utils/asgi.py index 5fa03b0a..696944df 100644 --- a/datasette/utils/asgi.py +++ b/datasette/utils/asgi.py @@ -75,7 +75,7 @@ class Request: @property def path(self): if self.scope.get("raw_path") is not None: - return self.scope["raw_path"].decode("latin-1") + return self.scope["raw_path"].decode("latin-1").partition("?")[0] else: path = self.scope["path"] if isinstance(path, str): @@ -122,7 +122,7 @@ class Request: "http_version": "1.1", "method": method, "path": path, - "raw_path": path.encode("latin-1"), + "raw_path": path_with_query_string.encode("latin-1"), "query_string": query_string.encode("latin-1"), "scheme": scheme, "type": "http", diff --git a/datasette/utils/testing.py b/datasette/utils/testing.py index a169a83d..94750b1f 100644 --- a/datasette/utils/testing.py +++ b/datasette/utils/testing.py @@ -55,10 +55,10 @@ class TestClient: @async_to_sync async def get( - self, path, allow_redirects=True, redirect_count=0, method="GET", cookies=None + self, path, follow_redirects=False, redirect_count=0, method="GET", cookies=None ): return await self._request( - path, allow_redirects, redirect_count, method, cookies + path, follow_redirects, redirect_count, method, cookies ) @async_to_sync @@ -67,7 +67,7 @@ class TestClient: path, post_data=None, body=None, - allow_redirects=True, + follow_redirects=False, redirect_count=0, content_type="application/x-www-form-urlencoded", cookies=None, @@ -90,7 +90,7 @@ class TestClient: body = urlencode(post_data, doseq=True) return await self._request( path=path, - allow_redirects=allow_redirects, + follow_redirects=follow_redirects, redirect_count=redirect_count, method="POST", cookies=cookies, @@ -103,7 +103,7 @@ class TestClient: async def request( self, path, - allow_redirects=True, + follow_redirects=True, redirect_count=0, method="GET", cookies=None, @@ -113,7 +113,7 @@ class TestClient: ): return await self._request( path, - allow_redirects=allow_redirects, + follow_redirects=follow_redirects, redirect_count=redirect_count, method=method, cookies=cookies, @@ -125,7 +125,7 @@ class TestClient: async def _request( self, path, - allow_redirects=True, + follow_redirects=True, redirect_count=0, method="GET", cookies=None, @@ -139,19 +139,19 @@ class TestClient: httpx_response = await self.ds.client.request( method, path, - allow_redirects=allow_redirects, + follow_redirects=follow_redirects, avoid_path_rewrites=True, cookies=cookies, headers=headers, content=post_body, ) response = TestResponse(httpx_response) - if allow_redirects and response.status in (301, 302): + if follow_redirects and response.status in (301, 302): assert ( redirect_count < self.max_redirects ), f"Redirected {redirect_count} times, max_redirects={self.max_redirects}" location = response.headers["Location"] return await self._request( - location, allow_redirects=True, redirect_count=redirect_count + 1 + location, follow_redirects=True, redirect_count=redirect_count + 1 ) return response diff --git a/setup.py b/setup.py index 905fed16..45eac040 100644 --- a/setup.py +++ b/setup.py @@ -47,7 +47,7 @@ setup( "click-default-group~=1.2.2", "Jinja2>=2.10.3,<3.1.0", "hupper~=1.9", - "httpx>=0.17", + "httpx>=0.20", "pint~=0.9", "pluggy>=0.13,<1.1", "uvicorn~=0.11", diff --git a/tests/test_api.py b/tests/test_api.py index 1e93c62e..38d1ba08 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -629,7 +629,7 @@ def test_no_files_uses_memory_database(app_client_no_files): ), ) def test_old_memory_urls_redirect(app_client_no_files, path, expected_redirect): - response = app_client_no_files.get(path, allow_redirects=False) + response = app_client_no_files.get(path) assert response.status == 301 assert response.headers["location"] == expected_redirect @@ -708,12 +708,8 @@ def test_table_not_exists_json(app_client): def test_jsono_redirects_to_shape_objects(app_client_with_hash): - response_1 = app_client_with_hash.get( - "/fixtures/simple_primary_key.jsono", allow_redirects=False - ) - response = app_client_with_hash.get( - response_1.headers["Location"], allow_redirects=False - ) + response_1 = app_client_with_hash.get("/fixtures/simple_primary_key.jsono") + response = app_client_with_hash.get(response_1.headers["Location"]) assert response.status == 302 assert response.headers["Location"].endswith("?_shape=objects") @@ -1488,7 +1484,7 @@ def test_settings_json(app_client): ), ) def test_config_redirects_to_settings(app_client, path, expected_redirect): - response = app_client.get(path, allow_redirects=False) + response = app_client.get(path) assert response.status == 301 assert response.headers["Location"] == expected_redirect @@ -1834,9 +1830,7 @@ def test_hash_parameter( current_hash = app_client_two_attached_databases_one_immutable.ds.databases[ "fixtures" ].hash[:7] - response = app_client_two_attached_databases_one_immutable.get( - path, allow_redirects=False - ) + response = app_client_two_attached_databases_one_immutable.get(path) assert response.status == 302 location = response.headers["Location"] assert expected_redirect.replace("HASH", current_hash) == location @@ -1844,7 +1838,7 @@ def test_hash_parameter( def test_hash_parameter_ignored_for_mutable_databases(app_client): path = "/fixtures/facetable.json?_hash=1" - response = app_client.get(path, allow_redirects=False) + response = app_client.get(path) assert response.status == 200 @@ -1976,7 +1970,9 @@ def test_cors(app_client_with_cors, path, status_code): ), ) def test_database_with_space_in_name(app_client_two_attached_databases, path): - response = app_client_two_attached_databases.get("/extra database" + path) + response = app_client_two_attached_databases.get( + "/extra database" + path, follow_redirects=True + ) assert response.status == 200 diff --git a/tests/test_auth.py b/tests/test_auth.py index 16397b7a..974f89ea 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -10,7 +10,6 @@ def test_auth_token(app_client): path = f"/-/auth-token?token={app_client.ds._root_token}" response = app_client.get( path, - allow_redirects=False, ) assert 302 == response.status assert "/" == response.headers["Location"] @@ -23,7 +22,6 @@ def test_auth_token(app_client): 403 == app_client.get( path, - allow_redirects=False, ).status ) @@ -78,14 +76,13 @@ def test_logout(app_client): in response2.text ) # If logged out you get a redirect to / - response3 = app_client.get("/-/logout", allow_redirects=False) + response3 = app_client.get("/-/logout") assert 302 == response3.status # A POST to that page should log the user out response4 = app_client.post( "/-/logout", csrftoken_from=True, cookies={"ds_actor": app_client.actor_cookie({"id": "test"})}, - allow_redirects=False, ) # The ds_actor cookie should have been unset assert response4.cookie_was_deleted("ds_actor") diff --git a/tests/test_canned_queries.py b/tests/test_canned_queries.py index b8c2baec..cea81ec7 100644 --- a/tests/test_canned_queries.py +++ b/tests/test_canned_queries.py @@ -59,7 +59,6 @@ def test_insert(canned_write_client): response = canned_write_client.post( "/data/add_name", {"name": "Hello"}, - allow_redirects=False, csrftoken_from=True, cookies={"foo": "bar"}, ) @@ -95,16 +94,13 @@ def test_insert_with_cookies_requires_csrf(canned_write_client): response = canned_write_client.post( "/data/add_name", {"name": "Hello"}, - allow_redirects=False, cookies={"foo": "bar"}, ) assert 403 == response.status def test_insert_no_cookies_no_csrf(canned_write_client): - response = canned_write_client.post( - "/data/add_name", {"name": "Hello"}, allow_redirects=False - ) + response = canned_write_client.post("/data/add_name", {"name": "Hello"}) assert 302 == response.status assert "/data/add_name?success" == response.headers["Location"] @@ -114,7 +110,6 @@ def test_custom_success_message(canned_write_client): "/data/delete_name", {"rowid": 1}, cookies={"ds_actor": canned_write_client.actor_cookie({"id": "root"})}, - allow_redirects=False, csrftoken_from=True, ) assert 302 == response.status @@ -129,7 +124,6 @@ def test_insert_error(canned_write_client): response = canned_write_client.post( "/data/add_name_specify_id", {"rowid": 1, "name": "Should fail"}, - allow_redirects=False, csrftoken_from=True, ) assert 302 == response.status @@ -145,7 +139,6 @@ def test_insert_error(canned_write_client): response = canned_write_client.post( "/data/add_name_specify_id", {"rowid": 1, "name": "Should fail"}, - allow_redirects=False, csrftoken_from=True, ) assert [["ERROR", 3]] == canned_write_client.ds.unsign( @@ -168,7 +161,6 @@ def test_json_post_body(canned_write_client): response = canned_write_client.post( "/data/add_name", body=json.dumps({"name": ["Hello", "there"]}), - allow_redirects=False, ) assert 302 == response.status assert "/data/add_name?success" == response.headers["Location"] @@ -189,7 +181,6 @@ def test_json_response(canned_write_client, headers, body, querystring): response = canned_write_client.post( "/data/add_name" + (querystring or ""), body=body, - allow_redirects=False, headers=headers, ) assert 200 == response.status @@ -331,7 +322,6 @@ def test_magic_parameters_csrf_json(magic_parameters_client, use_csrf, return_js f"/data/runme_post{qs}", {}, csrftoken_from=use_csrf or None, - allow_redirects=False, ) if return_json: assert response.status == 200 diff --git a/tests/test_custom_pages.py b/tests/test_custom_pages.py index 76c67397..66b7437a 100644 --- a/tests/test_custom_pages.py +++ b/tests/test_custom_pages.py @@ -67,13 +67,13 @@ def test_custom_content_type(custom_pages_client): def test_redirect(custom_pages_client): - response = custom_pages_client.get("/redirect", allow_redirects=False) + response = custom_pages_client.get("/redirect") assert 302 == response.status assert "/example" == response.headers["Location"] def test_redirect2(custom_pages_client): - response = custom_pages_client.get("/redirect2", allow_redirects=False) + response = custom_pages_client.get("/redirect2") assert 301 == response.status assert "/example" == response.headers["Location"] diff --git a/tests/test_html.py b/tests/test_html.py index 151ac5c3..5f2ba2f1 100644 --- a/tests/test_html.py +++ b/tests/test_html.py @@ -100,9 +100,9 @@ def test_not_allowed_methods(): def test_database_page_redirects_with_url_hash(app_client_with_hash): - response = app_client_with_hash.get("/fixtures", allow_redirects=False) - assert response.status == 302 response = app_client_with_hash.get("/fixtures") + assert response.status == 302 + response = app_client_with_hash.get("/fixtures", follow_redirects=True) assert "fixtures" in response.text @@ -161,22 +161,22 @@ def test_sql_time_limit(app_client_shorter_time_limit): def test_row_redirects_with_url_hash(app_client_with_hash): - response = app_client_with_hash.get( - "/fixtures/simple_primary_key/1", allow_redirects=False - ) + response = app_client_with_hash.get("/fixtures/simple_primary_key/1") assert response.status == 302 assert response.headers["Location"].endswith("/1") - response = app_client_with_hash.get("/fixtures/simple_primary_key/1") + response = app_client_with_hash.get( + "/fixtures/simple_primary_key/1", follow_redirects=True + ) assert response.status == 200 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", allow_redirects=False - ) + response = app_client_with_hash.get("/fixtures/table%2Fwith%2Fslashes.csv/3") assert response.status == 302 assert response.headers["Location"].endswith("/table%2Fwith%2Fslashes.csv/3") - response = app_client_with_hash.get("/fixtures/table%2Fwith%2Fslashes.csv/3") + response = app_client_with_hash.get( + "/fixtures/table%2Fwith%2Fslashes.csv/3", follow_redirects=True + ) assert response.status == 200 @@ -255,13 +255,13 @@ def test_add_filter_redirects(app_client): ) path_base = "/fixtures/simple_primary_key" path = path_base + "?" + filter_args - response = app_client.get(path, allow_redirects=False) + response = app_client.get(path) assert response.status == 302 assert response.headers["Location"].endswith("?content__startswith=x") # Adding a redirect to an existing query string: path = path_base + "?foo=bar&" + filter_args - response = app_client.get(path, allow_redirects=False) + response = app_client.get(path) assert response.status == 302 assert response.headers["Location"].endswith("?foo=bar&content__startswith=x") @@ -277,7 +277,7 @@ def test_add_filter_redirects(app_client): } ) ) - response = app_client.get(path, allow_redirects=False) + response = app_client.get(path) assert response.status == 302 assert response.headers["Location"].endswith("?content__isnull=5") @@ -299,7 +299,7 @@ def test_existing_filter_redirects(app_client): } path_base = "/fixtures/simple_primary_key" path = path_base + "?" + urllib.parse.urlencode(filter_args) - response = app_client.get(path, allow_redirects=False) + response = app_client.get(path) assert response.status == 302 assert_querystring_equal( "name__contains=hello&age__gte=22&age__lt=30&name__contains=world", @@ -309,7 +309,7 @@ def test_existing_filter_redirects(app_client): # Setting _filter_column_3 to empty string should remove *_3 entirely filter_args["_filter_column_3"] = "" path = path_base + "?" + urllib.parse.urlencode(filter_args) - response = app_client.get(path, allow_redirects=False) + response = app_client.get(path) assert response.status == 302 assert_querystring_equal( "name__contains=hello&age__gte=22&name__contains=world", @@ -317,7 +317,7 @@ def test_existing_filter_redirects(app_client): ) # ?_filter_op=exact should be removed if unaccompanied by _fiter_column - response = app_client.get(path_base + "?_filter_op=exact", allow_redirects=False) + response = app_client.get(path_base + "?_filter_op=exact") assert response.status == 302 assert "?" not in response.headers["Location"] @@ -336,7 +336,7 @@ def test_empty_search_parameter_gets_removed(app_client): } ) ) - response = app_client.get(path, allow_redirects=False) + response = app_client.get(path) assert response.status == 302 assert response.headers["Location"].endswith("?name__exact=chidi") @@ -360,7 +360,7 @@ def test_sort_by_desc_redirects(app_client): + "?" + urllib.parse.urlencode({"_sort": "sortable", "_sort_by_desc": "1"}) ) - response = app_client.get(path, allow_redirects=False) + response = app_client.get(path) assert response.status == 302 assert response.headers["Location"].endswith("?_sort_desc=sortable") @@ -1148,7 +1148,7 @@ def test_404(app_client, path): [("/fixtures/", "/fixtures"), ("/fixtures/simple_view/", "/fixtures/simple_view")], ) def test_404_trailing_slash_redirect(app_client, path, expected_redirect): - response = app_client.get(path, allow_redirects=False) + response = app_client.get(path) assert 302 == response.status assert expected_redirect == response.headers["Location"] diff --git a/tests/test_internals_datasette_client.py b/tests/test_internals_datasette_client.py index c538bef1..8c5b5bd3 100644 --- a/tests/test_internals_datasette_client.py +++ b/tests/test_internals_datasette_client.py @@ -42,7 +42,6 @@ async def test_client_post(datasette, prefix): data={ "message": "A message", }, - allow_redirects=False, ) assert isinstance(response, httpx.Response) assert response.status_code == 302 diff --git a/tests/test_internals_request.py b/tests/test_internals_request.py index fe273645..c42cfbd3 100644 --- a/tests/test_internals_request.py +++ b/tests/test_internals_request.py @@ -97,11 +97,14 @@ def test_request_url_vars(): [("/", "", "/"), ("/", "foo=bar", "/?foo=bar"), ("/foo", "bar", "/foo?bar")], ) def test_request_properties(path, query_string, expected_full_path): + path_with_query_string = path + if query_string: + path_with_query_string += "?" + query_string scope = { "http_version": "1.1", "method": "POST", "path": path, - "raw_path": path.encode("latin-1"), + "raw_path": path_with_query_string.encode("latin-1"), "query_string": query_string.encode("latin-1"), "scheme": "http", "type": "http", diff --git a/tests/test_plugins.py b/tests/test_plugins.py index a024c39b..7dac8002 100644 --- a/tests/test_plugins.py +++ b/tests/test_plugins.py @@ -71,7 +71,7 @@ def test_hook_plugin_prepare_connection_arguments(app_client): }, ), ( - "/fixtures/", + "/fixtures", { "template": "database.html", "database": "fixtures", @@ -106,6 +106,7 @@ def test_hook_plugin_prepare_connection_arguments(app_client): ) def test_hook_extra_css_urls(app_client, path, expected_decoded_object): response = app_client.get(path) + assert response.status == 200 links = Soup(response.body, "html.parser").findAll("link") special_href = [ l for l in links if l.attrs["href"].endswith("/extra-css-urls-demo.css") @@ -263,7 +264,7 @@ def test_plugin_config_file(app_client): }, ), ( - "/fixtures/", + "/fixtures", { "template": "database.html", "database": "fixtures", @@ -640,7 +641,7 @@ async def test_hook_permission_allowed(app_client, action, expected): def test_actor_json(app_client): assert {"actor": None} == app_client.get("/-/actor.json").json assert {"actor": {"id": "bot2", "1+1": 2}} == app_client.get( - "/-/actor.json/?_bot2=1" + "/-/actor.json?_bot2=1" ).json @@ -674,7 +675,7 @@ def test_hook_register_routes_with_datasette(configured_path): assert configured_path.upper() == response.text # Other one should 404 other_path = [p for p in ("path1", "path2") if configured_path != p][0] - assert client.get(f"/{other_path}/").status == 404 + assert client.get(f"/{other_path}/", follow_redirects=True).status == 404 def test_hook_register_routes_post(app_client): @@ -777,7 +778,7 @@ def test_hook_register_magic_parameters(restore_working_directory): }, ) as client: response = client.post("/data/runme", {}, csrftoken_from=True) - assert 200 == response.status + assert 302 == response.status actual = client.get("/data/logs.json?_sort_desc=rowid&_shape=array").json assert [{"rowid": 1, "line": "1.1"}] == actual # Now try the GET request against get_uuid @@ -794,7 +795,7 @@ def test_hook_forbidden(restore_working_directory): ) as client: response = client.get("/") assert 403 == response.status - response2 = client.get("/data2", allow_redirects=False) + response2 = client.get("/data2") assert 302 == response2.status assert "/login?message=view-database" == response2.headers["Location"] assert "view-database" == client.ds._last_forbidden_message