Bug 179527

Summary: Allow XHR to override the User-Agent header.
Product: WebKit Reporter: Ms2ger (he/him; ⌚ UTC+1/+2) <Ms2ger>
Component: Page LoadingAssignee: 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 Flags
Patch
none
Archive of layout-test-results from ews101 for mac-elcapitan
none
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
none
Archive of layout-test-results from ews126 for ios-simulator-wk2
none
Archive of layout-test-results from ews114 for mac-elcapitan
none
Patch none

Description Ms2ger (he/him; ⌚ UTC+1/+2) 2017-11-10 06:42:14 PST
.
Comment 1 Ms2ger (he/him; ⌚ UTC+1/+2) 2017-11-10 06:47:14 PST
Created attachment 326575 [details]
Patch
Comment 2 Chris Dumez 2017-11-10 06:55:06 PST
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 3 Build Bot 2017-11-10 07:42:59 PST
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
Comment 4 Build Bot 2017-11-10 07:43:00 PST
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 5 Build Bot 2017-11-10 07:55:55 PST
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
Comment 6 Build Bot 2017-11-10 07:55:57 PST
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
Comment 7 Chris Dumez 2017-11-10 07:57:46 PST
(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)
Comment 8 Ms2ger (he/him; ⌚ UTC+1/+2) 2017-11-10 07:59:23 PST
(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.
Comment 9 Chris Dumez 2017-11-10 08:02:33 PST
(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 10 Build Bot 2017-11-10 08:10:29 PST
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
Comment 11 Build Bot 2017-11-10 08:10:30 PST
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 12 Build Bot 2017-11-10 08:11:53 PST
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
Comment 13 Build Bot 2017-11-10 08:11:54 PST
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
Comment 14 Ms2ger (he/him; ⌚ UTC+1/+2) 2017-11-10 08:19:38 PST
Created attachment 326587 [details]
Patch
Comment 15 WebKit Commit Bot 2017-11-10 08:41:07 PST
Comment on attachment 326587 [details]
Patch

Clearing flags on attachment: 326587

Committed r224684: <https://trac.webkit.org/changeset/224684>
Comment 16 WebKit Commit Bot 2017-11-10 08:41:09 PST
All reviewed patches have been landed.  Closing bug.
Comment 17 Radar WebKit Bug Importer 2017-11-15 09:36:37 PST
<rdar://problem/35562062>
Comment 18 Brady Eidson 2018-05-29 17:13:40 PDT
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?
Comment 19 Ms2ger (he/him; ⌚ UTC+1/+2) 2018-05-30 23:23:37 PDT
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.
Comment 20 Sam Sneddon [:gsnedders] 2022-12-14 16:14:41 PST
*** Bug 168598 has been marked as a duplicate of this bug. ***