Bug 191858 – [GTK] Unexpected User-Agent on redirect

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Jim Mason]

WebKitGTK: 2.22.3
Epiphany:  3.30.2

It seems the User-Agent quirk mechanism is using the URL of a request to decide which quirk to send, not only for the original request, but also for any associated redirects.  As a result, in some situations, sites which should not receive a UA quirk receive it, and sites which should receive one do not.

This manifests itself readily during Google OAuth, in the case when Google challenges the user for credentials.

Below is an extract from a service provider web server log for a user who is attempting to login via Google SSO.  (I have suppressed the client IP address and truncated the query strings for readability):

> x.x.x.x - - [16/Nov/2018:14:37:31 +0100] "GET /kzsu/ssoLogin.php HTTP/1.1" 307 - "https://zookeeper.ibinx.com/kzsu/?" "Mozilla/5.0 (X11; SunOS i86pc) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/11.0 Safari/605.1.15 Epiphany/605.1.15"
> x.x.x.x - - [16/Nov/2018:14:37:36 +0100] "GET /kzsu/ssoLogin.php?state=0ea73ed9f8965966b201f5b70355795c... HTTP/1.1" 307 - "https://accounts.google.com/ServiceLogin?continue=https..." "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/11.0 Safari/605.1.15"
> x.x.x.x - - [16/Nov/2018:14:37:37 +0100] "GET /kzsu/?action=loginValidate&session=c8c39681f2db99f849998349... HTTP/1.1" 200 1665 "https://accounts.google.com/ServiceLogin?continue=https..." "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/11.0 Safari/605.1.15"


The first request above results in a redirect to Google.  The second request is the redirect from Google, which in turn results in an intra-site redirect (the third request).  We can see that Google challenged the user because the value of the Referer changed.

Observed behaviour:  UA on the second and third request is the `LinuxDesktopPlatform` quirk.

Expected behaviour: UA on the second and third request should match the first one (i.e., the normal UA)


As a counter example, if the user is already logged into Google, there is no challenge at Google, only a redirect:

> x.x.x.x - - [16/Nov/2018:16:51:04 +0100] "GET /kzsu/ssoLogin.php HTTP/1.1" 307 - "https://zookeeper.ibinx.com/kzsu/?session=" "Mozilla/5.0 (X11; SunOS i86pc) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/11.0 Safari/605.1.15 Epiphany/605.1.15"
> x.x.x.x - - [16/Nov/2018:16:51:05 +0100] "GET /kzsu/ssoLogin.php?state=6efaaea080fb97213d27d41266a41347... HTTP/1.1" 307 - "https://zookeeper.ibinx.com/kzsu/?session=" "Mozilla/5.0 (X11; SunOS i86pc) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/11.0 Safari/605.1.15 Epiphany/605.1.15"
> x.x.x.x - - [16/Nov/2018:16:51:05 +0100] "GET /kzsu/?action=loginValidate&session=12c17d8a4a222f23b8299942cee5347b HTTP/1.1" 200 1665 "https://zookeeper.ibinx.com/kzsu/?session=" "Mozilla/5.0 (X11; SunOS i86pc) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/11.0 Safari/605.1.15 Epiphany/605.1.15"


Above are the same three requests as the first log extract, except in this case, the Referer on the second and third requests is the original URL.  This indicates Google did not challenge the user for credentials.

In this case, the UA is correct for all three requests (though likely, the real UA and not the quirk is presented to Google).


Expected behaviour:  In all cases, the UA quirk, if any, that is presented to a site should be determined by that specific target URL, and not the original URL from which the request was redirected.

[Michael Catanzaro]

