Bug 179527 – Allow XHR to override the User-Agent header.

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Ms2ger (he/him; ⌚ UTC+1/+2)]

.

[Ms2ger (he/him; ⌚ UTC+1/+2)]

Created attachment 326575 [details]
Patch

[Chris Dumez]

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.

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Chris Dumez]

(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)

[Ms2ger (he/him; ⌚ UTC+1/+2)]

(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.

[Chris Dumez]

(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.

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Ms2ger (he/him; ⌚ UTC+1/+2)]

Created attachment 326587 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 326587 [details]
Patch

Clearing flags on attachment: 326587

Committed r224684: <https://trac.webkit.org/changeset/224684>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35562062>

[Brady Eidson]

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?

[Ms2ger (he/him; ⌚ UTC+1/+2)]

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.

[Sam Sneddon [:gsnedders]]

*** Bug 168598 has been marked as a duplicate of this bug. ***