gh-47005: fix do_open() to let regular headers override unredirected …#146506
Conversation
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
This PR is stale because it has been open for 30 days with no activity. |
…ected headers AbstractHTTPHandler.do_open() was building the request header dict by starting with unredirected_hdrs and only inserting regular headers that were not already present, giving unredirected headers priority. This contradicts get_header() and header_items(), both of which give regular headers the higher priority. Fix by unconditionally updating with req.headers so that a header set via add_header() always overrides one set via add_unredirected_header().
| 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) |
There was a problem hiding this comment.
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.
orsenthil
force-pushed
the
fix-urllib-header-priority
branch
from
c8bd4af to
f7eab12
Compare
|
I have reviewed the use cases of this long open bug. Headers set using add_header() should not be ignored in favor of I was only worried if we will be breaking some existing use-case that relied on this faulty behavior. But the possibility of that seems very remote. When an user has explicitly set The patch is simple and clean. |
orsenthil
added
needs backport to 3.13
|
Thanks @Das-Chinmay for the PR, and @orsenthil for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14, 3.15. |
|
GH-153377 is a backport of this pull request to the 3.15 branch. |
bedevere-app
Bot
removed
the
needs backport to 3.15
|
GH-153378 is a backport of this pull request to the 3.14 branch. |
|
GH-153379 is a backport of this pull request to the 3.13 branch. |
…rected … (GH-146506) (#153379) gh-47005: fix do_open() to let regular headers override unredirected … (GH-146506) AbstractHTTPHandler.do_open() was building the request header dict by starting with unredirected_hdrs and only inserting regular headers that were not already present, giving unredirected headers priority. This contradicts get_header() and header_items(), both of which give regular headers the higher priority. Fix by unconditionally updating with req.headers so that a header set via add_header() always overrides one set via add_unredirected_header(). --------- (cherry picked from commit 6d38668) Co-authored-by: CHINMAY <89741289+Das-Chinmay@users.noreply.github.com> Co-authored-by: Senthil Kumaran <senthil@python.org>
…rected … (GH-146506) (#153377) gh-47005: fix do_open() to let regular headers override unredirected … (GH-146506) AbstractHTTPHandler.do_open() was building the request header dict by starting with unredirected_hdrs and only inserting regular headers that were not already present, giving unredirected headers priority. This contradicts get_header() and header_items(), both of which give regular headers the higher priority. Fix by unconditionally updating with req.headers so that a header set via add_header() always overrides one set via add_unredirected_header(). --------- (cherry picked from commit 6d38668) Co-authored-by: CHINMAY <89741289+Das-Chinmay@users.noreply.github.com> Co-authored-by: Senthil Kumaran <senthil@python.org>
…rected … (GH-146506) (#153378) gh-47005: fix do_open() to let regular headers override unredirected … (GH-146506) AbstractHTTPHandler.do_open() was building the request header dict by starting with unredirected_hdrs and only inserting regular headers that were not already present, giving unredirected headers priority. This contradicts get_header() and header_items(), both of which give regular headers the higher priority. Fix by unconditionally updating with req.headers so that a header set via add_header() always overrides one set via add_unredirected_header(). --------- (cherry picked from commit 6d38668) Co-authored-by: CHINMAY <89741289+Das-Chinmay@users.noreply.github.com> Co-authored-by: Senthil Kumaran <senthil@python.org>
2 participants
AbstractHTTPHandler.do_open()builds the outgoing header dict by startingwith
unredirected_hdrsand only merging regular headers that are notalready present — giving unredirected headers the higher priority.
This contradicts both
get_header()(checksself.headersfirst) andheader_items()({**unredirected_hdrs, **headers}), which both giveregular headers priority. The result: a header set via
add_header()issilently ignored if the same name was already set via
add_unredirected_header().Fix: replace the conditional merge with an unconditional
headers.update(req.headers).Fixes gh-47005