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

RESOLVED FIXED in Firefox 62

Status

()

enhancement
P2
normal
RESOLVED FIXED
11 months ago
3 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)

(Assignee)

Description

11 months ago
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.
(Assignee)

Comment 1

11 months ago
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)

Updated

11 months ago
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+
(Assignee)

Comment 3

11 months ago
(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.
(Assignee)

Comment 5

11 months ago
Sure.  This does s/initial/special/ in that comment.
Attachment #8982009 - Attachment is obsolete: true
Attachment #8982259 - Flags: review+

Comment 6

11 months ago
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

Comment 7

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7e6bcb2cd0d8
Status: ASSIGNED → RESOLVED
Last Resolved: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.