propagate internal redirects initiated by an InterceptedHttpChannel back to the child process

RESOLVED FIXED in Firefox 62

Status

()

enhancement
P2
normal
RESOLVED FIXED
Last year
6 months ago

People

(Reporter: bkelly, Assigned: bkelly)

Tracking

(Blocks 1 bug)

unspecified
mozilla62
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox62 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

In bug 1391693 comment 62 I added some code that "hid" the internal redirect from nsHttpChannel to InterceptedHttpChannel from the child process's HttpChannelChild.  This was necessary to avoid confusing the complicated legacy service worker interception code that operates in the child process.

Unfortunately it seems that the code I added was a bit to aggressive in hiding internal redirects.  When a service worker intercepts with an opaque response we need to do an internal redirect to the new URL.  This goes from InterceptedHttpChannel to InterceptedHttpChannel.  In this case we *do* want to allow the redirect through to the child process so downstream listeners can act appropriately.
Honza, please see comment 0 for some background here.  Basically we have an ugly work around to make parent-side intercept work while still keeping the complicated child-side intercept code around.  This work around involves hiding an internal redirect from nsHttpChannel to InterceptedHttpChannel.  This code was also accidentally blocking internal redirects from InterceptedHttpChannel to another InterceptedHttpChannel.  This patch corrects that issue.
Attachment #8982009 - Flags: review?(honzab.moz)
Priority: -- → P2
Comment on attachment 8982009 [details] [diff] [review]
Only hide the initial internal redirect to an InterceptedHttpChannel and not internal redirects initiated from IHC. r=mayhemer

Review of attachment 8982009 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/protocol/http/HttpChannelParent.cpp
@@ +1773,5 @@
> +    // We only want to hide the initial internal redirect from nsHttpChannel
> +    // to InterceptedHttpChannel.  We want to allow through internal redirects
> +    // initiated from the InterceptedHttpChannel even if they are to another
> +    // InterceptedHttpChannel.
> +    if (!oldIntercepted && newIntercepted) {

what has to happen when the redirects go as (if even possible) httpchannel -> interceptchannel -> httpchannel -> interceptchannel?
Attachment #8982009 - Flags: review?(honzab.moz) → review+
(In reply to Honza Bambas (:mayhemer) from comment #2)
> what has to happen when the redirects go as (if even possible) httpchannel
> -> interceptchannel -> httpchannel -> interceptchannel?

This can happen.  For example:

1. Navigation A creates nsHttpChannel
2. Navigation A is intercepted, causing a hidden internal redirect to InterceptedHttpChannel
3. The service worker returns a 30x redirect response to URL B
4. The InterceptedHttpChannel performs a real redirect back to a new nsHttpChannel for URL B
5. The B nsHttpChannel is intercepted again, causing a hidden internal redirect to InterceptedHttpChannel

So each internal redirect from nsHttpChannel->InterceptedHttpChannel is hidden from the child process.  Redirects produced by the InterceptedHttpChannel are never hidden.

Hope that explains.
(In reply to Ben Kelly [:bkelly] from comment #3)
> (In reply to Honza Bambas (:mayhemer) from comment #2)
> > what has to happen when the redirects go as (if even possible) httpchannel
> > -> interceptchannel -> httpchannel -> interceptchannel?
> 
> This can happen.  For example:
> 
> 1. Navigation A creates nsHttpChannel
> 2. Navigation A is intercepted, causing a hidden internal redirect to
> InterceptedHttpChannel
> 3. The service worker returns a 30x redirect response to URL B
> 4. The InterceptedHttpChannel performs a real redirect back to a new
> nsHttpChannel for URL B
> 5. The B nsHttpChannel is intercepted again, causing a hidden internal
> redirect to InterceptedHttpChannel
> 
> So each internal redirect from nsHttpChannel->InterceptedHttpChannel is
> hidden from the child process.  Redirects produced by the
> InterceptedHttpChannel are never hidden.
> 
> Hope that explains.

Thanks.  Then I think the "the initial internal redirect" words in the comment should be changed.  That was what I was confused with, probably.
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7e6bcb2cd0d8
Only hide the initial internal redirect to an InterceptedHttpChannel and not internal redirects initiated from IHC. r=mayhemer
https://hg.mozilla.org/mozilla-central/rev/7e6bcb2cd0d8
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.