(In reply to Jim Mason from comment #0) 
> Observed behaviour:  UA on the second and third request is the
> `LinuxDesktopPlatform` quirk.
> 
> Expected behaviour: UA on the second and third request should match the
> first one (i.e., the normal UA)

Just to confirm: the second and third requests are requests to your server, which had been first made to Google and then later redirected?

I think I agree. The quirk should be changed to match the new host after the redirect.

[Jim Mason]

(In reply to Michael Catanzaro from comment #1)
> Just to confirm: the second and third requests are requests to your server,
> which had been first made to Google and then later redirected?

That is correct.  However, in the first log extract, Google initiates the redirect (it is not merely a redirect of my initial request).  It is only in this case that my server sees the UA quirk intended for Google.

[Carlos Garcia Campos]

Created attachment 407598 [details]
Patch

[Michael Catanzaro]

Comment on attachment 407598 [details]
Patch

I had suspected this wouldn't be simple.... Anyway, LGTM. I trust you'll find an owner for formal review.

[Carlos Garcia Campos]

(In reply to Michael Catanzaro from comment #4)
> Comment on attachment 407598 [details]
> Patch
> 
> I had suspected this wouldn't be simple.... Anyway, LGTM. I trust you'll
> find an owner for formal review.

It was indeed simple, the pain is sending the quirks setting to the network process to be applied in every request.

[youenn fablet]

Comment on attachment 407598 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=407598&action=review

> Source/WebKit/ChangeLog:9
> +        request when needsSiteSpecificQuirks setting is enabled.

It seems tests are missing here. Can we write some?
Also, it seems the patch adds a generic boolean but setting the user agent value is platform specific.
Currently, the work is mostly done in WebProcess with platform agnostic code, computation of the user agent value being done by WebPage.
How is the computation of this value supposed to work in NetworkProcess?
After the patch, GTK port will compute value in NetworkProcess while other ports will continue doing this in WebProcess.

[Adrian Perez]

Comment on attachment 407598 [details]
Patch

So this is conceptually simple, but it's amazing how many files
it needs to touch to make the setting trickle down to the place
where it's used, wow!

Should we get someone from Apple to rubber-stamp this change as
well, given that it touches common code?

[Carlos Garcia Campos]

(In reply to youenn fablet from comment #6)
> Comment on attachment 407598 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=407598&action=review
> 
> > Source/WebKit/ChangeLog:9
> > +        request when needsSiteSpecificQuirks setting is enabled.
> 
> It seems tests are missing here. Can we write some?

It's not easy to test I'm afraid. The user agent changes depending on the site, only when site specific quirks setting is enabled. In tests the host is always localhost. We would need a way to set quirks only used for testing for localhost (and not for 127.0.0.1, for example).

> Also, it seems the patch adds a generic boolean but setting the user agent
> value is platform specific.

Because the boolean is not specific to the user agent, it's the site specific quirks setting, but it's true it's currently used only by GTK port to set the user agent.

> Currently, the work is mostly done in WebProcess with platform agnostic
> code, computation of the user agent value being done by WebPage.

There's WebPage::platformUserAgent() that right now it's only implemented by iOS and Playstation ports. In the case of the iOS port, the change in the user agent doesn't really depend on the URL, but I think playstation also needs to be fixed, moving the user agent quirks to the network process. 

> How is the computation of this value supposed to work in NetworkProcess?

We can't just use a fixed user agent for all websites like major browsers do, because we get messages of unsupported browser, or features disabled, or things simply don't work. So, we need to pretend to be other browsers in some cases depending on the website. That's what we do in WebPage::platformUserAgent(). The problem is that after a redirection, we copy the user agent from the previous request, but the redirected URL might require a different user agent to work. You can see an¡ example of how this can break things in https://bugs.webkit.org/show_bug.cgi?id=215845#c3. So, in case of redirections we need to apply the user agent quirks again, but we are in the network process now. So, this patch moves the user agent quirks to the network process ensuring they are always applied, for the initial request, for downloads, and for the new request after a redirection.

> After the patch, GTK port will compute value in NetworkProcess while other
> ports will continue doing this in WebProcess.

Not exactly, most of the ports don't implement WebPage::platformUserAgent(), which means the configured user agent in settings is used, or the standard user agent defined for the port. So, we are receiving that user agent in the network process, like all other ports. But we need to ignore that user agent in some cases and apply a different one, that's what we do now in the network process, and we always did that differently even in the web process, because Safari doesn't need to pretend to be other browser.

[Carlos Garcia Campos]

(In reply to Adrian Perez from comment #7)
> Comment on attachment 407598 [details]
> Patch
> 
> So this is conceptually simple, but it's amazing how many files
> it needs to touch to make the setting trickle down to the place
> where it's used, wow!
> 
> Should we get someone from Apple to rubber-stamp this change as
> well, given that it touches common code?

Yes, it needs to be approved by an owner

[youenn fablet]

(In reply to Carlos Garcia Campos from comment #8)
> (In reply to youenn fablet from comment #6)
> > Comment on attachment 407598 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=407598&action=review
> > 
> > > Source/WebKit/ChangeLog:9
> > > +        request when needsSiteSpecificQuirks setting is enabled.
> > 
> > It seems tests are missing here. Can we write some?
> 
> It's not easy to test I'm afraid. The user agent changes depending on the
> site, only when site specific quirks setting is enabled. In tests the host
> is always localhost. We would need a way to set quirks only used for testing
> for localhost (and not for 127.0.0.1, for example).

Seems fine.
We can also think of using API tests as well.
Quirks are currently based on URL, maybe we can add schemes/port specific handling as part of testing.

> 
> > Also, it seems the patch adds a generic boolean but setting the user agent
> > value is platform specific.
> 
> Because the boolean is not specific to the user agent, it's the site
> specific quirks setting, but it's true it's currently used only by GTK port
> to set the user agent.
> 
> > Currently, the work is mostly done in WebProcess with platform agnostic
> > code, computation of the user agent value being done by WebPage.
> 
> There's WebPage::platformUserAgent() that right now it's only implemented by
> iOS and Playstation ports. In the case of the iOS port, the change in the
> user agent doesn't really depend on the URL, but I think playstation also
> needs to be fixed, moving the user agent quirks to the network process. 
> 
> > How is the computation of this value supposed to work in NetworkProcess?
> 
> We can't just use a fixed user agent for all websites like major browsers
> do, because we get messages of unsupported browser, or features disabled, or
> things simply don't work. So, we need to pretend to be other browsers in
> some cases depending on the website. That's what we do in
> WebPage::platformUserAgent(). The problem is that after a redirection, we
> copy the user agent from the previous request, but the redirected URL might
> require a different user agent to work. You can see an¡ example of how this
> can break things in https://bugs.webkit.org/show_bug.cgi?id=215845#c3. So,
> in case of redirections we need to apply the user agent quirks again, but we
> are in the network process now. So, this patch moves the user agent quirks
> to the network process ensuring they are always applied, for the initial
> request, for downloads, and for the new request after a redirection.

Which requests are triggering the issue?
Navigation loads should probably be fine since WebProcess is dealing with this issue properly.
That would leave sub resources and downloads.
Is the main issue downloads?

[Carlos Garcia Campos]

(In reply to youenn fablet from comment #10)
> (In reply to Carlos Garcia Campos from comment #8)
> > (In reply to youenn fablet from comment #6)
> > > Comment on attachment 407598 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=407598&action=review
> > > 
> > > > Source/WebKit/ChangeLog:9
> > > > +        request when needsSiteSpecificQuirks setting is enabled.
> > > 
> > > It seems tests are missing here. Can we write some?
> > 
> > It's not easy to test I'm afraid. The user agent changes depending on the
> > site, only when site specific quirks setting is enabled. In tests the host
> > is always localhost. We would need a way to set quirks only used for testing
> > for localhost (and not for 127.0.0.1, for example).
> 
> Seems fine.
> We can also think of using API tests as well.

Even more complicated, in the case of layout tests we have the internals api that we can use to add fake quirks or something like that, but that's more complicated from api tests.

> Quirks are currently based on URL, maybe we can add schemes/port specific
> handling as part of testing.

Well we receive the URL but end up using the hostname to determine the website, so using localhost/127.0.0.1 would work.

> > 
> > > Also, it seems the patch adds a generic boolean but setting the user agent
> > > value is platform specific.
> > 
> > Because the boolean is not specific to the user agent, it's the site
> > specific quirks setting, but it's true it's currently used only by GTK port
> > to set the user agent.
> > 
> > > Currently, the work is mostly done in WebProcess with platform agnostic
> > > code, computation of the user agent value being done by WebPage.
> > 
> > There's WebPage::platformUserAgent() that right now it's only implemented by
> > iOS and Playstation ports. In the case of the iOS port, the change in the
> > user agent doesn't really depend on the URL, but I think playstation also
> > needs to be fixed, moving the user agent quirks to the network process. 
> > 
> > > How is the computation of this value supposed to work in NetworkProcess?
> > 
> > We can't just use a fixed user agent for all websites like major browsers
> > do, because we get messages of unsupported browser, or features disabled, or
> > things simply don't work. So, we need to pretend to be other browsers in
> > some cases depending on the website. That's what we do in
> > WebPage::platformUserAgent(). The problem is that after a redirection, we
> > copy the user agent from the previous request, but the redirected URL might
> > require a different user agent to work. You can see an¡ example of how this
> > can break things in https://bugs.webkit.org/show_bug.cgi?id=215845#c3. So,
> > in case of redirections we need to apply the user agent quirks again, but we
> > are in the network process now. So, this patch moves the user agent quirks
> > to the network process ensuring they are always applied, for the initial
> > request, for downloads, and for the new request after a redirection.
> 
> Which requests are triggering the issue?

The request started after a redirection, because we copy the user agent from the previous request. We need to apply the quirks on the redirected url again. See https://bugs.webkit.org/show_bug.cgi?id=215845#c3.

> Navigation loads should probably be fine since WebProcess is dealing with
> this issue properly.

Right.

> That would leave sub resources and downloads.
> Is the main issue downloads?

The quirks are applied fine in the web process for subresources as well, frame loader asks the client that ends up calling WebPage::platformUserAgent().

Downloads started by the web proccess when a load is converted to a download are not a problem either, the quirks are applied correctly. Downloads started by the web process pool don't get the quirks applied.

I thought it could be even more confusing to apply the quirks both in the web and network processes depending on the request. Also, since we don't know if there will be a redirect, we always have to send the needs quirks setting to the network process for every request.

[Carlos Garcia Campos]

The needs quirk parameter is similar to isNavigatingToAppBoundDomain, in the sense that it's currently only used by one platform, but making it platform specific would make the code a lot more complicated with more platform ifdefs.

[youenn fablet]

I am still unclear about which load is triggering the issue.
I understand this only happens in case of redirection but for which load.

In https://bugs.webkit.org/show_bug.cgi?id=215845#c3, the initial request seems to be a navigation request. I would believe navigation request redirection to be properly handled by WebProcess. If that is not the case, there may be an issue in that handling. For instance, it seems that https://bugs.webkit.org/show_bug.cgi?id=215845#c3 is fixed by disabling PSON.

I can see that subresource redirections could have this issue, as well as downloads after redirections. Is Epiphany actually affected by such issues for these loads?

[Carlos Garcia Campos]

Ok, I see your point now, before starting the redirection we go back to the web process and willSendRequest is called for the new request, so it should be possible to apply the quirks there. I'm going to check that, it would simplify everything.

[Carlos Garcia Campos]

Comment on attachment 407598 [details]
Patch

I can confirm that simply clearing the user agent on the new request after a redirection fixes the issue, because the web process applies a new one. I have to check downloads now.

[Carlos Garcia Campos]

Created attachment 407676 [details]
Patch

[EWS Watchlist]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Michael Catanzaro]

Comment on attachment 407676 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=407676&action=review

Much nicer!

> Source/WebKit/UIProcess/API/glib/WebKitDownloadClient.cpp:60
> +    void willSendRequest(DownloadProxy& downloadProxy, ResourceRequest&& request, const ResourceResponse&, CompletionHandler<void(ResourceRequest&&)>&& completionHandler) override
> +    {
> +        if (!request.hasHTTPHeaderField(HTTPHeaderName::UserAgent)) {
> +            GRefPtr<WebKitDownload> download = webkitWebContextGetOrCreateDownload(&downloadProxy);
> +            auto* webView = webkit_download_get_web_view(download.get());
> +            request.setHTTPUserAgent(webView ? webkitWebViewGetPage(webView).userAgentForURL(request.url()) : WebPageProxy::standardUserAgent());
> +        }
> +
> +        completionHandler(WTFMove(request));
> +    }

I would do it in a separate bug (with r=me), since it's really an additional fix on top of the primary fix. But whatever.

> Source/WebKit/UIProcess/playstation/WebPageProxyPlayStation.cpp:49
> +String WebPageProxy::userAgentForURL(const URL&)
> +{
> +    return userAgent();
> +}

I understand from your previous comments that PlayStation port does have user agent quirks that depend on URL, and you just haven't implemented it here, so action is required for PlayStation port? Is that right, or do I misunderstand?

[Carlos Garcia Campos]

(In reply to Michael Catanzaro from comment #18)
> Comment on attachment 407676 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=407676&action=review
> 
> Much nicer!
> 
> > Source/WebKit/UIProcess/API/glib/WebKitDownloadClient.cpp:60
> > +    void willSendRequest(DownloadProxy& downloadProxy, ResourceRequest&& request, const ResourceResponse&, CompletionHandler<void(ResourceRequest&&)>&& completionHandler) override
> > +    {
> > +        if (!request.hasHTTPHeaderField(HTTPHeaderName::UserAgent)) {
> > +            GRefPtr<WebKitDownload> download = webkitWebContextGetOrCreateDownload(&downloadProxy);
> > +            auto* webView = webkit_download_get_web_view(download.get());
> > +            request.setHTTPUserAgent(webView ? webkitWebViewGetPage(webView).userAgentForURL(request.url()) : WebPageProxy::standardUserAgent());
> > +        }
> > +
> > +        completionHandler(WTFMove(request));
> > +    }
> 
> I would do it in a separate bug (with r=me), since it's really an additional
> fix on top of the primary fix. But whatever.

Not exactly, because now we are clearing the user agent on redirect, so before this patch we received the previous user agent here, now it's empty, if we don't fill it here we will end up with no user agent in the request.

> > Source/WebKit/UIProcess/playstation/WebPageProxyPlayStation.cpp:49
> > +String WebPageProxy::userAgentForURL(const URL&)
> > +{
> > +    return userAgent();
> > +}
> 
> I understand from your previous comments that PlayStation port does have
> user agent quirks that depend on URL, and you just haven't implemented it
> here, so action is required for PlayStation port? Is that right, or do I
> misunderstand?

Right, that's why I added Fujii to the CC.

[youenn fablet]

Much nicer indeed.

> Not exactly, because now we are clearing the user agent on redirect, so
> before this patch we received the previous user agent here, now it's empty,
> if we don't fill it here we will end up with no user agent in the request.

What is the impact of clearing user agent for subresource loads?

Adding tests would be valuable for all these loads.

[Carlos Garcia Campos]

(In reply to youenn fablet from comment #20)
> Much nicer indeed.
> 
> > Not exactly, because now we are clearing the user agent on redirect, so
> > before this patch we received the previous user agent here, now it's empty,
> > if we don't fill it here we will end up with no user agent in the request.
> 
> What is the impact of clearing user agent for subresource loads?

In the case of subresource loads the FrameLoader computes the user agent again if the header is not present, like for the main resource.

> Adding tests would be valuable for all these loads.

Right, but again, not easy to test :-(

[youenn fablet]

> > What is the impact of clearing user agent for subresource loads?
> 
> In the case of subresource loads the FrameLoader computes the user agent
> again if the header is not present, like for the main resource.

In general, navigation loads are not handling redirections so it is fine to hope that a new user agent will be created.

For subresource loads, redirections are expected to be followed and the current model is to keep the same user agent in that case (would need to be tested though if we do not have such tests).

It would be interesting to see what would happen for ping loads/fetch keep alive for instance with your change.

[Adrian Perez]

Comment on attachment 407676 [details]
Patch

The latest version of the patch is indeed much nicer and simpler, r=me for the GTK/WPE parts.

[Jim Mason]

Thanks Carlos!

I can confirm that the new patch yields the expected User Agents in the Google SSO OAuth cases reported in the description above.

[Carlos Garcia Campos]

(In reply to youenn fablet from comment #22)
> > > What is the impact of clearing user agent for subresource loads?
> > 
> > In the case of subresource loads the FrameLoader computes the user agent
> > again if the header is not present, like for the main resource.
> 
> In general, navigation loads are not handling redirections so it is fine to
> hope that a new user agent will be created.

You mean because of PSON?

> For subresource loads, redirections are expected to be followed and the
> current model is to keep the same user agent in that case (would need to be
> tested though if we do not have such tests).

Testing quirks is not easy, but at least now we can test that user-agent is preserved after redirects.

> It would be interesting to see what would happen for ping loads/fetch keep
> alive for instance with your change.

hmm, I think we could lose the user agent for ping loads.

[youenn fablet]

> > In general, navigation loads are not handling redirections so it is fine to
> > hope that a new user agent will be created.
> 
> You mean because of PSON?

Fetch spec is defining https://fetch.spec.whatwg.org/#requestredirect.
All navigation loads use 'manual'.
In that sense, user agent is making a navigation request and getting a response.
If the response is a redirection, it will do another navigation load, with a brand new request, possibly with a different user agent.

It would be interesting to understand why gtk is not able to override the User-Agent for navigation redirections (probably it is preserving the user agent and will not override it since present) while MacOS has not that issue.

For subresource loads, redirect mode is usually "follow".
https://fetch.spec.whatwg.org/#concept-http-redirect-fetch will most probably preserve the user-agent header and fetch spec will not override it.

[Carlos Garcia Campos]

(In reply to youenn fablet from comment #26)
> > > In general, navigation loads are not handling redirections so it is fine to
> > > hope that a new user agent will be created.
> > 
> > You mean because of PSON?
> 
> Fetch spec is defining https://fetch.spec.whatwg.org/#requestredirect.
> All navigation loads use 'manual'.
> In that sense, user agent is making a navigation request and getting a
> response.
> If the response is a redirection, it will do another navigation load, with a
> brand new request, possibly with a different user agent.
> 
> It would be interesting to understand why gtk is not able to override the
> User-Agent for navigation redirections (probably it is preserving the user
> agent and will not override it since present) while MacOS has not that issue.

Yes, because the user agent is present in the redirection request when it reaches the web process. I guess MacOS doesn't have the issue because the header is cleared or because the previous one is reused, which is ok when you don't need quirks.

> For subresource loads, redirect mode is usually "follow".
> https://fetch.spec.whatwg.org/#concept-http-redirect-fetch will most
> probably preserve the user-agent header and fetch spec will not override it.

Ok, so you are talking about fetch requests specifically?

[youenn fablet]

> > For subresource loads, redirect mode is usually "follow".
> > https://fetch.spec.whatwg.org/#concept-http-redirect-fetch will most
> > probably preserve the user-agent header and fetch spec will not override it.
> 
> Ok, so you are talking about fetch requests specifically?

fetch is the algorithm that implement loading of resources (be they navigation, fetch, xhr, css, js...)

[youenn fablet]

> Yes, because the user agent is present in the redirection request when it
> reaches the web process. I guess MacOS doesn't have the issue because the
> header is cleared or because the previous one is reused, which is ok when
> you don't need quirks.

User-Agent header does not seem to be cleared by CFNetwork at least.

[Carlos Garcia Campos]

Created attachment 407756 [details]
Patch

Updated patch to include unit tests.

[Carlos Garcia Campos]

Created attachment 407757 [details]
Patch

Updated to patch to ensure we always have a user agent after redirections. It saves the previous one before clearing it, and after the client handles the redirection, if we still don'tm have a user agent, the previous one is added.

[Jim Mason]

(In reply to Carlos Garcia Campos from comment #31)
> Created attachment 407757 [details]
> Patch
> 
> Updated to patch to ensure we always have a user agent after redirections.
> It saves the previous one before clearing it, and after the client handles
> the redirection, if we still don'tm have a user agent, the previous one is
> added.

I can confirm that the latest patch yields the expected User Agents in the Google OAuth use cases above.

[Carlos Garcia Campos]

Committed r266576: <https://trac.webkit.org/changeset/266576>