Bug 295030 - Update Manager doesn't work with SOCKS proxy
Summary: Update Manager doesn't work with SOCKS proxy
Status: RESOLVED FIXED
Alias: None
Product: ECF
Classification: RT
Component: ecf.filetransfer (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: ecf.core-inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-12 19:34 EST by John Cortell CLA
Modified: 2010-05-10 05:50 EDT (History)
8 users (show)

See Also:


Attachments
possible fix (2.24 KB, patch)
2009-11-13 02:08 EST, John Cortell CLA
no flags Details | Diff
Solution that works for me (4.43 KB, patch)
2009-11-13 12:38 EST, John Cortell CLA
no flags Details | Diff
Authenticaiton dialog that pops up (6.33 KB, image/gif)
2009-11-13 12:46 EST, John Cortell CLA
no flags Details
Proof that prior to my changes, SOCKS is used at the java.net layer (56.27 KB, image/gif)
2009-11-13 16:46 EST, John Cortell CLA
no flags Details
runtime workbench console output (2.20 KB, text/plain)
2010-01-29 09:38 EST, John Cortell CLA
no flags Details
Attachment to comment 26 (5.90 KB, text/plain)
2010-01-29 13:05 EST, Henrich Kraemer CLA
no flags Details
Untested patch (19.32 KB, patch)
2010-01-29 20:02 EST, Henrich Kraemer CLA
no flags Details | Diff
Proposed patch (27.68 KB, patch)
2010-04-22 14:47 EDT, Henrich Kraemer CLA
no flags Details | Diff
Error dialog mentioned in comment 65 (7.71 KB, image/gif)
2010-04-22 16:52 EDT, John Cortell CLA
no flags Details
Proposed patch readded removed API method (25.73 KB, patch)
2010-04-23 18:19 EDT, Henrich Kraemer CLA
slewis: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Cortell CLA 2009-11-12 19:34:58 EST
The Update Manager does not work with a SOCKS proxy (at least not in the common scenario of the update site being an http URL). It seem equally broken in 3.5.1 and 3.6M3.

Try in an eclipse instance that ONLY has a SOCKS server specified (native, and no http or https proxy), with the userid and password stored in the proxy definition. Then bring up the update manager. You'll see that you get prompted for a SOCKS userid/password, which is the first sign that things are going wrong (it shouldn't prompt since you provided that information in the proxy definition). In this scenario, the socket connection at least succeeds (set a breakpoint in SocketEventCreateUtil.createSocket()), but alas the ecf plugin ends up unable to identify an http server.

But it gets worse. Now try in an instance where you have both a SOCKS proxy and an HTTP one configured. In this situation, no only do you, again, get prompted for a SOCKS userid/password (which equally makes no sense in this case), but the SocketEventCreateUtil.createSocket() logic fails to even connect.
Comment 1 John Cortell CLA 2009-11-13 02:08:11 EST
Created attachment 152132 [details]
possible fix

Understanding about 0.01% of all this ecf code, I naively submit this patch. It solves the issue for me. At a minimum, maybe it'll provide some insight into what's going wrong
Comment 2 Henrik Lindberg CLA 2009-11-13 06:02:51 EST
For ECF
Comment 3 John Cortell CLA 2009-11-13 12:38:23 EST
Created attachment 152176 [details]
Solution that works for me
Comment 4 John Cortell CLA 2009-11-13 12:44:58 EST
Reproducibility steps: 
Requires access to a SOCKS server.
1. Launch eclipse
2. Ensure you have at least one non-local update site configured in Windows > Preferences > General > Install/Update > Available Software Sites
3. In Windows > Preferences > General > Network Connections, set Active Provider to Manual and configure the proxy server description. Include the authentication information
4. Go to Help > Install New Software...
5. Authentication dialog seen in attached screenshot comes up. That's the first problem. You've provided authentication information in step (3). You shouldn't be getting asked here. But go ahead and enter the userid and password.
6. Hit OK.
Dialog comes back up again and will do so many more times, regardless of the fact that you're entering the right authentication information. Note that you can't even cancel out of the long sequence of dialogs. That's a separate issue (295021)
Comment 5 John Cortell CLA 2009-11-13 12:46:26 EST
Created attachment 152177 [details]
Authenticaiton dialog that pops up
Comment 6 Scott Lewis CLA 2009-11-13 14:03:32 EST
Hi John.

Thanks kindly for the bug report and the patch.  I have a question before applying...just to make sure this won't cause any regression.  The question is for everyone on this bug...not necessarily just for you.

In your patch, the detection of a valid socks proxy essentially overrides the detection of any other proxies...no matter what the scheme. 

