Open Bug 558253 Opened 11 years ago Updated 5 months ago

Proxy Auto Config (PAC) Extended functions for multi-resolution and ipv6

Categories

(Core :: Networking, enhancement, P3)

enhancement

Tracking

()

People

(Reporter: aronrubin, Unassigned)

References

Details

(Whiteboard: [necko-backlog])

Attachments

(1 file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3 GTBDFff GTB7.0
Build Identifier: 

There are a couple of problems with the proxy auto config (PAC) API. It does not handle ipv6 and the resolve calls return only the first address. Personally I do not care about ipv6 but the single address is a real problem when it comes to myIpAddress. Virtual adapters/interfaces have addresses too so an address is chosen at random and returned. Microsoft created an extended API for PAC and I see no reason not to adopt it. I have made the necessary changes to nsProxyAutoConfig.js. See the following:
http://blogs.msdn.com/wndp/articles/IPV6_PAC_Extensions_v0_9.aspx
http://code.google.com/p/chromium/issues/detail?id=25407


Reproducible: Always




--- nsProxyAutoConfig.js	2010-04-05 15:20:38 -0400
+++ nsProxyAutoConfig.js	2010-04-08 23:20:31 -0400
@@ -81,6 +81,9 @@ nsProxyAutoConfig.prototype = {
         this._sandBox.importFunction(myIpAddress);
         this._sandBox.importFunction(dnsResolve);
         this._sandBox.importFunction(proxyAlert, "alert");
+        // extended predefined functions
+        this._sandBox.importFunction(myIpAddressEx);
+        this._sandBox.importFunction(dnsResolveEx);
 
         // evaluate loaded js file
         Components.utils.evalInSandbox(pacText, this._sandBox);
@@ -136,6 +139,44 @@ function dnsResolve(host) {
     }
 }
 
+function string_list_from_resolve_list(reslist) {
+    var addrs = [];
+    if( !reslist )
+        return "";
+    while( reslist.hasMore() )
+        addrs.push( reslist.getNextAddrAsString() );
+    return( addrs.join( ";" ) );
+}
+
+// extended wrapper for getting local IP address called by PAC file
+function myIpAddressEx() {
+    try {
+        var reslist = dns.resolve(dns.myHostName, 0);
+        var strlist = string_list_from_resolve_list( reslist );
+        if( strlist != "" )
+            return strlist;
+        else
+            return '127.0.0.1';
+    } catch (e) {
+        return '127.0.0.1';
+    }
+}
+
+// extended wrapper for resolving hostnames called by PAC file
+function dnsResolveEx(host) {
+    host = XPCSafeJSObjectWrapper(host);
+    try {
+        var reslist = dns.resolve(host, 0);
+        var strlist = string_list_from_resolve_list( reslist );
+        if( strlist != "" )
+            return strlist;
+        else
+            return null;
+    } catch (e) {
+        return null;
+    }
+}
+
 var pacModule = new Object();
 
 pacModule.registerSelf =
It would be good to get both a high level reaction as well as a
consideration of the patch itself. For example is this an unreasonable standard or does the implementation need to include ipv6 before accepted?
Set the feedback review request on the patch then.
Attachment #438034 - Flags: feedback+
Attachment #438034 - Flags: feedback+ → feedback?(bzbarsky)
Sorry for the lag here; I was on leave for a while, and am finally digging through my requests.

The spec seems ok to me at first glance, though this is not exactly my area of expertise.  Including ipv6 would be good if it's easy, but we can probably take it without as a first cut, I think.    Unless we expect compat issues due to PACs assuming ipv6 stuff will be in these functions' return values?

The attached patch doesn't implement the spec, though: the failure case return values are "127.0.0.1" and null instead of "", right?  This is a difference between these functions and the ones without "Ex" that's pointed out in the Chromium bug.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attachment #438034 - Flags: feedback?(bzbarsky) → feedback-
Duplicate of this bug: 640202
as far as i understand, all _non_ *Ex() functions shall report / work with IPv4 addresses only. currently FFs' myIpAddress() may report ::1 or other IPv6 addresses, even if a proper IPv4 address is assigned to a NIC, depending on the OS configuration (eq /etc/hosts, /etc/nsswitch.conf on unix).

the *Ex() functions, which shall be available within FindProxyforURLEx(url,host), may report / work with IPv6 and IPv4 addresses. myIpAddressEx() is very much required to get a semi-colon separated list of all IP (4 and 6) addresses of the local host. as of my knowledge this is not possible with the old FindProxyforURL(url,host) api.

http://blogs.msdn.com/b/wndp/archive/2006/07/13/ipv6-pac-extensions-v0-9.aspx

please let me emphasize again that IPv6 is a big topic today and workarounds like turning off IPv6 (ie via network.dns.disableIPv6) are not anymore feasable.

above patch may be a good start -- it would even fit my personal requirements -- but i feel simply adding the *Ex() functions for use within FindProxyforURL(url,host) is not correct.
The proxy services was completely rewritten in bug 769764 and moved from JavaScript to C++. This was for Firefox 18.

That means that this patch is no longer valid.

I'm adding Patrick McManus to this bug. Maybe since he now has such detailed knowledge of the PAC service from the rewrite, it might be possible to do a patch for this.
Duplicate of this bug: 964711
Whiteboard: [necko-backlog]
Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258
Priority: -- → P1
Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258
Priority: P1 → P3

So uh... I am looking into fixing this issue, but being new to the code base I have a few questions:

  1. It looks like dnsResolve is not doing anything to filter out IPv6 addresses. The upstream nsDNSService (I assume) is indeed capable of giving IPv6 back, and judging from the code dnsResolve might just give an IPv6 address sometime. The thing is I don't really have IPv6 here... Could anyone throw a line of alert(dnsResolve('some.ipv6-only.domain')) in their and confirm the hypothesis for me? [The resolver class is going to walk the whole GetNextAddr chain and filter by the raw.family thing. The dnsResolveEx spec wants us to return a list anyways so we will slways need to change that.]
  2. Same question as (1) goes for the myIpAddress. Well, the PACResolver-dependent first step is said to be v6-safe this time, so I guess I was right about the v6 possibility? The myIPAddressEx spec says it wants all unicast addresses for each interface too, so that's a new thing for this module to touch on. Will I find anything useful if I digged into the RTC Network handling for interfaces? (I searched DXR for "eth0".)
You need to log in before you can comment on or make changes to this bug.