89206 - SOCKS: http broken b/c GET malformed

Status: VERIFIED FIXED

(Core :: Networking, defect, P1)

Description

[Oleg Makarenko]

From Bugzilla Helper:
User-Agent: Mozilla/4.77 [en] (X11; U; Linux 2.2.20-0.6.12 i686)
BuildID:    2001070209

When the SOCKS proxy setting is used the browser sends requests in form GET
http://www.slashdot.org/ as if it works through http proxy. It should send
simply GET /. Most sites respond with 403 Forbidden (slsahdot is an example).
Older builds worked ok (before  0.9.1)

Reproducible: Always
Steps to Reproduce:
1. In Prefernces->Advanced->Proxy->Manual proxy Configuration clear everything
except Socks v5 Host.
2. Try to access www.slashdot.org or www.internetword.com


Actual Results:  You get 403 or 404 depending on apache settings. On some sites
GET http://somesite.com / HTTP/1.1 works ok on other s it gives 403 or 404

Expected Results:  Mozilla should send GET / HTTP/1.1 not GET
http://www.slashdot.org/ HTTP/1.1

Comment 1

[Bradley Baetz (:bbaetz)]

Without testing, but looking at the code, that seems about right. We're
conditioning this based on the presence of a proxy hostname, and not checking
for socks.

-> me

Reporter, can you confirm if connecting to https sites fails as well through a
socks proxy?

Comment 2

[Oleg Makarenko]

yes, https doesn't work at all when I use socks proxy (and SSL proxy is empty). 

For https://www.platina.ru/ I get "The connection was refused when attempting to
contact www.platina.ru" and I see no network activity (with tcpdump) at all. No
even an attempt to connect to socks server or to the target host. It looks like
if SOCKS5 V5 is not empty Mozilla always tries to use SSL proxy setting no
matter is it empty or not.

Comment 3

[Bradley Baetz (:bbaetz)]

OK, confirming. This is bad - I'll try to come up with a patch today.

https appears to be a separate problem - we don't even appear to be talking to
the socks server in that case.

Comment 4

[Bradley Baetz (:bbaetz)]

Well, I'm attaching a fix for http://

However, we send an http request for ftp and for gopher.

And we don't send anything at all for https.

This makes socks support somewhat useless. :(

Comment 5

[Bradley Baetz (:bbaetz)]

OK, fixed http and https.

Other protocols now won't try to use socks, and I'll relnote that. See bug
89500.

Patch coming up. dougt/mscott - r/sr for trunk + branch?

dougt - can you PDT+ this as virtual gagan?

Comment 6

[Bradley Baetz (:bbaetz)]

Attachment: fix

Comment 7

[Doug Turner (:dougt)]

okay on the http changes for trunk. nogo on the ioserver changes for trunk per 
our discussion.

send mail to pdt2 asking for permission to checkin.  I don't know if I can PDT 
this bug.

Comment 8

[Bradley Baetz (:bbaetz)]

Attachment: this one will compile on the branch

Comment 9

[Simon Fraser [no longer active]]

Minor quibble: I find
  if (PL_strcmp(foo, "bar") != 0)
more readable than 
  if (PL_strcmp(foo, "bar"))
because it indicates that you know that a zero return means string match (strcmp 
sucks).

sr=sfraser

Comment 10

[Bradley Baetz (:bbaetz)]

checked in on trunk.

Comment 11

[selmer (gone)]

Sorry for missing this request for PDT+ last night.  I'm giving it now based on
the description implying that socks for http & https only is better than socks
that mostly doesn't work at all for anything.  Please check in at the
appropriate time and we'll pick this up in the next build.

Comment 12

[benc]

changed summary.

What was the story w/ HTTPS?

Comment 13

[Bradley Baetz (:bbaetz)]

benc: We were trying socks over ssl rather than ssl over socks. Oops.

I'll check this in on the branch once my build completes, and it opens.

Comment 14

[Bradley Baetz (:bbaetz)]

-> gagan for checkin on the branch when it opens next week

Comment 15

[Gagan]

Fix checked in on the branch as well.

Comment 16

[benc]

VERIFIED:
All plats
07-17-03-0.9.2

NOTE: Mac is transmitting, but failing. Bradley says this was broken before, but
there is no bug, so I created bug 92002.