nsILoadInfo::AppendRedirectHistoryEntry should take an nsIChannel and construct the entry itself
Categories
(Core :: DOM: Navigation, defect, P2)
Tracking
()
People
(Reporter: Gijs, Assigned: Gijs)
Details
Attachments
(1 file)
There are 4 non-test consumers of appendRedirectHistoryEntry in tree: https://searchfox.org/mozilla-central/search?q=appendredirecthistoryentry
netwerk/base/nsBaseChannel.cpp
93 newLoadInfo->AppendRedirectHistoryEntry(entry, isInternalRedirect); // found in nsBaseChannel::Redirect
netwerk/ipc/DocumentLoadListener.cpp
1409 redirectLoadInfo->AppendRedirectHistoryEntry(entry, true); // found in mozilla::net::DocumentLoadListener::SerializeRedirectData
netwerk/protocol/http/HttpBaseChannel.cpp
3952 newLoadInfo->AppendRedirectHistoryEntry(entry, isInternalRedirect); // found in mozilla::net::HttpBaseChannel::CloneLoadInfoForRedirect
netwerk/protocol/http/HttpChannelChild.cpp
1550 mLoadInfo->AppendRedirectHistoryEntry(entry, false); // found in mozilla::net::HttpChannelChild::CleanupRedirectingChannel
Looking at these consumers:
https://searchfox.org/mozilla-central/rev/eecd2dd4ba630bea4ce103623a4bfb398065b64b/netwerk/base/nsBaseChannel.cpp#93
https://searchfox.org/mozilla-central/rev/eecd2dd4ba630bea4ce103623a4bfb398065b64b/netwerk/ipc/DocumentLoadListener.cpp#1409
https://searchfox.org/mozilla-central/rev/eecd2dd4ba630bea4ce103623a4bfb398065b64b/netwerk/protocol/http/HttpBaseChannel.cpp#3952
https://searchfox.org/mozilla-central/rev/eecd2dd4ba630bea4ce103623a4bfb398065b64b/netwerk/protocol/http/HttpChannelChild.cpp#1550
what's striking is that they all do the same thing to determine the URL to add: they ask the script security manager for the channel's URI principal (the http channels use the channel's own GetURIPrincipal helper for this, which just defers to the script security manager anyway, and has no other callers).
The ones in http code also both check the computed referrer and remote address off the channel.
So ISTM we could just push the construction of the entry into AppendRedirectHistoryEntry itself, and have it take an nsIChannel as an argument. As a bonus, this makes it easier to call this method from JS code, because it is likely to have a channel reference, but cannot itself construct the native redirect history entry objects, and any JS implementation is not going to be threadsafe.
I have a patch for this.
|
Assignee | |
Comment 1•4 months ago
|
||
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/c04e9ee53e53 stop duplicating append redirect history entry logic everywhere, r=ckerschb,necko-reviewers,valentin
|
||
Comment 3•4 months ago
|
||
Changing severity to S3 because of triaging a bug which has a patch which just landed to autoland.
|
||
Comment 4•4 months ago
|
||
| bugherder | ||
|
||
Comment 5•3 months ago
|
||
The patch landed in nightly and beta is affected.
:Gijs, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.
For more information, please visit auto_nag documentation.
|
||
Updated•3 months ago
|
Description
•