manugarg / pacparser

A library to parse proxy auto-config (PAC) files
http://pacparser.manugarg.com
GNU Lesser General Public License v3.0
506 stars 116 forks source link

Always perform syntax checking #18

Closed manugarg closed 10 years ago

manugarg commented 10 years ago

From dpinck...@gmail.com on December 29, 2011 08:17:31

If you have a syntax error, pactester doesn't always report it.

Our "WPAD.DAT" file consists mostly of lines like this: if (         dnsDomainIs(host, "nfe.fazenda.sp.gov.br") ||         dnsDomainIS(host, "produtos.sondaprocwork.com.br") ||         dnsDomainIs(host, ".proxibid.com")         ) return Direct_Access;

Notice the capital "S" in the 2nd condition?  That will break the file in IE on Windows and EVERYTHING will go directly out instead of through our proxy.

When I run pactester against this, if I don't test for that specific host name, pactester will not generate an error, so I think that my file is working great, but proxying is actually broken.

pactester.exe -p wpad.dat -f url_list.txt http://nfe.fazenda.sp.gov.br : DIRECT JSERROR: PAC script:43:     ReferenceError: dnsDomainIS is not defined pacparser.c: pacparser_find_proxy: Problem in executing FindProxyForURL.

pactester.c: Problem in finding proxy for http://produtos.sondaprocwork.com.br .

I've just downloaded the latest version from the pacparser page (v1.3.0) and I'm running it on Windows 7 Enterprise 64-bit.

The older version from the pactester page worked as expected, but I really like that the new version prints the URL in addition to the result.

This is a GREAT utility as folks don't always pay attention to detail and there's really no other way that I've found to accurately test this is IE.

Original issue: http://code.google.com/p/pacparser/issues/detail?id=18

manugarg commented 10 years ago

From dpinck...@gmail.com on December 29, 2011 08:53:18

I'm sorry, I was wrong.  The older version does the same thing.  I guess I just always added all of the URLs to my test file.

It would still be nice if it checked the syntax of the file.

manugarg commented 10 years ago

From m...@manugarg.com on December 30, 2011 06:23:48

Well, this is the expected behavior. What happens in IE is that when it encounters an error in PAC file evaluation, it stops using it. If it doesn't encounter an error it will keep on using the PAC file even if it has an 'undefined' function.

To illustrate this, with the following PAC file:

function FindProxyForURL(url, host) {  if (dnsDomainIs(host, ".manugarg.com") ||      dnsDomainIS(host, ".google.com"))    return "PROXY proxy.manugarg.com:3128";  else    return "PROXY proxy1.manugarg.com:3128; PROXY proxy2.manugarg.com:3128"; }

If you go to www.manugarg.com first (after setting proxy auto-config or restarting IE), it will use proxy.manugarg.com:3128 as proxy. If you go to a URL that is not in .manugarg.com domain, IE will encounter an error because first condition inside if statement will be false and it will have to evaluate second condition which has a call to an undefined function. After this failure, IE will send all further requests, including request to www.manugarg.com, DIRECT.

pacparser parses the PAC file in a JavaScript context before attempting to call FindProxyForURL - thus it actually performs syntax checking. The PAC file that you gave as an example is syntactically correct. It only contains an undefined function, which is actually not a syntax issue from JavaScript point of view.

manugarg commented 10 years ago

From m...@manugarg.com on December 30, 2011 06:27:45

Since pacparser is behaving as intended, I am closing this issue. Please feel free to reopen, if you're not satisfied with the explanation.

Status: Invalid