Skip to content

gh-47005: fix do_open() to let regular headers override unredirected … #146506

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
orsenthil merged 6 commits into from
Jul 9, 2026
+34 −2
Merged

gh-47005: fix do_open() to let regular headers override unredirected … #146506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
96be436
gh-47005: fix do_open() to let regular headers override unredirected …
Das-Chinmay Mar 27, 2026
35a8c08
gh-47005: add news entry
Das-Chinmay Mar 27, 2026
28eb8ff
gh-47005: trigger CI rerun
Das-Chinmay Mar 27, 2026
a08e13f
gh-47005: fix invalid cross-reference in news entry
Das-Chinmay Mar 27, 2026
f7eab12
Convert the test case into a table driven tests with additional test …
orsenthil Jul 9, 2026
2ac3d2a
Fix the test case for the key capitalized by add_header and add_unred…
orsenthil Jul 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions Lib/test/test_urllib2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -980,6 +980,35 @@ def test_http(self):
self.assertEqual(req.unredirected_hdrs["Host"], "baz")
self.assertEqual(req.unredirected_hdrs["Spam"], "foo")

def test_http_header_priority(self):
# gh-47005: regular headers set via add_header() must override
# unredirected headers with the same name in do_open(), consistent
# with get_header() and header_items().
cases = [
("Content-Type", "application/json", "application/x-www-form-urlencoded"),
("Content-Length", "99", "0"),
("Host", "override.example.com", "internal.example.com"),
("Authorization", "Bearer user-token", "Basic stale="),
("Cookie", "a=1", "b=2"),
("User-Agent", "MyApp/1.0", "Python-urllib/test"),
]
h = urllib.request.AbstractHTTPHandler()
h.parent = MockOpener()

for key, regular, unredirected in cases:
req = Request("http://example.com/", headers={key: regular})
req.timeout = None
req.add_unredirected_header(key, unredirected)

http = MockHTTPClass()
h.do_open(http, req)

sent_headers = dict(http.req_headers)
self.assertEqual(sent_headers[key], regular)
# key is capitalized by add_header() and add_unredirected_header() calls
self.assertEqual(req.get_header(key.capitalize()), regular)
self.assertEqual(dict(req.header_items())[key.capitalize()], regular)

def test_http_body_file(self):
# A regular file - chunked encoding is used unless Content Length is
# already set.
Expand Down
3 changes: 1 addition & 2 deletions Lib/urllib/request.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1293,8 +1293,7 @@ def do_open(self, http_class, req, **http_conn_args):
h.set_debuglevel(self._debuglevel)

headers = dict(req.unredirected_hdrs)
headers.update({k: v for k, v in req.headers.items()
if k not in headers})
headers.update(req.headers)

@orsenthil orsenthil Jul 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks good to me and sufficient.
I am reviewing the rest of the discussion of this long open issue, and ensuring we will have enough tests.


# TODO(jhylton): Should this be redesigned to handle
# persistent connections?
Expand Down
4 changes: 4 additions & 0 deletions Misc/NEWS.d/next/Library/2026-03-26-00-00-00.gh-issue-47005.xxc89c.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Fix :meth:`!urllib.request.AbstractHTTPHandler.do_open` to give regular
headers set via :meth:`~urllib.request.Request.add_header` priority over
unredirected headers, consistent with :meth:`~urllib.request.Request.get_header`
and :meth:`~urllib.request.Request.header_items`.
Loading