1855466 - It's not possible to block opaque responses given a "multipart/x-mixed-replace" channel.

Status: NEW

(Core :: DOM: Networking, defect, P2)

Description

[Andreas Farre [:farre]]

To be able to block "multipart/x-mixed-replace" responses we need to be able to look into that kind of channel in the parent and call the ORB related methods in nsHttpChannel::CallOnStartRequest (e.g PerformOpaqueResponseSafelistCheckBeforeSniff and EnsureOpaqueResponseIsAllowedAfterSniff), as well as participating in sniffing.

We already have a step where we install a multipart converter for DocumentLoadListener, but in that case we'll forget that we're an nsHttpChannel, and really only be able to access what we have from nsIMultipartChannel/nsPartChannel. Especially we'll not be able to participate in sniffing, so it doesn't help to abstract the ORB methods to be non-member helper functions either (nsIMultipartChannels doesn't have a response head getter, for example).

It feels like this is doable but a big task. I'd be happy to make a compromise somehow and only look at the first part, for example. But even that feels complicated.

Comment 1

[Andreas Farre [:farre]]

Valentin, do you know of a solution that I've overlooked here?

Comment 2

[Valentin Gosu [:valentin] (he/him)]

I'm not 100% sure if I understand the issue here. I'd appreciate more details of what's actually missing.

We already have a step where we install a multipart converter for DocumentLoadListener, but in that case we'll forget that we're an nsHttpChannel,

You can still access the underlying channel of a multipart channel if you want:

https://searchfox.org/mozilla-central/rev/2f5b657343ce18e4b8a56417f707022550f4b742/netwerk/base/nsIMultiPartChannel.idl#16-21

interface nsIMultiPartChannel : nsISupports
{
    /**
     * readonly attribute to access the underlying channel
     */
    readonly attribute nsIChannel baseChannel;

and really only be able to access what we have from nsIMultipartChannel/nsPartChannel. Especially we'll not be able to participate in sniffing, so it doesn't help to abstract the ORB methods to be non-member helper functions either (nsIMultipartChannels doesn't have a response head getter, for example).

What headers exactly are we interested in? We do process the headers for the multipart content, but we ignore most of them. See here
There is an unused mResponseHead - if we really do need access to all the headers, we can probably make that happen.

It feels like this is doable but a big task. I'd be happy to make a compromise somehow and only look at the first part, for example. But even that feels complicated.

Is the problem that we're only looking at one multipart channel, but delivering all of them to the content process?

Comment 3

[Andreas Farre [:farre]]

Yeah, I was a bit vague in my description, sorry. I'll expand a bit:

ORB works with nsHttpChannel and HttpBaseChannel to perform blocking and does so with PerformOpaqueResponseSafelistCheckBeforeSniff and PerformOpaqueResponseSafelistCheckAfterSniff + sniffing called from CallOnStartRequest. As far as I understand how nsIMultipartChannel/nsPartChannel works the ideal solution to handle ORB for nsIMultipartChannel would be to make every nsPartChannel do this as well. In that way we could allow or block a part in a very fine grained manner (I guess what would happen would be that we'd allow until we block the first time). But since we're implementing ORB on top of nsHttpChannel/HttpBaseChannel and nsPartChannel isn't a sub-class of these we'd either need to make it so, or extract PerformOpaqueResponseSafelistCheckBeforeSniff and PerformOpaqueResponseSafelistCheckAfterSniff + sniffing from nsHttpChannel/HttpBaseChannel and make it callable with a nsPartChannel.

I realize that this is probably a big change for a niche feature like nsIMultipartChannel and spec wise for ORB it would probably be ok to only look at the first part, and have that resolve whether we should block or not, but the fact that the capabilities of nsPartChannel is slimmer than nsHttpChannel/HttpBaseChannel is the thing I'm worried about mostly.

So yes, in some sense the problem is that we're looking at one multipart channel but in essence it's more the fact that when looking at a multipart channel you're actually not looking at the contents of the parts of the channel.

But the same issues for ORB applies, we shouldn't send any content from a multipart channel to a content process until we've had a chance to block it.

If we can find a solution to "look at the contents of the first part and in the context of ORB pretend that it's the content for the channel" or something similar I'm happy to do the implementation work.

Comment 4

[Andreas Farre [:farre]]

Attachment: WIP: Bug 1855466 - WIP: Handle multipart/x-mixed-replace for ORB.

This is very much a prototype to show that it is indeed possible to
fully handle ORB for multipart/x-mixed-replace. Unfortunately it means
a lot of duplicated code to make nsPartChannel also handle pre/post
sniff ORB as well as actually sniffing.

Comment 5

[Valentin Gosu [:valentin] (he/him)]

Looks promising. Indeed, having to chain more unknown decoders and all that duplicated code seems suboptimal.
Btw, I don't think we support content-encoding for partchannels - I'd be surprised if that was necessary - it might simplify the code "a bit™️"