212530 - view-source should drop almost all response headers

Status: RESOLVED FIXED

(Core Graveyard :: View Source, defect, P2)

Description

[Boris Zbarsky [:bzbarsky]]

At the moment, we drop the "refresh" header.  But the "link" header can attach
stylesheets and other headers may be introduced that do weird stuff.

So we should, as suggested by hixie, clear all headers but the ones we know we want.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed fix

Comment 2

[Darin Fisher]

so nsViewSourceChannel implements nsIHttpChannel, which means that it implements
GetResponseHeader.  so, why can't it just block queries for any header other
than "Content-Type"?  of course then the
NS_FORWARD_SAFE_NSIHTTPCHANNEL(mHttpChannel) could not be used :-/

Comment 3

[Boris Zbarsky [:bzbarsky]]

Yeah, that was sort of the motivation...

Taking bug; not sure why it wasn't assigned to me to start with.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Oh, we'd have to block VisitResponseHeaders too, not just GetResponseHeader....

Comment 5

[Darin Fisher]

so codesighs-wise, it'd probably be better to drop the IMPL macro and just write
the functions manually :-/  the visit method can just call the visitor once
explicitly for the value of the content-type header.  the only downside is
having to explicitly write out more methods, but at least we won't ever need to
modify those methods... nsIHttpChannel is frozen afterall ;-)

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Use darin's preferred approach

Comment 7

[Darin Fisher]

Comment on attachment 127759 [details] [diff] [review]
Use darin's preferred approach

>Index: netwerk/protocol/viewsource/src/nsViewSourceChannel.cpp

>+nsViewSourceChannel::GetRequestMethod(nsACString & aRequestMethod) {
>+    return !mHttpChannel ? NS_ERROR_NULL_POINTER :
>+        mHttpChannel->GetRequestMethod(aRequestMethod);
>+}

nit: keep the bracket style consistent.

hmm... QI to nsIHttpChannel is not possible if mHttpChannel == NULL,
so do we even need to null check mHttpChannel?	hmm... actually, it
is probably wise in case the QI in OnStartRequest fails for some odd
reason.


>+NS_IMETHODIMP
>+nsViewSourceChannel::VisitResponseHeaders(nsIHttpHeaderVisitor *aVisitor) {
...
>+        aVisitor->VisitHeader(NS_LITERAL_CSTRING("content-type"), contentType);

nit: NS_LITERAL_CSTRING("Content-Type") to be consistent with nsHttpChannel.
yeah, i know the interface doesn't require this, but uhm... yeah 8)

sr=darin with those changes

Comment 8

[Boris Zbarsky [:bzbarsky]]

Checked in.