Bug 257443 - [Net] Add a facility to set a custom proxy provider
Summary: [Net] Add a facility to set a custom proxy provider
Status: ASSIGNED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Team (show other bugs)
Version: 3.5   Edit
Hardware: PC Windows All
: P3 enhancement with 2 votes (vote)
Target Milestone: ---   Edit
Assignee: Platform Team Inbox CLA
QA Contact:
URL:
Whiteboard: API
Keywords:
: 246068 298813 299756 (view as bug list)
Depends on:
Blocks: 276939
  Show dependency tree
 
Reported: 2008-12-03 17:34 EST by Mark A. Ziesemer CLA
Modified: 2014-02-07 04:03 EST (History)
12 users (show)

See Also:
wojciech.galanciak: review? (Szymon.Brandys)


Attachments
Proposed initial patch. (89.58 KB, patch)
2008-12-09 22:20 EST, Mark A. Ziesemer CLA
no flags Details | Diff
initial patch + tests (46.62 KB, patch)
2010-11-19 07:20 EST, Wojciech Galanciak CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark A. Ziesemer CLA 2008-12-03 17:34:58 EST
Build ID: I20081030-1917 (3.5M3)

Steps To Reproduce:
1. Configure manual proxy server settings in IE.
2. Set to use "System proxy configuration" in Eclipse.
3. Verify that correct proxy information can be obtained for connections.
4. Change IE to not use manual proxy server settings, but to "Use automatic configuration script".
5. Verify that script is downloaded from server, and that settings work as expected within IE.
6. Confirm that correct proxy information is no longer available within Eclipse.


More information:
This bug is assuming the 32-bit version of Eclipse, as this isn't working at all with the 64-bit version of Eclipse.  (See bug 257420.)  I only have the 64-bit version of Vista available to test against, so it's not clear if this would also happen with the 32-bit version of Vista.

