Runtime debug assertion when using .pac file with file:// protocol

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Store pac file at some local path on Windows system
2. Specify pac file path in LAN Settings using file:// protocol
3. Call libproxy apis to fetch proxy for an URL

// Create the proxy factory object
pxProxyFactory *pf = px_proxy_factory_new();
if (!pf)
{
    return -1;
}
const char *chURL = "http://www.google.com";
char** proxies = px_proxy_factory_get_proxies(pf, chURL)

What is the expected output? What do you see instead?
There should not be run time crash

What version of the product are you using? On what operating system?
I am using libproxy 0.4.7

Please provide any additional information below.
I am using Windows 7 OS. 

I have debugged the libproxy for this issue, and I have find out that there is 
some problem with url::get_pac() function. For file:// protocol, the read() 
functions reads .pac file incorrectly when it contains "\t"(TAB character) or 
"\r\n" (carriage return) characters.

Original issue reported on code.google.com by udayja...@gmail.com on 26 Jun 2012 at 10:21

Attachments:

• ExpMatch.pac
• [LAN Settings.png](https://storage.googleapis.com/google-code-attachments/libproxy/issue-181/comment-0/LAN Settings.png)

[GoogleCodeExporter]

Can you provide proper backtrace for this crash. Your explanation of this bug 
make no sense as the content of the file is not parsed by get_pac(). It simply 
stat() the file and read it all at once. I would rather suspect the URI parser 
to be broken with windows style file://L:/... (if that is the standard on 
windows, but in any case it should not crash).

Original comment by nicolas.dufresne@gmail.com on 5 Jul 2012 at 6:42

[GoogleCodeExporter]

Ok, first thing is that you file:// URI is incorrect.

See this for simplified explanations:
http://blogs.msdn.com/b/ie/archive/2006/12/06/file-uris-in-windows.aspx

But that also mean we need a special code path for URI parsing on windows. As / 
need to be changed into \, and file://server/Doc/file need to be interpreted as 
UNC \\server\Doc\file while file:///C:/Doc/file has to have a resulting patch 
C:\Doc\file. Currently parser has a bug and ind a hostname "/C:". Simply fixing 
that bug result in /C:/Doc/File, so the initial / will need to be removed, 
along with / \ conversion.

Original comment by nicolas.dufresne@gmail.com on 5 Jul 2012 at 7:07

[GoogleCodeExporter]

r846 fixes the issue where the ":" is confused with port division. I added 2 
unit tests, that fails on windows at the moment. If you have a dev environment 
on windows, can you change url.cpp to make those tests pass (there is a test 
target) ?

Original comment by nicolas.dufresne@gmail.com on 5 Jul 2012 at 7:17

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Yes I agree that stat read the file at once, but the problem on windows 
platform is due to ::open function call which opens .pac file in read only 
mode. On windows there is carriage return characters which get escaped when you 
open file in read only mode. This causes exception in format_pac_response 
function with error "string subscript out of range" on windows platform. As 
file is not opened in binary mode, the escape sequence characters CRLF causes 
this exception may be because of the wrong length of file fetched by stat while 
reading it in read only mode.

If I open .pac file in binary mode, above exception gets disappear.

Original comment by udayja...@gmail.com on 25 Jul 2012 at 4:01

[GoogleCodeExporter]

