76649 - PAC: dnsDomainIs should match ANY subdomain to an FQDN

Status: VERIFIED FIXED

(Core :: Networking, defect)

Description

[Colin Blake]

I'm using the new proxy autoconfig feature from my build of yesterday (4/18/01).
The dnsDomainIs code doesn't appear to work as expected. If the target 
host is "a.b.c.d" then I would expect both of these to return true:

  dnsDomainIs(host,".b.c.d")
  dnsDomainIs(host,".c.d")

whereas only this one returns true:

  dnsDomainIs(host,".b.c.d")

I've looked around the net and seen several PAC files that would break with
this behavior. Is this intended or am I missing something here?

Comment 1

[Colin Blake]

Here's some dump output from my PAC file showing the problem:

URL: http://galaxy.zko.dec.com/includes/images/compaqCom.gif
host: galaxy.zko.dec.com
isPlainHostName: false
dnsDomainIs(zko.dec.com): false
dnsDomainIs(.zko.dec.com): true
dnsDomainIs(dec.com): false
dnsDomainIs(.dec.com): false
dnsDomainIs(.compaq.com): false

Comment 2

[Asa Dotzler [:asa]]

updating component

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

For reference, the classic implementation of dnsDomainIs is at
http://lxr.mozilla.org/classic/source/network/main/mkautocf.c#1288 and I think
all it did was return whether the domain string was a final substring (by
case-insensitive comparison) of the host string.

Comment 4

[neeti]

The new proxy autoconfig feature uses the dnsDomainIs(..) code in 
netwerk\base\src\nsProxyAutoConfig.js. 

cc'ing [e-mail] and [e-mail] . This is related to the landing 
of bug 53080

Comment 5

[Denis Antrushin]

Examples at http://home.netscape.com/eng/mozilla/2.0/relnotes/demo/proxy-
live.html#dnsDomainIs use dot before domain name to be checked against:
-----------------------------------
Examples:
dnsDomainIs("www.netscape.com", ".netscape.com") 
is true. 
dnsDomainIs("www", ".netscape.com") 
is false. 
dnsDomainIs("www.mcom.com", ".netscape.com") 
is false. 
--------------------------------------
As for matching domain parts, feel free to supply patch implementing
right behavior from your point of view :-)

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: attempt at a fix (I'm unable to test it)

Comment 7

[Gagan]

patch seems reasonable. If it works then r=gagan

Comment 8

[Colin Blake]

I have tried this patch on my system, and dnsDomainIs is now working as 
expected.

Comment 9

[neeti]

darin, could you sr= this.

Comment 10

[Christopher Blizzard (:blizzard)]

sr=blizzard, a=blizzard for 0.9.  dbaron, can you check this in please?

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Yeah, I'll check this in when branch and trunk open for checkins, assuming it's
before I go to sleep...

Do we know that all the other functions work in a backwards-compatible way?  How
much have they been tested?

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to branch and trunk, 2001-04-26 07:05/07:06.

Comment 13

[benc]

QA to me.

Later documents make this behavior more explicit:

http://docs.iplanet.com/docs/manuals/proxy/36/adminux/autoconf.htm

"The dnsDomainIs() function detects whether the URL host name belongs to a given
DNS domain"

Comment 14

[benc]

VERIFIED:
mozilla 0.9 Win32 and Linux

Verification on MacOS is blocked.

I got lazy and used some a && b logic to verify that both cases are true:

function FindProxyForURL(url, host)
{
if (dnsDomainIs(host,".packetgram.com") && dnsDomainIs(host,".com"))
        return "PROXY <proxy:port>";
else
        return "DIRECT";
}
www.packetgram.com is true for both cases.
packetgram.com is not.

I hooked up proxy-auth on my server so I would know when it used PROXY...

Someone who knows javascript please double-check my syntax.

Comment 15

[Darin Fisher]

what if you remove the leading '.' from .com and .packetgram.com?

Comment 16

[benc]

I don't know. I'm just testing the valid use of the syntax :)

I'll try it later...

Comment 17

[benc]

futuring qa of this pac function.

Comment 18

[benc]

V/fixed:
filed bug 242305 about Darin's comment 15.