Upgrade to httpx 0.20

* Upgrade to httpx 0.20, closes #1488
* TestClient.post() should not default to following redirects

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"/")

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",

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

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",

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(
+    response_1 = app_client_with_hash.get("/fixtures/simple_primary_key.jsono")
-        "/fixtures/simple_primary_key.jsono", allow_redirects=False
+    response = app_client_with_hash.get(response_1.headers["Location"])
-    )
-    response = app_client_with_hash.get(
-        response_1.headers["Location"], allow_redirects=False
-    )
     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(
+    response = app_client_two_attached_databases_one_immutable.get(path)
-        path, allow_redirects=False
-    )
     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
 
 

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")

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(
+    response = canned_write_client.post("/data/add_name", {"name": "Hello"})
-        "/data/add_name", {"name": "Hello"}, allow_redirects=False
-    )
     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

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"]
 

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(
+    response = app_client_with_hash.get("/fixtures/simple_primary_key/1")
-        "/fixtures/simple_primary_key/1", allow_redirects=False
-    )
     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(
+    response = app_client_with_hash.get("/fixtures/table%2Fwith%2Fslashes.csv/3")
-        "/fixtures/table%2Fwith%2Fslashes.csv/3", allow_redirects=False
-    )
     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"]
 

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

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",

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