Is this in keeping with the expected use of the proxy API?  That is, if the proxy API has a socks proxy and an http proxy defined (for example), your code will select the socks proxy, which I think is fine, but I would like to confirm that assumption.
Comment 7 John Cortell CLA 2009-11-13 14:30:29 EST
(In reply to comment #6)
> Hi John.
> 
> Thanks kindly for the bug report and the patch.  I have a question before
> applying...just to make sure this won't cause any regression.  The question is
> for everyone on this bug...not necessarily just for you.
> 
> In your patch, the detection of a valid socks proxy essentially overrides the
> detection of any other proxies...no matter what the scheme. 
> 
> Is this in keeping with the expected use of the proxy API?  That is, if the
> proxy API has a socks proxy and an http proxy defined (for example), your code
> will select the socks proxy, which I think is fine, but I would like to confirm
> that assumption.

Scott,

That's exactly right. I tried to explain why in the comment, but I may not have been clear enough. Regardless of what proxies are available (configured in Eclipse), we end up using a SocksSocketImpl. That implementation, which is down in the java.net library, automatically uses the available SOCKS proxy (if not available, it just defers activity to its base class--a standard socket implementation). SocksSocketImpl uses the available SOCKS proxy even if there's also a, e.g., an http proxy configured in Eclipse. For that reason, ecf needs to be in synch with what the SocksSocketImpl will eventually use, and that means giving precedence to SOCKS regardless of the target URL and any other available proxies.

That said, I'm not claiming to be an expert on ecf. I didn't even know what ecf was until 24 hours ago, so someone with a lot more experience should double check that what I'm saying makes sense. For certain, it fixes the problem I'm seeing.
Comment 8 Henrich Kraemer CLA 2009-11-13 14:48:04 EST
(In reply to comment #5)
> Created an attachment (id=152177) [details]
> Authenticaiton dialog that pops up

Switching off HttpClient retries in general might have unwanted effects for general robustness. 

I would like to see whether this can be done more selectively. At the moment I am having unrelated trouble setting me up for reproducing this.
Comment 9 John Cortell CLA 2009-11-13 14:55:32 EST
(In reply to comment #8)
> (In reply to comment #5)
> > Created an attachment (id=152177) [details] [details]
> > Authenticaiton dialog that pops up
> 
> Switching off HttpClient retries in general might have unwanted effects for
> general robustness. 
> 
> I would like to see whether this can be done more selectively. At the moment I
> am having unrelated trouble setting me up for reproducing this.

The retry issue is a different bug (295021). This has to do with SOCKS support outright not working
Comment 10 Scott Lewis CLA 2009-11-13 15:41:21 EST
(In reply to comment #7)
> (In reply to comment #6)
<stuff deleted>
> Scott,
> 
> That's exactly right. I tried to explain why in the comment, but I may not have
> been clear enough. Regardless of what proxies are available (configured in
> Eclipse), we end up using a SocksSocketImpl. That implementation, which is down
> in the java.net library, automatically uses the available SOCKS proxy (if not
> available, it just defers activity to its base class--a standard socket
> implementation). SocksSocketImpl uses the available SOCKS proxy even if there's
> also a, e.g., an http proxy configured in Eclipse. For that reason, ecf needs
> to be in synch with what the SocksSocketImpl will eventually use, and that
> means giving precedence to SOCKS regardless of the target URL and any other
> available proxies.
> 
> That said, I'm not claiming to be an expert on ecf. I didn't even know what ecf
> was until 24 hours ago, so someone with a lot more experience should double
> check that what I'm saying makes sense. For certain, it fixes the problem I'm
> seeing.

As much as ECF, this has to do with the eclipse core proxy API...as ECF uses
the core proxy API.  I guess what I would like to make sure of is that 'socks'
override is the expected/desired behavior when more than one proxy is
configured.
Comment 11 John Cortell CLA 2009-11-13 16:16:05 EST
(In reply to comment #10)
> As much as ECF, this has to do with the eclipse core proxy API...as ECF uses
> the core proxy API.  I guess what I would like to make sure of is that 'socks'
> override is the expected/desired behavior when more than one proxy is
> configured.

Just to be clear, technically my changes don't give SOCKS precedence. Prior to my changes, SOCKS already has precedence. This is evident by stepping through the SocksSocketImpl code (in java.net). All I did was align the ECF code to be on the same page, and not try to use a proxy that conflicts with what SocksSocketImpl ends up using.
Comment 12 Scott Lewis CLA 2009-11-13 16:26:26 EST
(In reply to comment #11)
> (In reply to comment #10)
> > As much as ECF, this has to do with the eclipse core proxy API...as ECF uses
> > the core proxy API.  I guess what I would like to make sure of is that 'socks'
> > override is the expected/desired behavior when more than one proxy is
> > configured.
> 
> Just to be clear, technically my changes don't give SOCKS precedence. Prior to
> my changes, SOCKS already has precedence. This is evident by stepping through
> the SocksSocketImpl code (in java.net). All I did was align the ECF code to be
> on the same page, and not try to use a proxy that conflicts with what
> SocksSocketImpl ends up using.

Hi John.  Ok...if socks has precedence in java.net then I'll assume it's OK to give it precedence WRT the proxy API also.  I'll work on applying the patch shortly.

If anyone reading this knows this to be incorrect under some conditions, then please let everyone know on this bug.  If at all possible, I would like someone who works on the eclipse proxy API to comment/review...but I'm not currently sure who that is.
Comment 13 John Cortell CLA 2009-11-13 16:46:37 EST
Created attachment 152212 [details]
Proof that prior to my changes, SOCKS is used at the java.net layer

Scott, this backs up my claim. Keep in mind that this is WITHOUT my changes. And note that ECF ends up setting up its state to use the HTTP proxy, thus causing the problem (a mismatch between ECF and java.net)
Comment 14 Henrich Kraemer CLA 2009-11-13 18:51:24 EST
(In reply to comment #9)
Yes you are right.
Comment 15 John Arthorne CLA 2010-01-07 10:00:21 EST
See bug 289933 for a similar bug about pick the most appropriate proxy for a given connection.
Comment 16 John Cortell CLA 2010-01-28 18:11:38 EST
It seems to me this and 295021 have stalled. These are very serious issues for anyone
  (a) behind a corporate firewall
  (b) simultaneously working with sources in CVS and Subversion

The workaround is to repeatedly add and remove the http and socks proxies when going between the two repositories. Not only is this very inconvenient, but if you forget, you end up having to cancel out of a dialog umpteen times, which is, frankly, intolerable. I'm hoping this post will renew interest in getting these problems fixed, hopefully for 3.6.
Comment 17 Scott Lewis CLA 2010-01-28 18:23:18 EST
(In reply to comment #16)
> It seems to me this and 295021 have stalled. These are very serious issues for
> anyone
>   (a) behind a corporate firewall
>   (b) simultaneously working with sources in CVS and Subversion
> 
> The workaround is to repeatedly add and remove the http and socks proxies when
> going between the two repositories. Not only is this very inconvenient, but if
> you forget, you end up having to cancel out of a dialog umpteen times, which
> is, frankly, intolerable. I'm hoping this post will renew interest in getting
> these problems fixed, hopefully for 3.6.

I (Scott) need input from others in order to address this bug.  Specifically, what should actually be done here?  Will the workaround by John in attachment 152176 [details] work in general? 

Specifically, Henrich could you provide a recommendation about addressing this bug and bug 295021?

If the fixes are relatively simple, and we can agree publicly on those fixes, and with testing they do not introduce significant regressions/other problems then I see no reason why this can't be addressed for 3.6.  But, I cannot currently dedicate very much of my own time to these bugs, and don't have access to any environments that allow me to reproduce or test what John and others are experiencing...so I have to depend upon the people on the cc list or others to reproduce, provide code changes, and test those changes...so that we can get something that will be workable for everyone.
Comment 18 Henrich Kraemer CLA 2010-01-28 19:10:35 EST
> I (Scott) need input from others in order to address this bug.  Specifically,
> what should actually be done here?  Will the workaround by John in attachment
> 152176 work in general? 
> 
> Specifically, Henrich could you provide a recommendation about addressing this
> bug and bug 295021?
> 

I just added a comment in 295021. 295021 should have not a lot of impact on this defect.

AFAIK if users have configured to use both socks and HTTP proxy that the communication should use both and SOCKS would be first. 

If I follow the scenario correctly the changes cause ECF to use the credentials provided by IProxyService related to the SOCKS proxy so that overrides Authenticator.setDefault() globally and without changing it back. As a consequence subsequent communication will get past the proxy. 

But it appears that ECF would then also not use the HTTP proxy configured. Am I wrong?
Comment 19 Scott Lewis CLA 2010-01-28 19:14:50 EST
(In reply to comment #18)
<stuff deleted>
> 
> AFAIK if users have configured to use both socks and HTTP proxy that the
> communication should use both and SOCKS would be first. 
> 
> If I follow the scenario correctly the changes cause ECF to use the credentials
> provided by IProxyService related to the SOCKS proxy so that overrides
> Authenticator.setDefault() globally and without changing it back. As a
> consequence subsequent communication will get past the proxy. 
> 
> But it appears that ECF would then also not use the HTTP proxy configured. Am I
> wrong?

I'm not sure if you are asking me or John but my answer to this question is:  I don't know.
Comment 20 Henrich Kraemer CLA 2010-01-28 19:26:58 EST
(In reply to comment #19)
> 
> I'm not sure if you are asking me or John but my answer to this question is:  I
> don't know.

Scott, I was asking John.

- Henrich
Comment 21 John Cortell CLA 2010-01-28 19:32:57 EST
(In reply to comment #18)
> I just added a comment in 295021. 295021 should have not a lot of impact on
> this defect.
> 
> AFAIK if users have configured to use both socks and HTTP proxy that the
> communication should use both and SOCKS would be first. 
> 
> If I follow the scenario correctly the changes cause ECF to use the credentials
> provided by IProxyService related to the SOCKS proxy so that overrides
> Authenticator.setDefault() globally and without changing it back. As a
> consequence subsequent communication will get past the proxy. 
> 
> But it appears that ECF would then also not use the HTTP proxy configured. Am I
> wrong?

You are correct. The SocksSocketImpl that's used by ECF will use the SOCKS
server configured in Eclipse, period--even though the Update Manager site is specified as an http URL. Thus, ECF cannot, and should not, try to use the http proxy, as that conflicts with what the underlying socket code is doing.

I'm not sure I answered your question, but just keep the questions coming if
you need more/different insight.
Comment 22 Henrich Kraemer CLA 2010-01-28 20:26:49 EST
> You are correct. The SocksSocketImpl that's used by ECF will use the SOCKS
> server configured in Eclipse, period--even though the Update Manager site is
> specified as an http URL. 

Hi John,

I am not so sure about this. 

Here is what I would expect:
Without your changes the ECF code will call org.eclipse.ecf.provider.filetransfer.httpclient.HttpClientFileSystemBrowser.setupProxy(Proxy) where the Proxy is of type HTTP. As a consequence it will *not* call 
JREProxyHelper.setupProxy(Proxy) with the socks proxy information. 
This means that the startup authenticator remains active which opens the dialog you have attached. 

But what is unclear to me is why the created socket (your proof attachment) uses socks in this case. I will have to debug this. 

Regarding the repro steps in comment 4. On 'Network Connections' preference page when I change the Active Provider to 'Manual' all types have a check mark in the UI even if they are empty. Did you fill in any information there beyond the SOCKS proxy and its authentication information?
Comment 23 John Cortell CLA 2010-01-29 09:20:14 EST
(In reply to comment #22)
> Here is what I would expect:
> Without your changes the ECF code will call
> org.eclipse.ecf.provider.filetransfer.httpclient.HttpClientFileSystemBrowser.setupProxy(Proxy)
> where the Proxy is of type HTTP. As a consequence it will *not* call 
> JREProxyHelper.setupProxy(Proxy) with the socks proxy information. 
> This means that the startup authenticator remains active which opens the dialog
> you have attached. 

Henrich, note that the dialog I attached is an authentication dialog for SOCKS, not HTTP. You're saying that's expected behavior even though ECF is trying to connect to an HTTP URL?

> 
> But what is unclear to me is why the created socket (your proof attachment)
> uses socks in this case. I will have to debug this. 

I tried all this again with the latest: 3.6M4 and the latest ECF sources from HEAD. I'm seeing the same exact behavior. I've done some more stepping in the debugger and I can provide a little more insight into the proof attachment.

If I have only an HTTP proxy configured in Eclipse, the call to

   sel.select(uri).iterator();

in SocksSocketImpl returns a list (iterator) that has one element: a Proxy of type DIRECT. Things proceed fine in this case.

If I have an HTTP and SOCKS proxy configured in Eclipse, the same call returns a list that has one element: a Proxy of type SOCKS

The selector is an instance of sun.net.spi.DefaultProxySelector and unfortunately, it doesn't have debug information, so I can't step through it and tell you why it's returning a SOCKS proxy.

> 
> Regarding the repro steps in comment 4. On 'Network Connections' preference
> page when I change the Active Provider to 'Manual' all types have a check mark
> in the UI even if they are empty. Did you fill in any information there beyond
> the SOCKS proxy and its authentication information?

Yes. I filled in the information for all three checked proxies (http, https, socks) including the userid and password. The Proxy bypass table has localhost and 127.0.0.1 with the Provider value "Manual". I did not add those; they automatically appeared.
Comment 24 John Cortell CLA 2010-01-29 09:38:54 EST
Created attachment 157623 [details]
runtime workbench console output

In my development workbench's Console view, I end up with this. Note that it represents output from the runtime workbench (the one I'm debugging).
Comment 25 John Cortell CLA 2010-01-29 09:39:41 EST
I've attached some interesting output I'm seeing in the development workbench's Console view. It might provide a clue as to what's going wrong.
Comment 26 Henrich Kraemer CLA 2010-01-29 13:02:14 EST
(In reply to comment #25)
> I've attached some interesting output I'm seeing in the development workbench's
> Console view. It might provide a clue as to what's going wrong.

Here is my analysis of what is happening:

When Eclipse starts up after having set manual proxy preferences it will initialize system properties to match these.

See attached stack trace StackTraceInitializeSystemProperties.txt.

As a consequence if the preferences have a SOCKS proxy the java socket connection code connects to a SOCKS server.
The code that initiates the connection could avoid this by specifying a direct connection. However this would require the code to use a 1.5 API.

The current ECF HttpClient based code will never actively select a SOCKS proxy because the call to IProxyService.select(url) uses the target URL. 

Looking at the CVS history of AbstractFileSystemBrowser the switch to select was made due to bug 268844 and bug 268758.
Comment 27 Henrich Kraemer CLA 2010-01-29 13:05:15 EST
Created attachment 157652 [details]
Attachment to comment 26

Notice in addition to what is described in comment 26, that the entries described in comment 25 are caused by the method ProxyType.verifySystemPropertyEquals on top of this stack trace.
Comment 28 John Cortell CLA 2010-01-29 13:17:07 EST
(In reply to comment #26)
> Here is my analysis of what is happening:
> ...

Nice. That's consistent with what I've reported. You found the missing detail that fully explains the observed behavior in the code. 

So, it seems to me that my patch would be a suitable solution until core Eclipse starts requiring 1.5, no?
Comment 29 Henrich Kraemer CLA 2010-01-29 13:38:45 EST
(In reply to comment #23)
> Henrich, note that the dialog I attached is an authentication dialog for SOCKS,
> not HTTP. You're saying that's expected behavior even though ECF is trying to
> connect to an HTTP URL?
> 

I am not sure what is expected. I conducted my own experiments. According to
these:
- Firefox 3.5.7 uses only the HTTP proxy if a SOCKS is also specified in manual
settings. When no HTTP proxy is specified the SOCKS proxy is used.
- IE 6 does not seem to use SOCKS for HTTP connection even if it is the only
proxy specified.
http://en.flossmanuals.net/CircumventionTools/ConfiguringSocksProxies lets me
think perhaps it uses SOCKS for other kinds of traffic such as streaming video.

It is unclear whether it is more desirable to follow the Firefox approach or to
instead use a chain of proxies ECF -> SOCKS -> HTTP-PROXY -> <target-web
server> if both are specified. 

Based on comment 27, it appears to me that IProxyService(from the
org.eclipse.core.net plugin) favors the chain of proxy approach. 
core.net also allows to plugin an authenticator. 
core.net also supports an extension point for the authenticator. It also
provides an implementation of that which pops up the dialog attached previously
(without using the credentials provided in manual proxy settings).

It seems to me that ideally the core.net authenticator would be improved to:
1. use provided credentials
2. allow plugins to provide additional credentials for certain connections.

In the absence of this I would suggest that the setupProxies code will in
addition to calling IProxyService.select(target-url) will also call 
IProxyService.select(socks://<host of target-url>) and call setupProxy for that
as well.
Comment 30 Henrich Kraemer CLA 2010-01-29 13:40:28 EST
(In reply to comment #28)
 
> So, it seems to me that my patch would be a suitable solution until core
> Eclipse starts requiring 1.5, no?

Almost but not quite see previous comment. John can you take a stab at this?
Comment 31 John Cortell CLA 2010-01-29 13:58:35 EST
(In reply to comment #30)
> Almost but not quite see previous comment. John can you take a stab at this?

Henrich, I fear that I may be out of my element and probably not the best to attempt this. There are concepts here that I'm not familiar with. I can give it a shot, but I would need further direction from you. I don't know if that will end up taking more or less of your time compared to you actually coding the fix. You decide. If you choose to mentor me on this fix, then we should probably take the detailed discussions off-list to reduce cluttering this entry further.
Comment 32 Henrich Kraemer CLA 2010-01-29 20:02:29 EST
Created attachment 157683 [details]
Untested patch

I have been having trouble to even smoke test this patch due to an (unrelated) error when bringing this up in my runtime workbench. I am hoping that it a problem specific to my workspace.
Unfortunately I am running out of time here. The attached patch is what I had in mind in comment 29.
Perhaps John can test whether this works at all and with different proxy types such as only SOCKS, only HTTP and both.

HTH Henrich
Comment 33 John Cortell CLA 2010-01-29 20:09:12 EST
You bet. I'll test it tomorrow and let you know.
Comment 34 John Cortell CLA 2010-02-01 10:56:19 EST
Henrich, with an addition of 

   getHostConfiguration().setProxyHost(null);

to HttpClientFileSystemBrowser.setupProxy(Proxy) in the case of SOCKS, your patch works great  (tried HTTP only, SOCKS only, and both)
Comment 35 John Cortell CLA 2010-02-15 10:19:28 EST
(In reply to comment #34)
> Henrich, with an addition of 
> 
>    getHostConfiguration().setProxyHost(null);
> 
> to HttpClientFileSystemBrowser.setupProxy(Proxy) in the case of SOCKS, your
> patch works great  (tried HTTP only, SOCKS only, and both)

After extensive sideline discussions with Henrich, we are in agreement that my suggested adjustment to his fix is misguided. Henrich's fix supports chaining of proxies. I.e., if you have both a SOCKS and HTTP server configured in Eclipse, an HTTP request through ECF should use a SOCKS connection that connects to the HTTP proxy. That doesn't work for me, but that's just because my company's SOCKS server doesn't accept the request to attach to a machine behind the firewall. In reality, I don't have a need to have both proxies defined, so the chaining behavior is fine by me. However, we do need someone to test that the chaining works--someone with access to both SOCKS and HTTP proxies, where the former allows connecting to the latter.
Comment 36 Henrich Kraemer CLA 2010-02-15 13:06:48 EST
Thanks to John's help we have a better understanding of what is happening.

We also know that the untested patch works for his SOCKS proxy when only a (manual) SOCKS proxy is specified in the preferences.

As John mentions more testing is needed:
1. Does it work when only a HTTP proxy server is specified in the preferences?
2. Does it work when both SOCKS and HTTP proxy servers are specified where the expectation is that both would be used?
 
In addition there are the following concerns:

1. Possible regressions in the basic URLConnection based ECF providers. 
As mentioned in comment 26 the changes affecting HttpClient based ECF provider (this issue) have been introduced when addressing bug 268844 and bug 268758. From looking at the code it appears that these changes may also affect the URLConnection based ECF providers. But I have not tested this. 
The patch provided earlier are expected to leave the behavior of the URLConnection based ECF provider as is.

2. I am not sure that the chain of proxies approach is in fact wanted and consistent both across ECF providers and possibly other eclipse code using the proxy service. If I am not mistaken the default proxy handling in HttpURLConnection is to use only one proxy server and to give HTTP proxies precedence if defined.
I welcome any guidance the proxy service team can give here.
Comment 37 John Cortell CLA 2010-02-15 14:36:13 EST
(In reply to comment #36)
> 1. Does it work when only a HTTP proxy server is specified in the preferences?

I did test that scenario, and, yes, that works.
Comment 38 John Cortell CLA 2010-02-24 12:52:10 EST
Scott, both Henrich and I have invested a lot of time into this matter. This issue seems to repeatedly lose traction, but I'm determined to avoid seeing Helios come out with this problem. We're talking very basic functionality that's not working. This is a serious issue and one I'll keep trying to get resolved.

Can you please review Henrich's last post and see if we can't make progress on this?
Comment 39 Scott Lewis CLA 2010-02-24 13:03:00 EST
(In reply to comment #38)
> Scott, both Henrich and I have invested a lot of time into this matter. This
> issue seems to repeatedly lose traction, but I'm determined to avoid seeing
> Helios come out with this problem. We're talking very basic functionality
> that's not working. This is a serious issue and one I'll keep trying to get
> resolved.
> 
> Can you please review Henrich's last post and see if we can't make progress on
> this?

Henrich's last comment (comment 36) ended with this:

>I welcome any guidance the proxy service team can give here.

So I guess according to Henrich what we need is input from the proxy API team (is that right Henrich?).  I don't have any answers to his request for guidance myself, since I don't have commit rights on the proxy service, don't know that codebase well, and don't have the resources to investigate it further myself.

Like I said before, I'm willing to move forward on any agreed-to fixes that do not have regressive side effects, and have been tested/shown to fix the issue at hand to your satisfaction.  Henrich is one of the people primarily responsible for this code (along with me), so it's his final decision about what the fix is.
Comment 40 Henrich Kraemer CLA 2010-02-24 17:29:13 EST
(In reply to comment #39)
...
> 
> Henrich's last comment (comment 36) ended with this:
> 
> >I welcome any guidance the proxy service team can give here.
> 
> So I guess according to Henrich what we need is input from the proxy API team
> (is that right Henrich?).

Yes. 

> Henrich is one of the people primarily
> responsible for this code (along with me), so it's his final decision about
> what the fix is.

In comment 26 I noted:
> Looking at the CVS history of AbstractFileSystemBrowser the switch to select
> was made due to bug 268844 and bug 268758.

The change to select was made in response to 268844. This was not a change I made and I am not quite sure how things were working before that change. In particular whether SOCKS or HTTP had priority or a chain of proxies was used. Scott do you know what the historical behavior was?
Comment 41 Scott Lewis CLA 2010-02-24 17:33:19 EST
(In reply to comment #40)
> (In reply to comment #39)
> ...
> > 
> > Henrich's last comment (comment 36) ended with this:
> > 
> > >I welcome any guidance the proxy service team can give here.
> > 
> > So I guess according to Henrich what we need is input from the proxy API team
> > (is that right Henrich?).
> 
> Yes. 


Would someone that is familiar with the platform team players (John A?) add someone that's maintaining/working with the proxy API in Helios release cycle so that Henrich can get input from the proxy API team?  Unfortunately, I don't even know who the players are now.


> 
> > Henrich is one of the people primarily
> > responsible for this code (along with me), so it's his final decision about
> > what the fix is.
> 
> In comment 26 I noted:
> > Looking at the CVS history of AbstractFileSystemBrowser the switch to select
> > was made due to bug 268844 and bug 268758.
> 
> The change to select was made in response to 268844. This was not a change I
> made and I am not quite sure how things were working before that change. In
> particular whether SOCKS or HTTP had priority or a chain of proxies was used.
> Scott do you know what the historical behavior was?

No...I'm sorry...I don't have that history in my head anymore.
Comment 42 Henrich Kraemer CLA 2010-02-24 18:15:18 EST
bug 289933 may not be definitive yet.

But it definitely questions using a chain of proxy approach. It suggests using at most one proxy, in order of priority:
- HTTP
- SOCKS
- direct connection.

If that is the desired behavior then the patch provided in comment 32 should not be used.

I see a problem implementing this which is that when core.net proxy service see a SOCKS proxy server defined it sets up the system properties so that subsequent Java Socket connections will connect to the SOCKS proxy server.  

This could only be switched off by having ECF make socket connects with 1.5 API's that allow to specify a proxy used for an individual connection. Having a 1.5 dependency in ECF would probably not be acceptable.

Alternatively it could change the the system preferences for the time it makes the transfer (or just the connection) so that no SOCKS proxy server would be used. This is also problematic as it has global effects. 

Guidance by the core.net proxy service team would be very welcome on this issue.
Comment 43 John Arthorne CLA 2010-02-25 08:55:00 EST
Szymon, can you help out here? I think the question related to core.net proxy support is in comment #36.
Comment 44 Szymon Brandys CLA 2010-02-25 11:20:43 EST
(In reply to comment #43)
> Szymon, can you help out here? I think the question related to core.net proxy
> support is in comment #36.
Adding Pawel, since he was the last one working on this stuff. I'll ping him to comment ASAP.
Comment 45 Pawel Pogorzelski CLA 2010-03-01 07:46:24 EST
(In reply to comment #42)
> But it definitely questions using a chain of proxy approach.

I agree with you Henrich.

> Having a 1.5 dependency in ECF would probably not be acceptable.

Couldn't the dependency be optional?

> Alternatively it could change the the system preferences for the time it makes
> the transfer (or just the connection) so that no SOCKS proxy server would be
> used. This is also problematic as it has global effects. 

This sounds really scary. I wouldn't consider it.
Comment 46 Henrich Kraemer CLA 2010-03-01 14:50:25 EST
> > Having a 1.5 dependency in ECF would probably not be acceptable.
> 
> Couldn't the dependency be optional?

Scott would that be acceptable? 
To summarize it appears the only solution found for users which have both SOCKS and HTTP proxy configured would be to introduce a 1.5 dependency. Users on older JRE's would have to use the JRE provider instead of HTTPClient based ECF provider.

Simple SOCKS bug: There is also a simple SOCKS bug in ECF's HttpClient provider, where users have only one proxy defined in their settings. In this instance the ECF provider does not provide the credentials by replacing the java.net.Authenticator for the transfer. Notice that this approach 

>> Having a 1.5 dependency in ECF would probably not be acceptable.
>Couldn't the dependency be optional?

Perhaps core net would want to encapsulate this 1.5 dependency which would allow an individual socket connection to be made without going through the SOCKS server as specified in proxy settings. 
Pawel, does this make sense to you?


>> Alternatively it could change the the system preferences for the time it makes
>> the transfer (or just the connection) so that no SOCKS proxy server would be
>> used. This is also problematic as it has global effects. 
>
>This sounds really scary. I wouldn't consider it.

Why does core net's authenticator not use manual SOCKS credentials provided by the user in the settings?  
Why does it not manage the SOCKS credentials? 
With manage I mean if no credentials are provided foe example for native authenticator would remember what is typed, possible allowing to persist the value.

This might be the proper resolution of the mentioned Simple SOCKS bug. The current ECF code which replaces the java.net Authenticator for a the transfer period raises similar concerns as changing system preferences. 
Pawel, do you agree with this?
Comment 47 Scott Lewis CLA 2010-03-01 15:08:46 EST
(In reply to comment #46)
> > > Having a 1.5 dependency in ECF would probably not be acceptable.
> > 
> > Couldn't the dependency be optional?
> 
> Scott would that be acceptable? 

Probably not.  Since this code is used in the Eclipse Platform, we/ECF are subject to the rules of the Platform WRT execution environment/dependencies.  That is, the Platform dictates that the file transfer part of ECF must work on 1.4.  If left only up to us/ECF we would have no problems with introducing a dependency on JRE 1.5 code, but for this code it's not up to us.

> To summarize it appears the only solution found for users which have both SOCKS
> and HTTP proxy configured would be to introduce a 1.5 dependency. Users on
> older JRE's would have to use the JRE provider instead of HTTPClient based ECF
> provider.

Moving away from the HttpClient provider is a problem...because one of the primary reasons we moved to the HttpClient provider in the first place was because JRE provider was somewhat buggy (resulting in hangs during file transfer)...particularly on older (pre 1.5) vms.

I may be out of date about the platform's execution environment requirements...i.e. maybe things have changed and 1.5 dependencies can be added.  Would someone from the platform team please comment on this? (e.g. John A, Syzmon, etc?)...or put someone else on this bug who could comment on our/ECF introducing a 1.5 dependency in the ECF filetransfer code as a solution to this problem?
Comment 48 Pawel Pogorzelski CLA 2010-03-02 05:05:17 EST
(In reply to comment #46)
> Why does core net's authenticator not use manual SOCKS credentials provided by
> the user in the settings?  
> Why does it not manage the SOCKS credentials?

I found some information regarding the matter, see bug 196780 comment 8 and subsequent.

> This might be the proper resolution of the mentioned Simple SOCKS bug. The
> current ECF code which replaces the java.net Authenticator for a the transfer
> period raises similar concerns as changing system preferences. 
> Pawel, do you agree with this?

I do.
Comment 49 Pawel Pogorzelski CLA 2010-03-02 05:10:59 EST
(In reply to comment #47)
> (In reply to comment #46)
> > > > Having a 1.5 dependency in ECF would probably not be acceptable.
> > > 
> > > Couldn't the dependency be optional?
> > 
> > Scott would that be acceptable? 
> 
> Probably not.  Since this code is used in the Eclipse Platform, we/ECF are
> subject to the rules of the Platform WRT execution environment/dependencies. 
> That is, the Platform dictates that the file transfer part of ECF must work on
> 1.4.

By optional dependency I meant forwarding calls to 1.5 API via reflection when those classes are available. It does not prevent the code from running on 1.4 class library and solves the problem for other users. If there's no other reasonable way to handle the issue this way we could reduce the scope of it.
Comment 50 Henrich Kraemer CLA 2010-03-08 01:10:16 EST
(In reply to comment #48)

Pawel,

It is not clear to me what you plans are.
Are you planning to make changes so that the proxy service would manage SOCKS proxy credentials?

If proxy credentials are not accepted, the code that makes the connection should probably be responsible to realize this and then clear the credentials. On the next attempt the proxy service would then prompt again.
Comment 51 Henrich Kraemer CLA 2010-03-08 01:11:29 EST
> 
> By optional dependency I meant forwarding calls to 1.5 API via reflection when
> those classes are available. It does not prevent the code from running on 1.4
> class library and solves the problem for other users. 

Scott, would such an optional 1.5 dependency be acceptable?

> If there's no other
> reasonable way to handle the issue this way we could reduce the scope of it.
Comment 52 Pawel Pogorzelski CLA 2010-03-08 06:26:26 EST
(In reply to comment #50)
> Are you planning to make changes so that the proxy service would manage SOCKS
> proxy credentials?

This is a separate issue and is being tracked by bug 295021. I'll investigate problems with provided credentials usage shortly.

As I understand the issue discussed here is usage/priority of SOCKS/HTTP settings. Seems like proxy component can't do noting in this matter since we can't stop propagating user defined proxies to system preferences at it would brake clients (this is the only way to provide proxy settings to JCL's connection handlers in JVMs older than 1.5). And once the settings are defined and propagated to system properties and proxy API clients our jurisdiction ends.
Comment 53 John Cortell CLA 2010-03-08 08:22:44 EST
(In reply to comment #52)
> As I understand the issue discussed here is usage/priority of SOCKS/HTTP
> settings. 

Indeed a lot of the discussion has been on how to handle the case of both a SOCKS and an HTTP proxy being defined in the proxy preferences. But this issue also is about the fact that one cannot successfully use the Update Manager even when there is only a SOCKS proxy defined. See comment #0
Comment 54 Scott Lewis CLA 2010-03-08 09:26:29 EST
(In reply to comment #51)
> > 
> > By optional dependency I meant forwarding calls to 1.5 API via reflection when
> > those classes are available. It does not prevent the code from running on 1.4
> > class library and solves the problem for other users. 
> 
> Scott, would such an optional 1.5 dependency be acceptable?

Yes.
Comment 55 John Cortell CLA 2010-03-08 09:33:40 EST
(In reply to comment #54)
> (In reply to comment #51)
> > > 
> > > By optional dependency I meant forwarding calls to 1.5 API via reflection when
> > > those classes are available. It does not prevent the code from running on 1.4
> > > class library and solves the problem for other users. 
> > 
> > Scott, would such an optional 1.5 dependency be acceptable?

Anything that encourages people off a JRE that is 8 years old and onto one that's 4+ years old sounds good to me :-) All those platform developers itchy to use Java 5 features would probably agree with me.
Comment 56 Scott Lewis CLA 2010-03-08 09:50:02 EST
(In reply to comment #55)
> (In reply to comment #54)
> > (In reply to comment #51)
> > > > 
> > > > By optional dependency I meant forwarding calls to 1.5 API via reflection when
> > > > those classes are available. It does not prevent the code from running on 1.4
> > > > class library and solves the problem for other users. 
> > > 
> > > Scott, would such an optional 1.5 dependency be acceptable?
> 
> Anything that encourages people off a JRE that is 8 years old and onto one
> that's 4+ years old sounds good to me :-) All those platform developers itchy
> to use Java 5 features would probably agree with me.

Yes.  If you want to push this, please raise with the platform pmc.  We (ECF) are just subject to the restriction and don't currently have any real say in it (I am not even on the platform pmc, for example).

Doing it with reflection does raise additional difficulties for us...e.g. testing, so I would personally rather not have this restriction...and be allowed to use j5 api directly.
Comment 57 Pawel Pogorzelski CLA 2010-03-11 05:28:56 EST
See bug 288587 for a discussion about using stored SSH password.
Comment 58 John Cortell CLA 2010-03-29 11:00:55 EDT
This issue seems to have stalled again.
Comment 59 Scott Lewis CLA 2010-03-30 15:16:06 EDT
(In reply to comment #58)
> This issue seems to have stalled again.

Hi John.

Thanks for the gentle reminder about this issue and associated bug 295021.

Since your comment 58, I've been corresponding directly with Henrich Kraemer.  From his and my perspective this issue hasn't stalled and he has every intention of cooperatively addressing it prior to Helios release.

We have already passed the deadline for API changes to ECF filetransfer (which are subject to the timing/rules for the Equinox p2/Eclipse platform API freeze), but this should not be a problem for this/these issues, as I don't think either issue has any ECF API implications.  

I can't comment directly about the proxy API, however...Pavel or Syzmon will have to comment about that...but I don't *think* any API changes are necessary for addressing this bug.  If they are, then I would request that they be made as soon as possible so that they can make it into Helios.

So for this bug and 295021 I can/will accept ECF filetransfer patches up until Helios RC1, which is Friday, May 21, 2010.  After that, the threshold for applying/testing patches gets rather high.  I'm hopeful that with the cooperation of Pavel and Syzmon (responsible for the proxy API in platform), and Henrich (responsible for necessary additions/changes in ECF apache httpclient filetransfer provider) we should be able to address these issues during April 2010.  If needed/desired, I can/will provide the some amount of coordination, as well as help with regression testing...although I do not have an environment to test the actual fix...hopefully you/John will be able to assist in testing in your environment.

Please everyone let me know on this bug if my assumptions/assertions are incorrect (about proxy api, needed additions/changes, etc)...or if more/other is needed directly from me so that this bug and 295021 are addressed in time for Helios.  Thanks to all in advance for your cooperation.
Comment 60 John Cortell CLA 2010-03-30 15:45:21 EDT
(In reply to comment #59)
Thanks for the update, Scott. I'm glad to hear there's activity going on behind the scenes. Again, given the seriousness of the issue, I've been trying to keep this alive as much as I can without being obnoxious :-) I'll definitely help test any solution with my environment.

John
Comment 61 Henrich Kraemer CLA 2010-04-02 00:21:19 EDT
The proper solution for this and bug 295021 requires managing the single java.net.Authenticator (per JRE process). This must include taking on responsibility to manage the credentials, so that user is prompted only if there are no existing credentials or the ones previously provided were not accepted by the server. This may require cooperation of the transfer code such as ECF, for example to tell the new API's if an authentication failed. This evolved proxy API would also handle user cancellation of prompts. 

This is a larger effort and I do not expect this to happen in 3.6, but there should be an enhancement request opened for this in my mind.

In the current state of the eclipse net authenticator (org.eclipse.ui.internal.net.auth.NetAuthenticator) is not usable as it prompts 
users repeatedly even if users provided the correct credentials to the prompt dialog or prior in the preference page. 

ECF's URLConnection based provider does set its own Authenticator and has done this for a long time. Judging from the history of class JREProxyHelper this was done since at least 12/2007. 

To fix bug 295021 it seems inevitable that ECF's HttpClient based provider will also have to set the ECF Authenticator. 

The issue that ECF's HttpClient provider is not working even if only a SOCKS proxy is used is caused by it not recognizing the SOCKS proxy and therefore it does not set its own authenticator. This should be changed. 

The desired proxy usage for an HTTP transfers is to use only a single proxy or a direct connection. If preferences are set for both HTTP and SOCKS are set then HTTP should be used. 

But implementing this requires a Java 1.5 dependency. I am assuming that the PMC guidance means that avoiding this dependency is more important. 
Without the 1.5 calls the best that can be achieved is to have ECF give the SOCKS proxy priority if both an HTTP and SOCKS proxy are defined.  
The impact is that:
1. Users who only use one or the other proxy have no issues as long as they only configure one or the other in the preferences. 
2. Users who use socks proxy for something besides ECF and want to use HTTP proxy for ECF's transfers could NOT use the HttpClient based ECF provider and would have to revert back to the ECF's URLConnection based provider.

To summarize:
I. A long term solution involves evolving eclipse net APIs to handle authenticator properly. An enhancement bugzilla should be filed.

II. The short term changes proposed are: 
a. Set ECF authenticator if SOCKS proxy is in preferences. 
b. If both HTTP and SOCK proxies are defined use SOCKS proxy only.

I will look into making the changes described in II but this may take some time.
Comment 62 Henrich Kraemer CLA 2010-04-22 14:47:54 EDT
Created attachment 165828 [details]
Proposed patch

I implemented the short term changes described in previous comment in the attached patch. These are:

1. Replace java.net.Authenticator with ECF authenticator when SOCK proxy is in the preferences. This is happening in both URLConnection based ECF provider and HttpClient based ECF provider. 

I made a change in JREProxyHelper that this always happens even if ECF does not have credentials for the socks proxy. This avoids the issue described in bug 295021 where a dialog keeps popping up even when good credentials are provided. Instead of the team net provider the authentication now fails (see testing notes below). 

2. Fixed an issue were providing bad proxy credentials would keep the HttpClient based ECF transfer in an endless retry look. See changes in HttpClientProxyCredentialProvider.

3. Give socks proxy priority in HttpClient based ECF provider. See previous comment on why this is not using the desired priority to give the HTTP proxy the higher priority instead. 

Also:
- Created centralized class ProxySetupHelper used in both ECF providers. 

Testing notes:
General Steps:
Step 1. Started SDK
Step 2. Set proxy preferences according to specific test in Window|Preferences.. On page 'Network Connections' select:
Active Provider: Manual and set the values.
Leave the preferences dialog.
Step 3: Goto Help|Install New Software, and see whether page is properly populated. I verified this by selecting site 'HeliosH - http://download.eclipse.org/releases/helios'

I. Test cases using an HTTP proxy only which requires authentication
1. Provide correct credentials in the fields provided in Step 2.
Result: Step 3 succeeds. No errors reported.

2. Provide no credentials in step 2
Result Step 3 fails with an Error dialog.
The test says:
HTTP Proxy Authentication Required: http://download.eclipse.org/releases/helios/content.xml
Proxy Authentication Required

P2 could consider adjusting their code to prompt for credentials in this case. 

3. Provide invalid credentials in step 2
Result: As with no credentials.

II. Test cases using a SOCKS proxy only which requires authentication
Similar to cases in I. However error dialog is different:
Unable to read repository at http://download.eclipse.org/releases/helios/content.xml.
SOCKS : authentication failed


III. Specify both SOCKS and PROXY settings.
Tested that this works an that SOCK server is used in this case. 
Behavior is as observer in II.
Comment 63 Scott Lewis CLA 2010-04-22 15:34:32 EDT
(In reply to comment #62)
> Created an attachment (id=165828) [details]
> Proposed patch
> 
> I implemented the short term changes described in previous comment in the
> attached patch. These are:

<details deleted>...see comment 62 for these details.

1) I want to thank Henrich for his efforts on this bug...both up to this point and going forward.  You have at least one beer headed your way from me.

2) I was able to apply the patch to HEAD without problems, and after visual inspection the changes look technically good to me.

3) I (Scott) will test this patch in all environments to which I have access for regression, but I need to make clear that I personally do *not* have access to the proxy environment that needs to be tested (i.e. socks proxy and/or http proxy), so need to request help with testing...hopefully from John (bug author) but also from any others on this bug that are able to help.  This testing should/must be done as quickly as possible, as we are very close to M7.  If you have access to an environment relevant to this issue, please consider helping out with testing this patch.

4) There is one change flagged by API tools that results in an API breakage and I need input from p2 folks about what they want to do about it.  The breakage being reported is the removal of a method:

org.eclipse.ecf.provider.filetransfer.browse.AbstractFileSystemBrowser.selectProxyFromProxies(String, IProxyData[]) has been removed

This was a protected method in abstract super class AbstractFileSystemBrowser.  How would p2/platform pmc like to handle this?  This protected method in super class is indeed API, and the changes for this bug do indeed imply it's removal.

Should we get an API freeze waiver for this change so it can get into Helios?  I believe the chance of breaking existing clients with this change is *very small*...and I personally know of no clients that would be broken by this change.  But it is possible that it will break clients...and I would like this change to go into Helios...assuming that it tests out with no problems.

To summarize

a) John and others on this bug please help with testing as quickly as possible in relevant environments
b) Pascal, yjob, and/or pmc...what would you/pmc like to do WRT the api breakage and the handling for Helios?

Thanks to all for cooperation.
Comment 64 John Cortell CLA 2010-04-22 15:37:50 EDT
Cool! I will test this out today.
Comment 65 John Cortell CLA 2010-04-22 16:51:55 EDT
I'm glad to report the patch fixes the following key scenarios:

1. only SOCKS proxy defined, with userid and password
2. only HTTP proxy defined (mine doesn't require authentication)
3. both SOCKS and HTTP proxies defined

My definition of "works" is that I can bring up the Update Manager, select the update site 'http://download.eclipse.org/releases/helios', and the dialog's table fills up with the categories it retrieved from the update site. In all three cases, there is no need for an authentication prompt, and none shows up. Also note that I tried each scenario in isolation, only. That is, I launch Eclipse, set up the network proxies, then go to the Update Manager. I don't ever go back and change the proxy setting and try the Update Manager again. Not sure if that will work, or if it should work. I can try it if you guys want, but the isolation scenarios working is good enough for me.

Unfortunately, authentication prompting doesn't work. In scenario #1, I had the userid and password set with the proxy definition (and again, that works fine). But I decided to try some scenarios where I didn't enter the user/password with the proxy definition. The following two scenarios don't work:

4. only SOCKS proxy with 'requires authentication' deactivated
5. only SOCKS proxy with 'requires authentication' activated, userid entered (dialog requires it), but password not entered.

For sure, I expect #5 to prompt me. I also expect #4 to prompt me, but I'm not certain if it's supposed to. Regardless, in both cases, I get the error dialog (screenshot attached).

Henrich, I owe you a beer, also. Maybe two...an extra one for all the nagging.
Comment 66 John Cortell CLA 2010-04-22 16:52:37 EDT
Created attachment 165852 [details]
Error dialog mentioned in comment 65
Comment 67 John Cortell CLA 2010-04-22 16:59:38 EDT
I should also mention that I'm still seeing the following type of messages in stdout/stderr:

!MESSAGE System property socksProxyHost is not set but should be yadayada
!MESSAGE System property socksProxyPort is not set but should be 1234

Don't know if the fix intends to address that, or if the messages are indicative of a problem. Just wanted to point it out.
Comment 68 Henrich Kraemer CLA 2010-04-22 17:50:54 EDT
> 4) There is one change flagged by API tools that results in an API breakage and
> I need input from p2 folks about what they want to do about it.  The breakage
> being reported is the removal of a method:
> 
> org.eclipse.ecf.provider.filetransfer.browse.AbstractFileSystemBrowser.selectProxyFromProxies(String,
> IProxyData[]) has been removed
> 
> This was a protected method in abstract super class AbstractFileSystemBrowser. 


Sorry I did not mentioned that there was an API change, but I have been working on this for a longer time and forgot this (and have also not setup the API baseline). 

While one could bring it back it would still not be called by the HttpClient ECF provider if user configures socks and http proxy, where socks is now prioritized. Not sure this would be worth the effort.
Comment 69 Henrich Kraemer CLA 2010-04-22 18:05:52 EDT
(In reply to comment #65)

Thanks John for testing this. 

> I can try it if you guys want,
> but the isolation scenarios working is good enough for me.

Changing properties does not work well for me. But this is not affected by these changes.

> Unfortunately, authentication prompting doesn't work. 

It works in that you get an error message about this. I think this is much better than before, where you have to keep entering credentials.

This is a consequence of the fact that now the caller is responsible in prompting if needed. 
As mentioned with the phrase 'P2 could consider adjusting their code to prompt for credentials in this case.' in comment 62 I think P2 could do the prompting for this case similar to how P2 would prompt for non-proxy credentials. 

But I am not sure this is truly required. I think a meaningful error message already helps a lot. 
Prompting might be needed if one relies on native proxy provider which does not provide credentials. 
To address this without having the caller be responsible for the prompting the 'proper solution' mentioned in comment 61 is needed. Unfortunately this is out of scope for this patch.
Comment 70 John Cortell CLA 2010-04-22 18:22:01 EDT
(In reply to comment #69)
> > Unfortunately, authentication prompting doesn't work. 
> 
> It works in that you get an error message about this. I think this is much
> better than before, where you have to keep entering credentials.

Amen to that! (Entering the credentials to no avail, no less).

> This is a consequence of the fact that now the caller is responsible in
> prompting if needed. 
> As mentioned with the phrase 'P2 could consider adjusting their code to prompt
> for credentials in this case.' in comment 62 I think P2 could do the prompting
> for this case similar to how P2 would prompt for non-proxy credentials. 
> 
> But I am not sure this is truly required. I think a meaningful error message
> already helps a lot. 

I agree. I'm sure there are people who are uncomfortable storing passwords in applications, so they'll be the ones to cry foul on this one, but I'm not that paranoid, so no complaints from me.

> Prompting might be needed if one relies on native proxy provider which does not
> provide credentials. 
> To address this without having the caller be responsible for the prompting the
> 'proper solution' mentioned in comment 61 is needed. Unfortunately this is out
> of scope for this patch.

The fact that I can now use P2 from behind the firewall with SOCKS, and that having dual proxies (SOCKS and HTTP) defined no longer causes a problem...that's a HUGE improvement. Everything else is icing on the cake as far as I'm concerned.
Comment 71 John Arthorne CLA 2010-04-22 22:50:43 EDT
(In reply to comment #63)
> This was a protected method in abstract super class AbstractFileSystemBrowser. 
> How would p2/platform pmc like to handle this?  This protected method in super
> class is indeed API, and the changes for this bug do indeed imply it's removal.
> 
> Should we get an API freeze waiver for this change so it can get into Helios? 
> I believe the chance of breaking existing clients with this change is *very
> small*...and I personally know of no clients that would be broken by this
> change.  But it is possible that it will break clients...and I would like this
> change to go into Helios...assuming that it tests out with no problems.

As the ECF project lead it is your call to decide if the breaking API change should be approved. However, from a quick look at the patch it doesn't look to me like the deletion is actually necessary. The method has just been moved to the ProxySetupHelper class. Couldn't the old method be left behind in case someone is calling it? It looks like it was just removed because you aren't calling it yourself anymore. The implementation of the method in the new location looks identical to the old one. In fact its javadoc still says subclasses may override even though it is now private and static...
Comment 72 Scott Lewis CLA 2010-04-22 23:31:09 EDT
(In reply to comment #71)
<stuff deleted>
> 
> As the ECF project lead it is your call to decide if the breaking API change
> should be approved. However, from a quick look at the patch it doesn't look to
> me like the deletion is actually necessary. The method has just been moved to
> the ProxySetupHelper class. Couldn't the old method be left behind in case
> someone is calling it? It looks like it was just removed because you aren't
> calling it yourself anymore. The implementation of the method in the new
> location looks identical to the old one. In fact its javadoc still says
> subclasses may override even though it is now private and static...


If Henrich agrees to a refactoring that leaves the method intact (and maybe just calls out to the new method?)  then it seems to me that's the best/easiest way to go.
Comment 73 Henrich Kraemer CLA 2010-04-23 02:13:41 EDT
(In reply to comment #72)
> 
> If Henrich agrees to a refactoring that leaves the method intact (and maybe
> just calls out to the new method?)  then it seems to me that's the best/easiest
> way to go.

I will do that.
Comment 74 Henrich Kraemer CLA 2010-04-23 18:19:32 EDT
Created attachment 165981 [details]
Proposed patch readded removed API method

Here is the revised patch. I briefly smoke tested it.
Comment 75 Scott Lewis CLA 2010-04-23 19:26:55 EDT
(In reply to comment #74)
> Created an attachment (id=165981) [details]
> Proposed patch readded removed API method
> 
> Here is the revised patch. I briefly smoke tested it.

I've examined, applied this patch and released to HEAD.  Thanks much Henrich for the updated patch to deal with the API change.

Before resolving this bug I would like some time to go by for regression testing to take place...among folks with various proxy configurations.  I cannot do much if any regression testing myself of the proxy changes...as I don't have access to a proxy server, but if those on this bug with access to a proxy would please check this change out for regression I would be most appreciative.

Also, I am going to try to get this into platform M7 next week if at all possible.
Comment 76 Pascal Rapicault CLA 2010-04-24 08:50:42 EDT
This is an awesome news again showing the effort of a community!
A special thank to Scott and Heinrich for not giving up on this and providing once again something that we will be able to put in production. A big Thank You!

As for the API changes, I'm unfortunately not in a position to get them approved since I'm not a PMC. You will have to open a bug in ECF and cc an RT PMC member asking for the approval of the change (feel free to cc me). From my point of view, the API breakage is much compensated by the value to all our users.
Comment 77 Pascal Rapicault CLA 2010-04-24 08:55:46 EDT
I will further test these issues during next week test pass. 
Scott, I have recently learnt from a colleague at Sonatype that ccProxy can be used (http://www.youngzsoft.net/ccproxy/). It runs on a windows box, is *very* easily setup and is free for less than 3 users.
Comment 78 Scott Lewis CLA 2010-05-05 20:21:02 EDT
John C, have you (and/or others with environments that show this issue) had occasion to test out Helios M7 (which has this fix?).  

If the patch/M7 works for you, I would like to resolve this bug.  Should I also resolve bug 295021?  

NOTE:  As described in comment 61, there are some longer term changes implied by this issue for the core proxy API.  We should open bug/enhancement requests as appropriate.  Would someone from the platform team please open bugs against the core proxy API...so that the history on this bug (summarized in comment 61) is meaningfully represented to the core proxy API developers in the next release cycle?

Thanks to everyone for cooperation and persistence in getting these issues addressed for Helios...but particular thanks go to ECF contributor Henrich Kraemer.  Assuming everything is working as per previous tests, I'm ready to buy that beer Henrich...so let me know when we can hit the Lucky Lab (or wherever) together.
Comment 79 Pawel Pogorzelski CLA 2010-05-06 07:00:09 EDT
(In reply to comment #78)
> NOTE:  As described in comment 61, there are some longer term changes implied
> by this issue for the core proxy API.  We should open bug/enhancement requests
> as appropriate.

I'm not sure what exactly is needed from proxy API. A part of comment 61 mentiones providing ECF Authenticator, another one says about enhancing the current core.net authenticator. Please clarify it.
Comment 80 John Cortell CLA 2010-05-06 09:45:44 EDT
(In reply to comment #78)
> John C, have you (and/or others with environments that show this issue) had
> occasion to test out Helios M7 (which has this fix?).  

I just gave it a try and the three critical scenarios (see comment # 65) work in 3.6M7. Thanks!!!
Comment 81 Scott Lewis CLA 2010-05-07 15:09:23 EDT
(In reply to comment #79)
> (In reply to comment #78)
> > NOTE:  As described in comment 61, there are some longer term changes implied
> > by this issue for the core proxy API.  We should open bug/enhancement requests
> > as appropriate.
> 
> I'm not sure what exactly is needed from proxy API. A part of comment 61
> mentiones providing ECF Authenticator, another one says about enhancing the
> current core.net authenticator. Please clarify it.


Henrich will need to respond to this clarification request.  If he is unable to respond directly on this bug, please contact him directly for clarification via email or whatever means desired.  

Please do create bugs/enhancements for the proxy API as appropriate.  I want to make sure that these longer term needs don't get lost.

Resolving this bug.  Thanks to all involved.
Comment 82 Henrich Kraemer CLA 2010-05-09 19:23:35 EDT
(In reply to comment #79)
> I'm not sure what exactly is needed from proxy API. A part of comment 61
> mentions providing ECF Authenticator, another one says about enhancing the
> current core.net authenticator. Please clarify it.

Pawel,

Currently ECF replaces the java.net.Authenticator with its own ECF authenticator. This is more of a workaround than a desirable solution. 

A improved Eclipse proxy service should enable ECF to remove this workaround and instead integrate with an authenticator provided by the proxy service. 

A possibility might be that the proxy service would provide the java.net.Authenticator and define a protocol for plugins to register plugin authenticators with an API perhaps via thread locals. 

The proxy service would no longer by itself ask for credentials. Instead the registered plugin authenticators could do this, although I could also see authenticators itself to not prompt, but rather let the connection so that the caller can prompt. It would seem useful if the proxy service would provide some standard prompting UI, but I am not sure it is required. 

The proxy service should also set the rules how to manage proxy credentials. There are some different scenarios:
1. Situations where there is no place to provide credentials in advance such as for native providers. 
2. None or stale credentials are provided. 
I am not sure this aspect is required but otherwise different plugins may solve this in different ways. Proper handling of cancel is important as well.

A problem is to enable making a socket connection which favors HTTP versus SOCKS proxy when both are defined. This may require a 1.5 dependency. Providing a common mechanism as part of the proxy services might make sense for this as well.
Comment 83 Pawel Pogorzelski CLA 2010-05-10 05:13:27 EDT
(In reply to comment #82)
> A problem is to enable making a socket connection which favors HTTP versus
> SOCKS proxy when both are defined. This may require a 1.5 dependency. Providing
> a common mechanism as part of the proxy services might make sense for this as
> well.

I've opened bug 312224 to track this.
Comment 84 Pawel Pogorzelski CLA 2010-05-10 05:50:40 EDT
(In reply to comment #82)
> Currently ECF replaces the java.net.Authenticator with its own ECF
> authenticator. This is more of a workaround than a desirable solution. 

It's bug 312228.