Skip to content

Commit dcadf7a

Browse files
authored
Debug issues with duplicate POST keys (#16755)
* Exercise duplicate POST keys in functional test * Assert on response text too * Explicitly just count keys instead of casting * Include URL in error message * Add classifiers to test too * Linting * Failing test * Test both ways of providing the action
1 parent d3ed6e0 commit dcadf7a

File tree

3 files changed

+34
-12
lines changed

3 files changed

+34
-12
lines changed

tests/functional/forklift/test_legacy.py

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
import pymacaroons
1818
import pytest
1919

20+
from webob.multidict import MultiDict
21+
2022
from warehouse.macaroons import caveats
2123

2224
from ...common.db.accounts import UserFactory
@@ -58,7 +60,14 @@ def test_remove_doc_upload(webtest):
5860
)
5961

6062

61-
def test_file_upload(webtest):
63+
@pytest.mark.parametrize(
64+
("upload_url", "additional_data"),
65+
[
66+
("/legacy/?:action=file_upload", {}),
67+
("/legacy/", {":action": "file_upload", "protocol_version": "1"}),
68+
],
69+
)
70+
def test_file_upload(webtest, upload_url, additional_data):
6271
user = UserFactory.create(
6372
with_verified_primary_email=True,
6473
password=( # 'password'
@@ -90,18 +99,27 @@ def test_file_upload(webtest):
9099
with open("./tests/functional/_fixtures/sampleproject-3.0.0.tar.gz", "rb") as f:
91100
content = f.read()
92101

93-
webtest.post(
94-
"/legacy/?:action=file_upload",
95-
headers={"Authorization": f"Basic {credentials}"},
96-
params={
102+
params = MultiDict(
103+
{
97104
"name": "sampleproject",
98105
"sha256_digest": (
99106
"117ed88e5db073bb92969a7545745fd977ee85b7019706dd256a64058f70963d"
100107
),
101108
"filetype": "sdist",
102109
"metadata_version": "2.1",
103110
"version": "3.0.0",
104-
},
111+
}
112+
)
113+
params.update(additional_data)
114+
params.add("project-url", "https://example.com/foo")
115+
params.add("project-url", "https://example.com/bar")
116+
params.add("classifiers", "Programming Language :: Python :: 3.10")
117+
params.add("classifiers", "Programming Language :: Python :: 3.11")
118+
119+
webtest.post(
120+
upload_url,
121+
headers={"Authorization": f"Basic {credentials}"},
122+
params=params,
105123
upload_files=[("content", "sampleproject-3.0.0.tar.gz", content)],
106124
status=HTTPStatus.OK,
107125
)

tests/functional/test_config.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,4 +36,6 @@ def test_rejects_duplicate_post_keys(webtest, socket_enabled):
3636
body.add("foo", "bar")
3737
body.add("foo", "baz")
3838

39-
webtest.post("/account/login", params=body, status=HTTPStatus.BAD_REQUEST)
39+
resp = webtest.post("/account/login", params=body, status=HTTPStatus.BAD_REQUEST)
40+
assert "POST body may not contain duplicate keys" in resp.body.decode()
41+
assert "(URL: 'http://localhost/account/login')" in resp.body.decode()

warehouse/config.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -274,11 +274,13 @@ def reject_duplicate_post_keys_view(view, info):
274274
@functools.wraps(view)
275275
def wrapped(context, request):
276276
if request.POST:
277-
# Attempt to cast to a dict to catch duplicate keys
278-
try:
279-
dict(**request.POST)
280-
except TypeError:
281-
return HTTPBadRequest("POST body may not contain duplicate keys")
277+
# Determine if there are any duplicate keys
278+
keys = list(request.POST.keys())
279+
if len(keys) != len(set(keys)):
280+
return HTTPBadRequest(
281+
"POST body may not contain duplicate keys "
282+
f"(URL: {request.url!r})"
283+
)
282284

283285
# Casting succeeded, so just return the regular view
284286
return view(context, request)

0 commit comments

Comments
 (0)