Bug 191858 - [GTK] Unexpected User-Agent on redirect
Summary: [GTK] Unexpected User-Agent on redirect
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-11-20 03:15 PST by Jim Mason
Modified: 2021-02-17 07:06 PST (History)
10 users (show)

See Also:


Attachments
Patch (67.11 KB, patch)
2020-08-31 06:44 PDT, Carlos Garcia Campos
aperez: review+
Details | Formatted Diff | Diff
Patch (9.52 KB, patch)
2020-09-01 06:43 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Patch (20.54 KB, patch)
2020-09-02 04:35 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Patch (21.56 KB, patch)
2020-09-02 05:12 PDT, Carlos Garcia Campos
youennf: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jim Mason 2018-11-20 03:15:06 PST
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.
Comment 1 Michael Catanzaro 2018-11-21 11:36:38 PST
(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.
Comment 2 Jim Mason 2018-11-22 02:46:42 PST
(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.
Comment 3 Carlos Garcia Campos 2020-08-31 06:44:49 PDT
Created attachment 407598 [details]
Patch
Comment 4 Michael Catanzaro 2020-08-31 10:34:37 PDT
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.
Comment 5 Carlos Garcia Campos 2020-09-01 01:27:38 PDT
(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.
Comment 6 youenn fablet 2020-09-01 01:58:37 PDT
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.
Comment 7 Adrian Perez 2020-09-01 02:00:45 PDT
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?
Comment 8 Carlos Garcia Campos 2020-09-01 02:18:26 PDT
(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.
Comment 9 Carlos Garcia Campos 2020-09-01 02:19:07 PDT
(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
Comment 10 youenn fablet 2020-09-01 02:31:31 PDT
(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?
Comment 11 Carlos Garcia Campos 2020-09-01 02:46:38 PDT
(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.
Comment 12 Carlos Garcia Campos 2020-09-01 02:49:10 PDT
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.
Comment 13 youenn fablet 2020-09-01 02:59:37 PDT
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?
Comment 14 Carlos Garcia Campos 2020-09-01 03:22:03 PDT
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.
Comment 15 Carlos Garcia Campos 2020-09-01 04:07:47 PDT
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.
Comment 16 Carlos Garcia Campos 2020-09-01 06:43:25 PDT
Created attachment 407676 [details]
Patch
Comment 17 EWS Watchlist 2020-09-01 06:44:23 PDT
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
Comment 18 Michael Catanzaro 2020-09-01 07:27:49 PDT
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?
Comment 19 Carlos Garcia Campos 2020-09-01 07:31:17 PDT
(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.
Comment 20 youenn fablet 2020-09-01 07:34:45 PDT
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.
Comment 21 Carlos Garcia Campos 2020-09-01 08:07:37 PDT
(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 :-(
Comment 22 youenn fablet 2020-09-01 08:23:09 PDT
> > 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.
Comment 23 Adrian Perez 2020-09-01 13:26:13 PDT
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.
Comment 24 Jim Mason 2020-09-02 01:45:51 PDT
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.
Comment 25 Carlos Garcia Campos 2020-09-02 02:39:41 PDT
(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.
Comment 26 youenn fablet 2020-09-02 02:54:38 PDT
> > 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.
Comment 27 Carlos Garcia Campos 2020-09-02 03:00:24 PDT
(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?
Comment 28 youenn fablet 2020-09-02 03:04:00 PDT
> > 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...)
Comment 29 youenn fablet 2020-09-02 03:07:41 PDT
> 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.
Comment 30 Carlos Garcia Campos 2020-09-02 04:35:12 PDT
Created attachment 407756 [details]
Patch

Updated patch to include unit tests.
Comment 31 Carlos Garcia Campos 2020-09-02 05:12:08 PDT
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.
Comment 32 Jim Mason 2020-09-02 06:32:11 PDT
(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.
Comment 33 Carlos Garcia Campos 2020-09-03 22:14:02 PDT
Committed r266576: <https://trac.webkit.org/changeset/266576>