I have also got runtime crash, in get_pac function in url.cpp in following 
cases:
When web services of server(where .pac files are kept) is not running (e.g. 
Apache server is stopped running on server where .pac files are kept, there 
crash while closing socket on windows platform.

I have got following stack frames while debugging libproxy:

libproxy.dll!libproxy::url::get_pac()  Line 423 C++

libproxy.dll!libproxy::proxy_factory::_get_proxies(libproxy::url * realurl, 
std::vector<std::basic_string<char,std::char_traits<char>,std::allocator<char> 
>,std::allocator<std::basic_string<char,std::char_traits<char>,std::allocator<ch
ar> > > > & response)  Line 352 + 0xb bytes C++

libproxy.dll!libproxy::proxy_factory::get_proxies(std::basic_string<char,std::ch
ar_traits<char>,std::allocator<char> > url_)  Line 196  C++

libproxy.dll!px_proxy_factory_get_proxies(pxProxyFactory_ * self, const char * 
url)  Line 421 + 0x20 bytes C++

I have also attached snapshot of runtime exception caused.

Original comment by udayja...@gmail.com on 25 Jul 2012 at 4:17

Attachments:

• [Exception While closing socket.png](https://storage.googleapis.com/google-code-attachments/libproxy/issue-181/comment-8/Exception While closing socket.png)

[GoogleCodeExporter]

I have also observed runtime crash in following scenario:

Test step 1:
 - Set up DNS server
 - Add wpad host entry in the DNS 
 - Configure DNS setting on the test machine
 - Open “Internet option” – “Connections” – “Local Area Network (LAN) Settings”
 - Enable “Automatically detect settings” 
 - Launch the application which uses libproxy
 - There is runtime crash

Test step 2:

 - Set up DNS server
 - Add wpad host entry in the DNS 
 - Configure DNS setting on the test machine
 - Open “Internet option” – “Connections” – “Local Area Network (LAN) Settings”
 - Enable “Automatically detect settings” 
 - Add the wpad host entry in hosts file
 - Launch the application which uses libproxy
 - There is runtime crash

In both of these cases, stack unwinding information is as follows:

0038e8a4 6f4caf8a 00000000 00000000 00000000 msvcr100!invoke_watson+0x12
0038e8f4 72f1b994 000002bc 717dd718 007e7570 
msvcr100!invalid_parameter_noinfo+0xc
0038ea00 72f1fc8a 717dd748 021d1f68 021d10a0 
libproxy!libproxy::url::get_pac+0x364
0038ea50 72f18ddc 021d1f9c 717dd1c8 021d1f68 
libproxy!libmodman::extension<libproxy::pacrunner_extension,1>::singleton+0x35da
0038ecd0 72f19553 007e73a8 0038ed78 717dd058 
libproxy!libmodman::module_manager::module_manager+0x12ec
0038ed40 72f196ec 0038ed78 007e7300 00100022 
libproxy!libmodman::module_manager::module_manager+0x1a63
0038edac 003f0325 021d1f68 023dfa88 717d7cb3 
libproxy!px_proxy_factory_get_proxies+0x7c

And call stack is:

- libproxy.dll!libproxy::url::get_pac()  Line 423   C++

- libproxy.dll!libproxy::proxy_factory::_get_proxies(libproxy::url * realurl, 
std::vector<std::basic_string<char,std::char_traits<char>,std::allocator<char> 
>,std::allocator<std::basic_string<char,std::char_traits<char>,std::allocator<ch
ar> > > > & response)  Line 352 + 0xb bytes C++

- 
libproxy.dll!libproxy::proxy_factory::get_proxies(std::basic_string<char,std::ch
ar_traits<char>,std::allocator<char> > url_)  Line 196  C++

- libproxy.dll!px_proxy_factory_get_proxies(pxProxyFactory_ * self, const char 
* url)  Line 421 + 0x20 bytes   C++

This runtime crash is at same location i.e. while closing socket. I dont know 
the reason of crash behind it.

Original comment by udayja...@gmail.com on 25 Jul 2012 at 4:33

[GoogleCodeExporter]

The are no crash but debug assertion. Unless run with the development version 
of msvcrt, they should not be visible to user. Having these may result in leaks 
(in this case client socket leaks).

My first impression is that this code is not using the windows closesocket() 
function. On windows, unlike all other OS, a socket is not a file descriptor, 
and must be closed using closesocket(). Using normal close() causes debug 
assertion, and leaks the socket.

It would be nice if you split this bug, as you started reporting many things on 
the same bug. Also, have you had time to run the unit test suggested earlier, 
and write a fix to url.cpp to make them work ?

Original comment by nicolas.dufresne@gmail.com on 15 Oct 2012 at 8:27

• Changed title: Runtime debug assertion when using .pac file with file:// protocol

[GoogleCodeExporter]

Also, what I don't get here is why didn't you proposed a patch after testing 
the binary mode fix some of the issue? 

Original comment by nicolas.dufresne@gmail.com on 12 Aug 2015 at 2:19

[GoogleCodeExporter]

Sorry, but I do not have code base with me which contains fix.

Original comment by udayja...@gmail.com on 12 Aug 2015 at 2:37