61167 - nsHTTPChannel::GetURI returns wrong result when proxying

Status: VERIFIED INVALID

(Core :: Networking: HTTP, defect, P3)

Description

[Bradley Baetz]

nsHTTPChannel::GetURI always returns the uri passed in. For ftp proxying (and
gopher), this uri is a garbage (but syntactically valid) uri (http://test.com
for ftp).

If you then get an html page with isindex (like a gopher query page as
constructed by squid), layout ends up calling channel->GetURI, and then tries to
send the query request to the dummy uri (when using a proxy). The attached patch
fixes this.

I doubt it can cause a problem - if somewhere was getting "http://test.com" back
as the uri and trying to do something with it then it was broken anyway

As well, the status bar text is incorrect ("Connecting to test.com") The
attached patch doesnt't fix that - I can't find out where that is being set, but
it probably needs to be changed.

Note that you won't see this problem with ftp, because ftp wraps the proxying
channel within nsFTPConnectionThread.cpp, and it also does its own status
messages. HTTP/HTTPS does it as special cases as well (beacuse of the different
headers), so you won't see the problem with that, and no other protocol in
mozilla supports proxies.

Gopher is a simpler protocol, and I have NewChannel just returning the proxy
channel directly, which is simpler, and seems to match the comments in
nsIProxy.idl. Patches to support gopher are attached to bug 49334. To duplicate,
apply those patches, set up a gopher proxy, then go to:

gopher://gopher.ptloma.edu/7/v2/vs

and try a search. You'll end up at http://example.com/?search-term instead of
getting the results as expected (non proxied gopher queries don't work, so you
can't verify it with that - apply the attached patch to see the difference in
the result)

Comment 1

[Bradley Baetz]

Attachment: patch for nsHTTPChannel

Comment 2

[Ben Bucksch (:BenB)]

ops, accidently removed the dependency.

Bug 58872 seems to be the culprit here. Code is mentioned.

Comment 3

[Gagan]

http bugs to "Networking::HTTP"

Comment 4

[Darin Fisher]

nominating for moz 0.9

Comment 5

[Bradley Baetz]

After some comments from darin on IRC, I looked into this a bit more. When
proxying, nsGopherHandler::NewChannel just returns the proxy channel, rather
than doing what the ftp channel does (ie I should return the gopher channel, and
begin most calls to the channel with:

    if (mProxyChannel)
        return mProxyChannel->Foo(blah);

)
I could do this, and I suppose it would work, but I don't think it should be
needed.

Comment 6

[Darin Fisher]

bbaetz: gopher should not expose the http channel to its clients.  it must fully
encapsulate the http channel.  it is wrong to hand out an http channel to clients
of gopher.

Comment 7

[Bradley Baetz]

OK, I'll change gopher then. I don't know if this is a bug then or not.

Why does it have to be fully encapsulated? I'm offering a channel that, when
read from, gives the gopher stream requested. If theres no reason, I may just
make the test.com uri the real one, and then everything should work.

Comment 8

[Darin Fisher]

It needs to be encapsulated because people will query information off of the
channel that you hand back.  They may very well query more than just the URI.
Consider for example code that QI's for a nsIHTTPChannel.  That code might
do something completely different if it gets a HTTP channel as opposed to
some other channel.  You don't know and can't predict what people will do.
It is just far safer to encapsulate Gopher so that clients never see the
underlying HTTP channel.  Abusing necko in this way has resulted in many
subtle difficult to track down bugs.  Just don't go there ;-)

Comment 9

[Darin Fisher]

marking INVALID

Comment 10

[Tom Everingham]

verified INVALID