Also as noted in bug 257420, local PAC files (using a file:// URI) do work correctly with IE, even version 7 despite other reports found on Google.  However, this syntax produces an error in the Eclipse error log:

WinHttp.GetProxyForUrl for pac failed with error 'The URL does not use a
recognized protocol' #12006.

Relocating the PAC to a http:// URL solves the #12006 error.  However, the PAC is still effectively ignored within Eclipse.

Here's a very simple test PAC file:

function FindProxyForURL(url, host){
	return "PROXY localhost:12345";
}

In order to eliminate any issues with the CVS provider or other plugins, I wrote a quick plugin to test the results from IProxyService myself.  Using the above PAC configured in IE, and having Eclipse configured to use the "System proxy configuration", any of the .getProxyDataForHost(...) calls return null / empty.

Changing the proxy settings in IE back to manual results in the IProxyService returning expected values.

I will retest on Windows XP.
Comment 1 Mark A. Ziesemer CLA 2008-12-03 17:53:30 EST
Identical results under Windows XP 32-bit and IE6.
Comment 2 Mark A. Ziesemer CLA 2008-12-03 19:30:54 EST
As also reflected in comments on bug 180921 and 204563, too bad even Eclipse 3.5 still needs to remain Java 1.4 compatible (http://www.eclipse.org/projects/project-plan.php?projectid=eclipse), considering the java.net.ProxySelector available in Java 1.5 / 5.0.

That considered, please consider making it possible to allow a plugin to provide an implementation of IProxyService.  As also noted in bug 257420, this doesn't seem currently possible, short of providing a patched "org.eclipse.core.net" package.

Maybe this could even be implemented a new provider type added to ProxySelector, next to Direct, Eclipse, and Native, called "Plugin"?

One such use of this would be the patched "org.eclipse.core.net" project I just made, which could be converted to a separate plugin.  It simply redirects any calls to getProxyDataForHost(String host) and getProxyDataForHost(String host, String type) to select(URI uri), which then calls java.net.ProxySelector.getDefault().select(uri).

(This also eliminated the need for just about everything else in "org.eclipse.core.internal.net", including all the native code.  Any code still required could be transferred into platform-specific plugins, etc.)

Especially considering the decision in bug 240856 to enable automatic proxy detection by default, this is not only an important bug, but maybe a chance to delegate some of this functionality back up to Java and/or the OS.  Just my $0.02...

Even if none of the improvements listed above are possible for 3.5, thanks for looking into the main issue!
Comment 3 Pawel Pogorzelski CLA 2008-12-04 07:12:18 EST
(In reply to comment #2)
> That considered, please consider making it possible to allow a plugin to
> provide an implementation of IProxyService.  As also noted in bug 257420, this
> doesn't seem currently possible, short of providing a patched
> "org.eclipse.core.net" package.
> 
> Maybe this could even be implemented a new provider type added to
> ProxySelector, next to Direct, Eclipse, and Native, called "Plugin"?

We are considering such a move. Bug 246068 was opened as a first step toward this (if all native libraries implement a common interface we can easily attach an arbitrary one through an extension point). Mark, are you willing to provide some help in the area?
Comment 4 Mark A. Ziesemer CLA 2008-12-04 09:37:00 EST
> Mark, are you willing to provide some help in the area?

What exactly did you have in mind?  I'll see what I can do, though I have other commitments that don't leave me a lot of time.  I can definitely help with analysis and testing.  Maybe a patch to add support for extension points, but it may be at least a week before I could start on that.
Comment 5 Mark A. Ziesemer CLA 2008-12-09 15:32:00 EST
As I'm starting to look at this some more, shouldn't the UI be split by provider?

Right now, running with the latest "org.eclipse.core.net" and "org.eclipse.ui.net" from CVS, it seems that any provider is expected to use the same UI, consisting of options such as host, port, authentication, and proxy bypass.  Depending upon the provider, these options may not be relevant, and other options may be needed.

For example, I'd plan to write an extension that will allow configuration from a PAC file, without any native dependencies.  Even if configured to use "native", and native is configured to use PAC, the same argument applies.  Really none of the current options are applicable to PAC.  Instead, an option to configure a file/URL location (along with maybe an "auto-detect PAC location") option would be needed.  While the current PAC functionality doesn't even allow for querying a list of configured proxies, etc., it is infinitely more flexible and scalable.

I recommend that each provider will need to contribute both a provider implementation and a UI section.  (Though these could optionally be provided by and within one extension.)  The current UI panels could be kept as one UI implementation available by default or for extension by other providers, but it should not be required, or at least easily replaced by extension providers.

This seems like it would also make more sense from the user's perspective as well.  Unless I'm misunderstanding the purpose, always having multiple providers and options listed despite the configured "active provider" seems a bit confusing.
Comment 6 Mark A. Ziesemer CLA 2008-12-09 15:49:22 EST
Another question:  I assume an interface will need to be defined for extensions to implement.  Following my previous recommendation, such an interface should only be concerned with providing the service, not configuration.

I.E., IProxyService currently contains the select(URI uri) method, which callers use to find a proxy for a given URI, as well as numerous methods for configuration, e.g. setProxyData(IProxyData[] proxies), setNonProxiedHosts(String[]), and others.  An extension interface should only provide the methods necessary for determining and returning a proxy for use.

One possible outline for this interface could be simply:
IProxyData[] select(URI uri);

However, since we're seemingly starting to mirror java.net.ProxySelector, would it be possible to just use it?  That API is quite contained.  We'd only need java.net.ProxySelector and java.net.Proxy (which contains the Proxy.Type Enum).  What would be the technical and "legal" issues in simply including copies or a rewrite of this API, just for pre-Java 1.5 compatibility?  If running Eclipse 1.5+, its classes would take precedence, and the Eclipse-provided version could be simply removed once Java 1.5 becomes the minimum requirement.

The only technical issue I see from a class perspective is the List<Proxy> returned by java.net.ProxySelector.select(URI uri), as generics aren't available until Java 1.5.  However, considering the erasures, this shouldn't cause any compatibility issues.
Comment 7 Mark A. Ziesemer CLA 2008-12-09 22:18:08 EST
I'm submitting a patch that is a pretty solid start to externalizing the proxy providers to separate plugins.  It defines the interface and the extension point, separates the "Direct", "Eclipse", and "Native" functionality into different extensions (all still contained within the same project), and factors the UI configuration into extensions as well.

My primary goal for submitting this is for getting some feedback, and a feel as to whether this is a workable approach before I put more time into this.

The affected projects are "org.eclipse.core.net" and "org.eclipse.ui.net".


Comments for "org.eclipse.core.net":

Based on my comment #6, I defined a org.eclipse.core.net.proxy.IProxyProvider interface, consisting of only "String getName()" and "IProxyData[] select(URI uri)".  Extensions should actually implement IProxyProviderExtension, which extends IProxyProvider as well as IExecutableExtension.

Methods were added to IProxyService to get the available providers, and to get and set the current provider.  All other methods except for "select(URI uri)" and the listeners are now marked as deprecated.  It would be great if we could finalize this and actually remove them before the API freeze for 3.5.

Most of ProxyManager was moved to the new EclipseProvider class, with any of the native-specific code moved to the new NativeProvider class.  There is also a DirectProvider class - which is also the only one I'd consider complete.

The NativeProvider should also be complete with not much effort, especially if we can agree on slimming down the IProxyService API.  It shouldn't need to provide any methods for configuration, not even for the UI, as these should be provided by the OS.

Two other things that should be looked at:
1) "WindowsProxyProvider" is out-of-place, and I don't see where it is called from.  It should either be removed, or moved into the platform-specific ".proxy" sub-package.
2) The "ProxyType" class is in need of a major trimming.  It shouldn't contain much more than java.net.Proxy - just a proxy type, host, and port to use.  Everything else should be moved into the provider extensions.  Direct shouldn't need anything, and Native shouldn't need much.  Most probably belongs in the EclipseProvider extension.



Comments for "org.eclipse.ui.net":

Following my recommendation in comment #5, I've split up the UI.  The existing "proxyEntriesComposite" and "nonProxyHostsComposite" were moved into a new EclipseProviderPreferencePage extension.  It currently fails to display due to dependencies on the old ProxySelector, etc., something I can look into further if this is looking like a go-forward solution.  The "Direct" and "Native" options currently don't have a UI extension, nor should they need one.  If no UI extension is supplied, a default label explaining this is shown.
Comment 8 Mark A. Ziesemer CLA 2008-12-09 22:20:25 EST
Created attachment 119991 [details]
Proposed initial patch.
Comment 9 Szymon Brandys CLA 2008-12-10 04:29:43 EST
Thanks Mark. It seems that you've started fixing issues that we plan for the near future :-) Pawel will review it and I hope that we will be able to use your contribution.
Comment 10 David Samuelsson CLA 2009-02-03 10:57:55 EST
hello, we have seen other variants and implications of this bug


prerequisites: internal updatesite available on http adress, proxy available to connect externally

1. insert a proxy 
2. try to connect to proxy but with internal updatesite adress-> does not work our proxy ignores to forward internall adresses
3. remove the proxy mark in "direct connection to internet"
4. try to connect to internal update site -> does not work, Eclipse is still trying to use proxy
5. workaround = insert exception for internal update site then it will work

we noticed that we do not see this behaviour until we filled in a proxy, then it always tries to use it.

cheers, good to see that these seems to be progress on this one!

/Dave
Comment 11 Pawel Pogorzelski CLA 2009-02-05 11:14:50 EST
David, you're probably referring to situation described on bug 255981 (and its duplicates) which has been fixed. If not please provide a build ID.
Comment 12 Pawel Pogorzelski CLA 2009-02-05 11:19:56 EST
Mark, I agree on most of the things you mentioned on the bug. Thanks for the patch and please give me another few days to have a look at it. I'll try to provide a more detailed opinion.
Comment 13 Pawel Pogorzelski CLA 2009-03-09 09:05:19 EDT
(In reply to comment #1)
> Identical results under Windows XP 32-bit and IE6.
> 

Can't reproduce the problem in this configuration. I expose PAC script (from comment 0) via a web server installed on my local machine. However, the correct setting is returned only if URI passed to IProxyService is prefixed with "HTTP". For example:
IProxyManager.select(new URI("http://eclipse"));
IProxyManager.getProxyDataForHost("eclipse", "http");

How does the invocation of the service in your test code look like?
Comment 14 Pawel Pogorzelski CLA 2009-03-09 09:19:12 EDT
(In reply to comment #8)
> Created an attachment (id=119991) [details]
> Proposed initial patch.
> 

Mark, your patch looks promising but I didn't go to deeply into it. The main reason for that is that we don't plan to release it during 3.5 development cycle. However we want to address bug 246068 early in the 3.6 cycle and then maybe use your contribution.

From now on please focus on PAC support on this bug and add the comments about exposing an extension point for custom proxy providers to bug 246068. Mark has put a reference from bug 246068 to this one so we don't loose anything.

Comment 15 Pawel Pogorzelski CLA 2009-03-10 17:59:48 EDT
Moving target milestone to 3.5 M7 while waiting for a response from Mark.
Comment 16 Mark A. Ziesemer CLA 2009-03-10 18:34:14 EDT
I guess I'm really quite confused at what I'm being asked.

I do strongly feel that my patch is a much better design over than the latest milestone, both in comparison to 3.4, especially in that it cleans up a lot of the design and provides support for custom proxy providers.  (The current patch addresses both this bug and bug 246068.)

Unfortunately, I don't believe I saw or did anything in the work of this bug to address the root PAC issue.  I guess my plan / approach was to primarily target bug 246068.

A "PAC Proxy Provider" could then be provided as an extension to Eclipse to handle working with PAC files, and could be system/platform-independent through the use of Mozilla Rhino, etc.  As an extension, it could require Java 1.5 or additional libraries without affecting the core Eclipse dependencies.  The only thing this wouldn't accomplish is sharing the PAC URL from the Internet Explorer LAN Settings property page - but that is something that could probably be done with minimal native code, and could eliminate the need for the WinHttp wrapper entirely.

Additionally / alternatively, a "WinHttp" extension could be included that maintains the current native code, without adding any dependencies.  It would only apply to Windows, and would probably make platform packaging easier?

I guess I'm not up to fixing the native PAC issue by itself, at least not under the proxy code currently checked-in to HEAD.  Especially if this needs to be fixed within the native code, I'm currently not setup with the toolset, etc., to work with it.

I'd ask to just push ahead with the approach contained in the attached patch, but according to the project plan, we're just 3 days away from the next milestone and the API freeze.  Maybe it would be best to just commit to both this and bug 246068 by setting them for 3.6 target milestones?
Comment 17 Pawel Pogorzelski CLA 2009-03-11 05:37:11 EDT
(In reply to comment #16)
> Unfortunately, I don't believe I saw or did anything in the work of this bug to
> address the root PAC issue.

See comment 13.
Comment 18 Pawel Pogorzelski CLA 2009-03-11 05:48:27 EDT
(In reply to comment #16)
> <text deleted/>

Mark, I agree that the patch simplifies the design. And it's obvious that the best solution is to provide an ability to provide a custom proxy provider. We'd be happy to use your contribution. 

However we had a lot of problems with proxy support and we decided to stick with current, not flexible but stable code. The area is very fragile because in many environments proxy related bugs render the whole IDE (as it comes out of the box) useless. Just querry bugzilla for proxy bugs fixed in this cycle to see what I mean.
Comment 19 Pawel Pogorzelski CLA 2009-03-11 05:53:19 EDT
Since the discussion on this bug is all about proxy changes I'm changing the abstract. The old value was: "[Net] Proxy Auto Configuration (PAC) not working". Also, setting target to 3.6.

Please respond to comment 13 and if the problem still appears open a separate bug on it. We can't track multiple issues on this bug no more.
Comment 20 Pawel Pogorzelski CLA 2009-03-11 05:54:14 EDT
*** Bug 246068 has been marked as a duplicate of this bug. ***
Comment 21 Pawel Pogorzelski CLA 2009-05-05 10:30:28 EDT
The current proxy UI with checkboxes that can't be selected/deselected is not the most intuitive one. During the redesign we have to decide whether to drop the UI and provide an ability to plug-in a custom one or to change the current look. Go to bug 273211, comment 16 to see Kiril's suggestion.

Comment 22 Pawel Pogorzelski CLA 2009-05-14 06:37:27 EDT
One more thing to investigate when resolving this bug. The project core.net uses  equinox preferences extension point by contributing PreferenceInitializer and PreferenceModifyListener classes. Seems like these classes are not utilized in a way they should be because we provide equivalent functionality explicitly in the code.
Comment 23 Scott Hendrickson CLA 2010-01-05 18:09:50 EST
Would it be possible to support proxy settings specified by the command line for headless instantations of eclispse (e.g., to build a distribution using buckminster). In my case, the proxy settings are changed regularly by my company, so it's not really an option to use the GUI. Instead I'd like the automated script running eclipse to simply tell eclipse what the proxy settings for that moment.

I posted bug 29883 with a patch (but, it's aimed at working with 3.5 and the solution on this bug report is a much better long term solution). Regardless of the outcome of that bug, a completely headless solution as part of this bug would greatly help me. Thanks!
Comment 24 Scott Hendrickson CLA 2010-01-05 18:12:35 EST
Oops, I meant bug 298813 not 29883 :)
Comment 25 Scott Hendrickson CLA 2010-01-07 16:40:42 EST
*** Bug 298813 has been marked as a duplicate of this bug. ***
Comment 26 Ivan Motsch CLA 2010-01-18 09:17:59 EST
*** Bug 299756 has been marked as a duplicate of this bug. ***
Comment 27 Szymon Brandys CLA 2010-05-07 05:09:36 EDT
We are not going to fix it in 3.6.
Comment 28 Wojciech Galanciak CLA 2010-11-19 07:20:07 EST
Created attachment 183464 [details]
initial patch + tests

This patch provides an extension point (in org.eclipse.core.net) which allows to add custom proxy providers. Extension has to extend AbstractroxyProvider which is now an internal class. We could consider to move it to API and change to interface cause now it does not contain any impelemtation. The patch is fully compatible with previous provider switching mechanism (isProxyEnabled and isSystemProxyEnabled preferences). There are also new method in API which allow to change provider to a custom one using IProxyService.

Patch also contains some unit tests which test all new features. There is still problem with one regression test which is not passed  - see bug 325483 to get more details.
Comment 29 Konrad Windszus CLA 2011-04-09 05:57:02 EDT
Please also provide a native ProxyProvider which uses the java.net.ProxySelector. This way at least the native Mac proxy settings can be reused. Of course this is only available since Java 5. So far no native proxy provider is available under Mac OS, so using the ProxySelector might be an easy way to provide this (see also bug 330691)