Summary: | Allow XHR to override the User-Agent header. | ||
---|---|---|---|
Product: | WebKit | Reporter: | Ms2ger (he/him; ⌚ UTC+1/+2) <Ms2ger> |
Component: | Page Loading | Assignee: | Ms2ger (he/him; ⌚ UTC+1/+2) <Ms2ger> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | annevk, beidson, buildbot, cdumez, commit-queue, dbates, japhet, rniwa, webkit-bug-importer |
Priority: | P2 | Keywords: | InRadar |
Version: | WebKit Nightly Build | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Attachments: |
Description
Ms2ger (he/him; ⌚ UTC+1/+2)
2017-11-10 06:42:14 PST
Created attachment 326575 [details]
Patch
Comment on attachment 326575 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=326575&action=review > Source/WebCore/loader/FrameLoader.cpp:3272 > void FrameLoader::applyUserAgent(ResourceRequest& request) I think we should add IfNecessay suffix given the new implementation. Comment on attachment 326575 [details] Patch Attachment 326575 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/5176775 New failing tests: http/tests/xmlhttprequest/check-combining-headers.html Created attachment 326577 [details]
Archive of layout-test-results from ews101 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 326575 [details] Patch Attachment 326575 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/5176789 New failing tests: http/tests/xmlhttprequest/check-combining-headers.html Created attachment 326583 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
(In reply to Build Bot from comment #5) > Comment on attachment 326575 [details] > Patch > > Attachment 326575 [details] did not pass mac-wk2-ews (mac-wk2): > Output: http://webkit-queues.webkit.org/results/5176789 > > New failing tests: > http/tests/xmlhttprequest/check-combining-headers.html Looks like a progression, please make sure you rebaseline this test before landing: --- /Volumes/Data/EWS/WebKit/WebKitBuild/Release/layout-test-results/http/tests/xmlhttprequest/check-combining-headers-expected.txt +++ /Volumes/Data/EWS/WebKit/WebKitBuild/Release/layout-test-results/http/tests/xmlhttprequest/check-combining-headers-actual.txt @@ -1,7 +1,7 @@ PASS XMLHttpRequest: setRequestHeader() - combining headers (Authorization) PASS XMLHttpRequest: setRequestHeader() - combining headers (Pragma) -FAIL XMLHttpRequest: setRequestHeader() - combining headers (User-Agent) assert_true: Combined header value should be t1, t2 expected true got false +PASS XMLHttpRequest: setRequestHeader() - combining headers (User-Agent) PASS XMLHttpRequest: setRequestHeader() - combining headers (Content-Transfer-Encoding) PASS XMLHttpRequest: setRequestHeader() - combining headers (Content-Type) PASS XMLHttpRequest: setRequestHeader() - combining headers (Overwrite) (In reply to Chris Dumez from comment #7) > (In reply to Build Bot from comment #5) > > Comment on attachment 326575 [details] > > Patch > > > > Attachment 326575 [details] did not pass mac-wk2-ews (mac-wk2): > > Output: http://webkit-queues.webkit.org/results/5176789 > > > > New failing tests: > > http/tests/xmlhttprequest/check-combining-headers.html > > Looks like a progression, please make sure you rebaseline this test before > landing: > --- > /Volumes/Data/EWS/WebKit/WebKitBuild/Release/layout-test-results/http/tests/ > xmlhttprequest/check-combining-headers-expected.txt > +++ > /Volumes/Data/EWS/WebKit/WebKitBuild/Release/layout-test-results/http/tests/ > xmlhttprequest/check-combining-headers-actual.txt > @@ -1,7 +1,7 @@ > > PASS XMLHttpRequest: setRequestHeader() - combining headers (Authorization) > PASS XMLHttpRequest: setRequestHeader() - combining headers (Pragma) > -FAIL XMLHttpRequest: setRequestHeader() - combining headers (User-Agent) > assert_true: Combined header value should be t1, t2 expected true got false > +PASS XMLHttpRequest: setRequestHeader() - combining headers (User-Agent) > PASS XMLHttpRequest: setRequestHeader() - combining headers > (Content-Transfer-Encoding) > PASS XMLHttpRequest: setRequestHeader() - combining headers (Content-Type) > PASS XMLHttpRequest: setRequestHeader() - combining headers (Overwrite) I'm just going to remove the test, it's an old fork of the test I'm already rebaselining. (In reply to Ms2ger from comment #8) > (In reply to Chris Dumez from comment #7) > > (In reply to Build Bot from comment #5) > > > Comment on attachment 326575 [details] > > > Patch > > > > > > Attachment 326575 [details] did not pass mac-wk2-ews (mac-wk2): > > > Output: http://webkit-queues.webkit.org/results/5176789 > > > > > > New failing tests: > > > http/tests/xmlhttprequest/check-combining-headers.html > > > > Looks like a progression, please make sure you rebaseline this test before > > landing: > > --- > > /Volumes/Data/EWS/WebKit/WebKitBuild/Release/layout-test-results/http/tests/ > > xmlhttprequest/check-combining-headers-expected.txt > > +++ > > /Volumes/Data/EWS/WebKit/WebKitBuild/Release/layout-test-results/http/tests/ > > xmlhttprequest/check-combining-headers-actual.txt > > @@ -1,7 +1,7 @@ > > > > PASS XMLHttpRequest: setRequestHeader() - combining headers (Authorization) > > PASS XMLHttpRequest: setRequestHeader() - combining headers (Pragma) > > -FAIL XMLHttpRequest: setRequestHeader() - combining headers (User-Agent) > > assert_true: Combined header value should be t1, t2 expected true got false > > +PASS XMLHttpRequest: setRequestHeader() - combining headers (User-Agent) > > PASS XMLHttpRequest: setRequestHeader() - combining headers > > (Content-Transfer-Encoding) > > PASS XMLHttpRequest: setRequestHeader() - combining headers (Content-Type) > > PASS XMLHttpRequest: setRequestHeader() - combining headers (Overwrite) > > I'm just going to remove the test, it's an old fork of the test I'm already > rebaselining. SGTM. Comment on attachment 326575 [details] Patch Attachment 326575 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5176804 New failing tests: http/tests/xmlhttprequest/check-combining-headers.html Created attachment 326584 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 326575 [details] Patch Attachment 326575 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/5176817 New failing tests: http/tests/xmlhttprequest/check-combining-headers.html Created attachment 326585 [details]
Archive of layout-test-results from ews114 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 326587 [details]
Patch
Comment on attachment 326587 [details] Patch Clearing flags on attachment: 326587 Committed r224684: <https://trac.webkit.org/changeset/224684> All reviewed patches have been landed. Closing bug. This patch broke changing the User Agent string then reloading the Document, because the Document's request already has a UA and therefore doesn't get the new one: void FrameLoader::applyUserAgentIfNeeded(ResourceRequest& request) { if (!request.hasHTTPHeaderField(HTTPHeaderName::UserAgent)) { String userAgent = this->userAgent(request.url()); ASSERT(!userAgent.isNull()); request.setHTTPUserAgent(userAgent); } } Note - That function is hit for EVERY single subresource. This bug was purported to be about allowing XHR to override the UA string. Is there any reason it changed the behavior of EVERY subresource, instead? So reloading reuses the ResourceRequest somehow? That seems unfortunate. Anyway, I don't know what other approach I could use to fix this bug, so feel free to revert, I suppose. *** Bug 168598 has been marked as a duplicate of this bug. *** |