[icedtea-web] RFC: add support in netx for using proxy settings from browser

[Omair Majid]

On 01/26/2011 02:56 PM, Dr Andrew John Hughes wrote:
> On 17:57 Mon 24 Jan     , Omair Majid wrote:
>> On 01/24/2011 12:21 PM, Dr Andrew John Hughes wrote:
>>>
>
> snip...
>
>>> 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.
>>
>
> Yeah, it will for the end user.  The code comment was just unclear.
>
> snip...
>>
>>>> +    /**
>>>> +     * 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.
>>
>
> Ok I'm just wondering how you distinguish between http, https and ftp in the proxies list?  Presumably
> you don't need to.
>

Yeah, the java libraries take care of it. The ProxySelector is used by 
the networking bits to query what proxy should be used whenever 
initiating a new connection. They expect a Proxy object with the right 
type - Type.HTTP for ftp, http or https connections. For example, 
sun.net.www.protocol.ftp.FtpURLConnection.connect() will only use the 
proxy if Proxy.Type is HTTP. 
sun.net.www.protocol.https.HttpsClient.needsTunnelling() also uses a 
Proxy with Type HTTPS.

>>
>> Does this look okay? Any further suggestions for improvement?
>>
>
> Looks ok now.  Just check the indenting (mentioned inline below) on BrowserAwareProxySelector and it
> seems good to go.
>

Fixed.

Thanks for reviewing the patch!

Cheers,
Omair