[PATCH 0/2] MR1528: winhttp: Improve WinHttpGetProxyForUrl() execution time when wpad is not available

[Paul Gofman (＠gofman)]

Fixes Marvel Snap being unable to connect to servers (part of it, another part is wmic patches), and maybe improves wait times in many other apps.

Typically "http://wpad/..." (which is commonly requested as lpszAutoConfigUrl for WinHttpGetProxyForUrl) is not available and WinHttpGetProxyForUrl() is hanging for a really long time resolving this name in download_script().

On Windows 10 here the name resolution timeout is about 9sec (and I could not find a way to configure it for a longer timeout). On Linux default DNS timeouts are typically longer. Yet, while WinHttpGetProxyForUrl() execution time is not consistent here, when I run it right after reboot or after network interface reconnection it typically takes ~4-7 seconds to complete on the first run. Then, testing it again shortly after yields no noticeable timeout. I suppose Windows caches and manages that globally in some service, so maximum possible 9 seconds name resolution timeout is rarely hit. Since proxy config URL, if available, is supposed to be in the local network, having the timeout of 5 seconds for name resolution looks more than enough to me.

[PATCH 1/2] winhttp: Set name resolution timeout in download_script().

[Paul Gofman]

From: Paul Gofman

---
 dlls/winhttp/session.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/dlls/winhttp/session.c b/dlls/winhttp/session.c
index fa8ccd7a9de..3d99e9b0ac6 100644
--- a/dlls/winhttp/session.c
+++ b/dlls/winhttp/session.c
@@ -1922,6 +1922,7 @@ static char *download_script( const WCHAR *url, DWORD *out_size )
     hostname[uc.dwHostNameLength] = 0;

if (!(ses = WinHttpOpen( NULL, WINHTTP_ACCESS_TYPE_NO_PROXY, NULL, NULL, 0 ))) goto done;
+    WinHttpSetTimeouts( ses, 5000, 60000, 30000, 30000 );
     if (!(con = WinHttpConnect( ses, hostname, uc.nPort, 0 ))) goto done;
     if (uc.nScheme == INTERNET_SCHEME_HTTPS) flags |= WINHTTP_FLAG_SECURE;
     if (!(req = WinHttpOpenRequest( con, NULL, uc.lpszUrlPath, NULL, NULL, acceptW, flags ))) goto done;

[PATCH 2/2] winhttp: Cache script in download_script().

[Paul Gofman]

From: Paul Gofman

---
 dlls/winhttp/session.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/dlls/winhttp/session.c b/dlls/winhttp/session.c
index 3d99e9b0ac6..b8a480671de 100644
--- a/dlls/winhttp/session.c
+++ b/dlls/winhttp/session.c
@@ -1903,15 +1903,41 @@ static BOOL parse_script_result( const char *result, WINHTTP_PROXY_INFO *info )

static char *download_script( const WCHAR *url, DWORD *out_size )
 {
+    static SRWLOCK cache_lock = SRWLOCK_INIT;
+    static DWORD cached_script_size;
+    static DWORD cache_update_time;
+    static char *cached_script;
+    static WCHAR *cached_url;
+
     static const WCHAR *acceptW[] = {L"*/*", NULL};
     HINTERNET ses, con = NULL, req = NULL;
     WCHAR *hostname;
     URL_COMPONENTSW uc;
     DWORD status, size = sizeof(status), offset, to_read, bytes_read, flags = 0;
     char *tmp, *buffer = NULL;
+    BOOL cached = FALSE;

*out_size = 0;

+    AcquireSRWLockExclusive( &cache_lock );
+    if (cached_url && !wcscmp( cached_url, url ) && GetTickCount() - cache_update_time < 60000)
+    {
+        cached = TRUE;
+        if (cached_script && (buffer = malloc( cached_script_size )))
+        {
+            memcpy( buffer, cached_script, cached_script_size );
+            *out_size = cached_script_size;
+        }
+    }
+    ReleaseSRWLockExclusive( &cache_lock );
+
+    if (cached)
+    {
+        TRACE( "Returning cached result.\n" );
+        if (!buffer) SetLastError( ERROR_WINHTTP_UNABLE_TO_DOWNLOAD_SCRIPT );
+        return buffer;
+    }
+
     memset( &uc, 0, sizeof(uc) );
     uc.dwStructSize = sizeof(uc);
     uc.dwHostNameLength = -1;
@@ -1957,6 +1983,21 @@ done:
     WinHttpCloseHandle( con );
     WinHttpCloseHandle( ses );
     free( hostname );
+
+    AcquireSRWLockExclusive( &cache_lock );
+    free( cached_url );
+    free( cached_script );
+    cached_url = wcsdup( url );
+    cached_script_size = 0;
+    cached_script = NULL;
+    if (buffer && (cached_script = malloc( *out_size )))
+    {
+        memcpy( cached_script, buffer, *out_size );
+        cached_script_size = *out_size;
+    }
+    cache_update_time = GetTickCount();
+    ReleaseSRWLockExclusive( &cache_lock );
+
     if (!buffer) SetLastError( ERROR_WINHTTP_UNABLE_TO_DOWNLOAD_SCRIPT );
     return buffer;
 }

[Hans Leidekker (＠hans)]

Hans Leidekker (@hans) commented about dlls/winhttp/session.c:
>  char *tmp, *buffer = NULL;
> -
>
> BOOL cached = FALSE;
>
> *out_size = 0;
> -
>
> AcquireSRWLockExclusive( &cache_lock );
> -
>
> if (cached_url && !wcscmp( cached_url, url ) && GetTickCount() - cache_update_time < 60000)
> -
>
> {
> -
>    cached = TRUE;
> -
>    if (cached_script && (buffer = malloc( cached_script_size )))
> -
>    {
> -
>        memcpy( buffer, cached_script, cached_script_size );
> -
>        *out_size = cached_script_size;
> -
>    }
> -
>
> }
> -
>
> ReleaseSRWLockExclusive( &cache_lock );

I guess we should use GetTickCount64() just in case someone runs a process for more than 49 days :-) This function is getting large, maybe you could introduce some helpers to cache and retrieve the script?

[Hans Leidekker (＠hans)]

Hans Leidekker (@hans) commented about dlls/winhttp/session.c:
>  WinHttpCloseHandle( con );
>  WinHttpCloseHandle( ses );
>  free( hostname );
> -
> - AcquireSRWLockExclusive( &cache_lock );
> - free( cached_url );
> - free( cached_script );
> - cached_url = wcsdup( url );
> - cached_script_size = 0;
> - cached_script = NULL;

Please check for allocation failure.