See bug 89206. Fixing this involves a bit of code reorganisation, and won't happen for a while. If someone can comment on the proposed release note, I'll go and get it added. DRAFT RELEASE NOTE: Netscape/Mozilla only supports the use of SOCKS proxies for http and https connections. Other protocols (such as ftp) will not attempt to use a socks proxy. The mox relnotes can reference this bug.

I need to discuss API changes with darin.

Oh, and this will allow mail/news to work through socks as well, which according to a post on npm.general ns4 could do.

Check with someone in mailnews about bug 44995.

I should hopefully have an infrastructure patch for this early next week

OK, I'm going to attach a patch. This allows datetime to work through a socks proxy. Its not finished - next is ftp, then I'll try to remove all the now-unneeded socks cruft from http. Then I'll work on mailnews. ssl support will need a change to the helper function signature to pass the type in. We need a better way of handling proxable types, to remove the checks from nsProtocolProxyService. I'm thinking of two extra flags on the protocol handler (via the recently added URIType attribute); URI_CAN_PROXY and URI_CAN_PROXY_HTTP. Because of PAC, I need to separate this out (some pac files will just return "PROXY foo:80", and that doesn't mean that smtp should be done through the proxy, and PAC will support socks soon - I may do this as part of my changes). If a protocol doesn't have the URI_CAN_PROXY flag then CreateTransportInfo will return null before talking to the protocolproxy service. If it doesn't have URI_CAN_PROXY_HTTP, then if we end up getting a proxy of type "http", we'll return null. (This also means that existing protocol handlers only have to be modified if they want to support proxying. All protocols supported by http proxying are in the mozilla tree, so thats not an issue for third parties) comments? ccing valeski for the proposed minor api change, and any other comments.

AAfter sepaking with darin, I plan to move the various helper functions onto more appropriate interfaces, and possibly get rid of the transport creation helper, and just change the signature of nsSocketTransport::CreateTransport and friends. And I got the check for a null transportInfo wrong.

I had to redo the API a bit to allow https via a proxy to work (PSM needs the correct final hostname to do cert verification). I think that its cleaner now, as well. All non mailnews protcols have been modified - looking for testing, r/sr.

some nits: 1) there exist references to nsISupportsTransparentProxy in nsIProtocolHandler.idl 2) the comments for URI_PROXY and URI_PROXY_HTTP need to be fixed. the meaning of these flags is not clear from the comments? 3) does the for loop near line 270 of nsSocketTransport.cpp need to be wrapped? 4) out of curiosity: are there http proxies out there that implement the datetime protocol? 5) off by 1 indentation problem near callsite of NewProxiedChannel in nsHttpHandler.cpp 6) fix instances of NewProxyChannel in nsHttpHandler::NewProxiedChannel 7) add comment block for nsHttpHandler::nsIProxiedProtocolHandler fix these, test it out a bunch, and r/sr=darin

dougt should review this too.

r=rginda for the chatzilla changes.

1) Fixed 2) Fixed 3) If I don't then its 86 chars, but given the rest of the file I've just unwrapped it. 4) Doubt it. And even if there are, we don't have the pref for manual proxies, and not all proxies support that. The flag being there was a typo, and I didn't pick it up because it made no difference with manual proxies. Fixed. 5) I really dislike msvc's editor... fixed. 6) Fixed 7) I don't understand this. nsIHttpProtocolHandler now inherits of nsIProxiedProtocolHandler, rather than nsIProtocolHandler. What do you mean by this?

the comment block //----------------------------------------------------------------------------- // nsHttpHandler::nsIProxiedProtocolHandler //----------------------------------------------------------------------------- is just meant to indicate that this section contains nsHttpHandler's implementation of the methods declared on nsIProxiedProtocolHandler. please add this comment above the NewProxiedChannel method definition. thanks! also, can you attach a new patch when you get around to it?

I think that bbaetz is gone, reassigning to smeredith.

I'm here til tuesday, and have r/sr=darin, and r/sr=dougt after we discuss the overloading of URIFlags with darin (and changing URI_PROXY to URI_ALLOW_PROXY, which I've done in my local tree) Taking back for now.

OK, patch with minor changes (URIType->ProtocolFlags), and a few style nits. This one is larger, because I changed the method name on all the protocol handlers. I'll take care of the commercial one-liner fix. I'll mail drivers for approval.

ok, so drivers rejected this. Moving to 0.9.5, and nominating nsBranch. This means that it won't go in before Tuesday (monday is a public holiday) Either darin will check this in for me, or I'll have net access by then and will do it myself (darin mailed me the mac mcp files). Either way I'll sort it out next week.

Marking nsbranch+ and PDT+. Who is going to check this in? Darin? Steve? Thanks!

i can do it if bradley is unable to. bradley?

Darin: Yes, please check this in for me - I still don't have net access at home. Remember the commercial one-liner.

-> me for checkin

fixed-on-trunk

-> 0.9.4 branch

untargeted until mozilla.org ships 0.9.4

back to 0.9.4, awaiting PDT approval for checkin on the 0.9.4 branch

0.9.4 is out the door

-> 0.9.4 for checkin on the nsbranch

Get r/sr = in patch status, and you can check it in.

Comment on attachment 47287 [details] [diff] [review] v1.2 r=dougt sr=darin

hoping to land this on the branch tomorrow.

Has this landed on the branch?

nope.. i got delayed.. i'm now planning to land this today.

fixed-on-branch

->qa to trix. I'll dogfood, but I'm not the lead tester here.

reassigning qa to lrg

RELNOTE Netscape 6.2: Mozilla|Netscape supports the use of SOCKS 4 and 5 proxies for http, https and ftp. Other protocols will ignore the SOCKS preference.

benc: No - the bug was that only http|https was proxied. This has been fixed with this patch (except for mailnews) Resetting summary relnote: netscape6.x: Non-mailnews protocols will use socks proxies. However, news and mail protocls will ignore any socks proxy settings you have made. (I assume theres a separate relnote on password-authenticated socks proxies not working)

FTP works right? that's all I really want to know.

Hmm, last message didn't seem to get through. Verfied fixed on MacOS Linux and Windows, no major problems. Handled ftp http and https well. When the proxy address was invalid, Mail/News, IMAP, and AIM requests went through fine across all platforms. Small concern dealing with IMAP, the images refered in these files are often hosted externally to the imap document; In the case that the server where these images are stored is invalid, the page loading process is extremely slow. In other cases of invalid (by proxy configuration) urls, it seems that this failure is quickly noticed, and Netscape doesn't try to load the blocked page.