1905037 - Add API similar to redirectTo using internal redirect

Status: RESOLVED FIXED

(Core :: Networking, task, P2)

Description

[Julian Descottes [:jdescottes]]

For Bug 1898158, webdriver BiDi would need an API to redirect using internal redirects so that the content page is not aware of the redirect. More over we need the redirect to be handled after http-on-before-connect, because that's where WebDriver BiDi will attempt to update the URL.

(In reply to Kershaw Chang [:kershaw] from Bug 1898158 comment #6)

(In reply to Julian Descottes [:jdescottes] from Bug 1898158 comment #5)

Let me start by asking again here, because while working on provideResponse I spotted some APIs which could be helpful.

Kershaw: For WebDriver BiDi network interception, we need to be able to change the url of a request blocked in the "beforeRequestSent" phase, which corresponds to http-on-before-connect at the moment.

We might be able to handle this as an internal redirect, as long as the page cannot detect the redirection. Eg if we navigate to a.com and update the url to b.com via network interception, the window.location.href should still be a.com.

Do you think an internal redirect would work here? It might lead to create duplicated events, but that's something we might be able to handle on our side. Or would we need to add a new API on necko side to support this scenario?

Yes, I think internal redirect should work here. However, our current RedirectTo API doesn't support internal redirect. I assume we my need to add another function argument for this.
Moreover, we need some changes around here, because the channel doesn't handle redirect immediately after http-on-before-connect.

Comment 1

[Julian Descottes [:jdescottes]]

Moreover, we need some changes around here, because the channel doesn't handle redirect immediately after http-on-before-connect.

Do you mean the redirect is usually handled later or is it handled before?
In theory if it's handled "later" it could be fine for us, we don't need it to happen immediately in http-on-before-connect. Or maybe redirects are only handled when we start processing the response so that would be an issue here?

Comment 2

[Kershaw Chang [:kershaw]]

(In reply to Julian Descottes [:jdescottes] from comment #1)

Moreover, we need some changes around here, because the channel doesn't handle redirect immediately after http-on-before-connect.

Do you mean the redirect is usually handled later or is it handled before?
In theory if it's handled "later" it could be fine for us, we don't need it to happen immediately in http-on-before-connect. Or maybe redirects are only handled when we start processing the response so that would be an issue here?

I mean the redirect is handled too late. If you call redirectTo in http-on-before-connect, the channel handles the redirect at here, which is too late.

Comment 3

[Kershaw Chang [:kershaw]]

Hi Sean,
Would you like to work on this one?

Comment 4

[Sean Kim]

By looking at the relevant code, the following points should be addressed if I understood everything correctly:

1. redirectTo should have an extra argument for internal redirect.
2. redirectTo should be moved such that it is called right after http-on-before-connect:
   https://searchfox.org/mozilla-central/rev/56dd89bcf4d3b85f66621e89eac6e2936ad382d9/netwerk/protocol/http/nsHttpChannel.cpp#879

Are there any additional considerations to keep in mind?

Comment 5

[Kershaw Chang [:kershaw]]

(In reply to Sean Kim from comment #4)

By looking at the relevant code, the following points should be addressed if I understood everything correctly:

1. redirectTo should have an extra argument for internal redirect.

This is correct.

2. redirectTo should be moved such that it is called right after http-on-before-connect:
   https://searchfox.org/mozilla-central/rev/56dd89bcf4d3b85f66621e89eac6e2936ad382d9/netwerk/protocol/http/nsHttpChannel.cpp#879

What we need is handling the redirect after http-on-before-connect, not moving redirectTo.
Maybe adding the code below is enough, but there could be more details need to be figured out.

  if (mAPIRedirectToURI) {
    return AsyncCall(&nsHttpChannel::HandleAsyncAPIRedirect);
  }

Comment 6

[Sean Kim]

Attachment: Bug 1905037 - Add Internal Redirect Support for RedirectTo API r=kershaw,jdescottes

Comment 7

[Sean Kim]

Attachment: Bug 1905037 - Add a test for Internal Redirect Support r=kershaw,jdescottes

Comment 8

[Sean Kim]

Attachment: Bug 1905037 - Update existing redirectTo API calls r=kershaw

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/303792420667
Add Internal Redirect Support for RedirectTo API r=kershaw,necko-reviewers
https://hg.mozilla.org/integration/autoland/rev/361b0802c006
Add a test for Internal Redirect Support r=kershaw,jdescottes,necko-reviewers

Comment 10

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/303792420667
https://hg.mozilla.org/mozilla-central/rev/361b0802c006