[icedtea-web] RFC: add support in netx for using proxy settings from browser
Omair Majid
omajid at redhat.com
Mon Jan 24 14:57:02 PST 2011
On 01/24/2011 12:21 PM, Dr Andrew John Hughes wrote:
> On 17:29 Fri 24 Dec , Omair Majid wrote:
>> On 12/24/2010 03:31 PM, Omair Majid wrote:
>>> On 12/22/2010 07:40 PM, Dr Andrew John Hughes wrote:
>>>> On 17:34 Wed 22 Dec , Omair Majid wrote:
>>>>> Hi,
>>>>>
>>>>> The attached patch makes netx (not plugin) behave correctly (or at least
>>>>> slightly better) when deployment.proxy.type is set to 3 (for
>>>>> PROXY_TYPE_BROWSER).
>>>>>
>>>>> With this patch, netx will parse the firefox preferences file to figure
>>>>> out the browser's settings and use that when trying to figure out the
>>>>> proxy to use for accessing a given URI.
>>>>>
>>>>
>>>> I think there could be better handling if the profiles.ini does not
>>>> exist.
>>>> AFAICS, it would print out a stack trace. We should handle that much more
>>>> gracefully. It's not really an exceptional event that the user hasn't
>>>> used Firefox before, but something we should account for.
>>>>
>>> Good point. I will post an updated patch to address this issue.
>>>
>>
>> This updated patch takes care of it. If the preferences file (or the
>> profiles file) do not exist (or are damaged), BROWSER_PROXY_TYPE_NONE is
>> assumed and the user is notified:
>> Unable to use Firefox's proxy settings. Using "DIRECT" as proxy type.
>>
>
> Ok. Further comments inline.
>
>
>> diff -r 7ddab63cf8fe netx/net/sourceforge/jnlp/browser/BrowserAwareProxySelector.java
>> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
>> +++ b/netx/net/sourceforge/jnlp/browser/BrowserAwareProxySelector.java Fri Dec 24 17:19:09 2010 -0500
>> +public class BrowserAwareProxySelector extends JNLPProxySelector {
>> +
>> + /* firefox's constants */
>> + public static final int BROWSER_PROXY_TYPE_NONE = 0;
>> + public static final int BROWSER_PROXY_TYPE_NONE2 = 3;
>> + /** use gconf, WPAD and then env (and possibly others)*/
>> + public static final int BROWSER_PROXY_TYPE_AUTO = 4;
>> + /** use env variables */
>> + public static final int BROWSER_PROXY_TYPE_SYSTEM = 5;
>> + public static final int BROWSER_PROXY_TYPE_MANUAL = 1;
>> + public static final int BROWSER_PROXY_TYPE_PAC = 2;
>> +
>
> Is there a reason the integer values are out of order here?
>
Not that I know of. It must have been a mistake. Fixed.
>> + /**
>> + * Initialize configuration by reading preferences from the browser (firefox)
>> + */
>> + private void initFromBrowserConfig() {
>> +
>> + try {
>> + File preferencesFile = FirefoxPreferencesFinder.find();
>> + if (preferencesFile == null) {
>> + System.out.println(R("RProxyFirefoxNotFound"));
>> + browserProxyType = PROXY_TYPE_NONE;
>> + return;
>> + }
>
> Should the output not be to err rather than out (as in the method below)?
Fixed.
>
>> + FirefoxPreferencesParser parser = new FirefoxPreferencesParser(preferencesFile);
>> + parser.parse();
>> + Map<String, String> prefs = parser.getPreferences();
>> +
>> + String type = prefs.get("network.proxy.type");
>> + if (type != null) {
>> + browserProxyType = Integer.valueOf(type);
>> + } else {
>> + browserProxyType = BROWSER_PROXY_TYPE_AUTO;
>> + }
>> +
>> + try {
>> + browserAutoConfigUrl = new URL(prefs.get("network.proxy.autoconfig_url"));
>> + } catch (MalformedURLException e) {
>> + e.printStackTrace();
>> + }
>> +
>> + browserUseSameProxy = Boolean.valueOf(prefs.get("network.proxy.share_proxy_settings"));
>> +
>> + browserHttpProxyHost = prefs.get("network.proxy.http");
>> + browserHttpProxyPort = Integer.valueOf(prefs.get("network.proxy.http_port"));
>> + browserHttpsProxyHost = prefs.get("network.proxy.ssl");
>> + browserHttpsProxyPort = Integer.valueOf(prefs.get("network.proxy.ssl_port"));
>> + browserFtpProxyHost = prefs.get("network.proxy.ftp");
>> + browserFtpProxyPort = Integer.valueOf(prefs.get("network.proxy.ftp_port"));
>> + browserSocks4ProxyHost = prefs.get("networking.proxy.socks");
>> + browserSocks4ProxyPort = Integer.valueOf(prefs.get("network.proxy.socks_port"));
>> +
>> + } catch (IOException e) {
>> + e.printStackTrace();
>> + System.out.println(R("RProxyFirefoxNotFound"));
>
> Likewise.
>
Fixed.
> Could FirefoxPreferencesFinder not throw a FileNotFoundException (or
> whatever is appropriate for the other cases) rather than returning
> null, so the same logic could be used for both?
>
Good point. Done.
>> + /**
>> + * The main entry point for {@link BrowserAwareProxySelector}. Based on
>> + * the browser settings, determines proxy information for a given URI.
>> + *<p>
>> + * The appropriate proxy may be determined by reading static information
>> + * from the browser's preferences file, or it may be computed dynamically,
>> + * by, for example, running javascript code.
>> + */
>> + @Override
>> + protected List<Proxy> getFromBrowser(URI uri) {
>> + List<Proxy> proxies = new ArrayList<Proxy>();
>> +
>> + switch (browserProxyType) {
>> + case BROWSER_PROXY_TYPE_PAC:
>> + proxies.addAll(getFromBrowserPAC(uri));
>> + break;
>> + case BROWSER_PROXY_TYPE_MANUAL:
>> + proxies.addAll(getFromBrowserConfiguration(uri));
>> + break;
>> + case BROWSER_PROXY_TYPE_NONE:
>> + proxies.add(Proxy.NO_PROXY);
>> + break;
>> + case BROWSER_PROXY_TYPE_AUTO:
>> + // firefox will do a whole lot of stuff to automagically
>> + // figure out the right settings. gconf, WPAD, and ENV are used.
>> + // https://bugzilla.mozilla.org/show_bug.cgi?id=66057#c32
>> + case BROWSER_PROXY_TYPE_SYSTEM:
>> + // means use $http_proxy, $ftp_proxy etc.
>
> Is this a TODO or is this case really empty?
>
Definitely a TODO. I thought the error message in the fall-through case
(next-line) made it clear. I have made it more explicit.
>> + default:
>> + System.err.println(R("RProxyFirefoxOptionNotImplemented"));
>> + proxies.add(Proxy.NO_PROXY);
>
> Maybe the error should include the value of browserProxyType?
>
Done. It now prints a rough description and the value of browserProxyType.
>> + /**
>> + * Get an appropriate proxy for the given URI using static information from
>> + * the browser's preferences file.
>> + */
>> + private List<Proxy> getFromBrowserConfiguration(URI uri) {
>> + List<Proxy> proxies = new ArrayList<Proxy>();
>> +
>> + String scheme = uri.getScheme();
>> +
>> + if (browserUseSameProxy) {
>> + SocketAddress sa = new InetSocketAddress(browserHttpProxyHost, browserHttpProxyPort);
>> + Proxy proxy;
>> + if (scheme.equals("socket")) {
>> + proxy = new Proxy(Type.SOCKS, sa);
>> + } else {
>> + proxy = new Proxy(Type.HTTP, sa);
>> + }
>> + proxies.add(proxy);
>> + } else if (scheme.equals("http")) {
>> + SocketAddress sa = new InetSocketAddress(browserHttpProxyHost, browserHttpProxyPort);
>> + proxies.add(new Proxy(Type.HTTP, sa));
>> + } else if (scheme.equals("https")) {
>> + SocketAddress sa = new InetSocketAddress(browserHttpsProxyHost, browserHttpsProxyPort);
>> + proxies.add(new Proxy(Type.HTTP, sa));
>
> Do you not need to do something different for HTTPS? Or is that handled elsewhere?
>
Are you referring to Type.HTTP? There are only 3 Types defined: DIRECT,
HTTP and SOCKS. Type.HTTP is for http, https and ftp.
>> + } else if (scheme.equals("ftp")) {
>> + SocketAddress sa = new InetSocketAddress(browserFtpProxyHost, browserFtpProxyPort);
>> + proxies.add(new Proxy(Type.HTTP, sa));
>> + } else if (scheme.equals("socket")) {
>> + SocketAddress sa = new InetSocketAddress(browserSocks4ProxyHost, browserSocks4ProxyPort);
>> + proxies.add(new Proxy(Type.SOCKS, sa));
>> + } else {
>> + proxies.add(Proxy.NO_PROXY);
>> + }
>> +
>> + return proxies;
>> + }
>> +
>> +}
>> diff -r 7ddab63cf8fe netx/net/sourceforge/jnlp/browser/FirefoxPreferencesParser.java
>> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
>> +++ b/netx/net/sourceforge/jnlp/browser/FirefoxPreferencesParser.java Fri Dec 24 17:19:09 2010 -0500
>> @@ -0,0 +1,152 @@
>> +/* FirefoxPreferencesParser.java
>> + Copyright (C) 2010 Red Hat, Inc.
>> +
>> +This file is part of IcedTea.
>> +
>> +IcedTea is free software; you can redistribute it and/or
>> +modify it under the terms of the GNU General Public License as published by
>> +the Free Software Foundation, version 2.
>> +
>> +IcedTea is distributed in the hope that it will be useful,
>> +but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> +General Public License for more details.
>> +
>> +You should have received a copy of the GNU General Public License
>> +along with IcedTea; see the file COPYING. If not, write to
>> +the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> +02110-1301 USA.
>> +
>> +Linking this library statically or dynamically with other modules is
>> +making a combined work based on this library. Thus, the terms and
>> +conditions of the GNU General Public License cover the whole
>> +combination.
>> +
>> +As a special exception, the copyright holders of this library give you
>> +permission to link this library with independent modules to produce an
>> +executable, regardless of the license terms of these independent
>> +modules, and to copy and distribute the resulting executable under
>> +terms of your choice, provided that you also meet, for each linked
>> +independent module, the terms and conditions of the license of that
>> +module. An independent module is a module which is not derived from
>> +or based on this library. If you modify this library, you may extend
>> +this exception to your version of the library, but you are not
>> +obligated to do so. If you do not wish to do so, delete this
>> +exception statement from your version.
>> +*/
>> +
>> +package net.sourceforge.jnlp.browser;
>> +
>> +import java.io.BufferedReader;
>> +import java.io.File;
>> +import java.io.FileReader;
>> +import java.io.IOException;
>> +import java.util.HashMap;
>> +import java.util.Map;
>> +
>> +/**
>> + * A parser for Firefox's preferences file. It can 'parse' Firefox's
>> + * preferences file and expose the prefrences in a simple to use format.
>> + *<p>
>> + * Sample usage:
>> + *<pre>
>> + * FirefoxPreferencesParser p = new FirefoxPreferencesParser(prefsFile);
>> + * p.parse();
>> + * Map<String,String> prefs = p.getPreferences();
>> + * System.out.println("blink allowed: " + prefs.get("browser.blink_allowed"));
>> + *</pre>
>> + */
>> +public final class FirefoxPreferencesParser {
>> +
>> +
>> + /**
>> + * Parse the prefernces file
>> + * @throws IOException if an exception ocurrs while reading the
>> + * preferences file.
>> + */
>> + public void parse() throws IOException {
>> + System.out.println("Read " + prefs.size() + " entries from Firefox's preferences");
>
> System.err?
>
Whoops, this is meant for debugging only. I have wrapped it in a if
(debug) block now.
>> diff -r 7ddab63cf8fe netx/net/sourceforge/jnlp/resources/Messages.properties
>> --- a/netx/net/sourceforge/jnlp/resources/Messages.properties Tue Dec 21 16:48:12 2010 -0500
>> +++ b/netx/net/sourceforge/jnlp/resources/Messages.properties Fri Dec 24 17:19:09 2010 -0500
>> @@ -141,6 +141,9 @@
>> RUnexpected=Unexpected {0} at {1}
>> RConfigurationError=Fatal error while reading the configuration
>> RConfigurationFatal=ERROR: a fatal error has occurred while loading configuration. Perhaps a global configuration was required but could not be found
>> +RPRoxyPacNotImplemented=Using Proxy Auto Config (PAC) files is not supported yet.
>> +RProxyFirefoxNotFound=Unable to use Firefox's proxy settings. Using "DIRECT" as proxy type.
>> +RProxyFirefoxOptionNotImplemented=Some browser proxy options are not supported yet
>>
>> # Boot options, message should be shorter than this ---------------->
>> BOUsage=javaws [-run-options]<jnlp file>
>> diff -r 7ddab63cf8fe netx/net/sourceforge/jnlp/runtime/JNLPProxySelector.java
>> --- a/netx/net/sourceforge/jnlp/runtime/JNLPProxySelector.java Tue Dec 21 16:48:12 2010 -0500
>> +++ b/netx/net/sourceforge/jnlp/runtime/JNLPProxySelector.java Fri Dec 24 17:19:09 2010 -0500
>> @@ -336,10 +337,7 @@
>> if (autoConfigUrl == null) {
>> return Arrays.asList(new Proxy[] { Proxy.NO_PROXY });
>> }
>> - // TODO implement this by reading and using the PAC file
>> - if (JNLPRuntime.isDebug()) {
>> - System.err.println("WARNING: Using a Proxy Auto Config file is not implemented yet");
>> - }
>> + System.err.println(R("RPRoxyPacNotImplemented"));
>
> I'd leave the TODO in place.
>
Fixed.
Does this look okay? Any further suggestions for improvement?
Cheers,
Omair
-------------- next part --------------
A non-text attachment was scrubbed...
Name: firefox-integration-03.patch
Type: text/x-patch
Size: 25317 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110124/12828e10/firefox-integration-03.patch
More information about the distro-pkg-dev
mailing list