[Bug 16831] NETCON_secure_connect SSL_connect failed

http://bugs.winehq.org/show_bug.cgi?id=16831

--- Comment #9 from TJ   2009-01-07 13:21:23 ---
It looks as if the suggested patch contained W.I.P. for setupapi, which I
removed.

When tested the trace shows:

err:wininet:NETCON_secure_connect SSL_connect failed: error:140770FC:SSL
routines:SSL23_GET_SERVER_HELLO:unknown protocol

I've been reading the source-code and there's some things that don't seem quite
right, but as I'm still getting the feel for it you may know better.

It looks as if the patch won't take effect because in
INTERNET_InternetOpenUrlW(), the result of

if (urlComponents.nPort == 0)

will be false since the previous call to InternetCrackUrlW() initially sets
nPort to 0:

lpUC->nPort = INTERNET_INVALID_PORT_NUMBER

but later assigns the default port to nPort based on the Scheme when no port is
specified in the URI:

if (lpszPort != lpszNetLoc)
                lpUC->nPort = atoiW(++lpszPort);
        else switch (lpUC->nScheme)
        {
        case INTERNET_SCHEME_HTTP:
                lpUC->nPort = INTERNET_DEFAULT_HTTP_PORT;
                break;
        case INTERNET_SCHEME_HTTPS:
                lpUC->nPort = INTERNET_DEFAULT_HTTPS_PORT;
                break;

This suggests that the patch logic, and the code it replaced, was unreachable
since nPort was always being set by InternetCrackUrlW().

An obvious solution for this bug would be to remove the HTTP/HTTPS default
assignments in InternetCrackUrlW() but it looks likely that the function is
relied on by many other callers.

I've been considering various alternatives. The key problem is how, in
INTERNET_InternetOpenUrlW(), to detect when InternetCrackUrlW() has applied the
default values:

1. Set a flag in InternetCrackUrlW() when the defaults are used, that is
checked upon return. The problem with this would be where that flag would live.
There isn't any obvious spare capacity in URL_COMPONENTS that wouldn't affect
other callers.

2. Locate the returned UrlComponents.lpszHostName in the original lpszUrl and
check if it is followed by a port specifier ( :[:digit:]{1,5} ). If not, apply
the SSL nPort over-ride if the INTERNET_FLAG_SECURE flag is set and the scheme
is INTERNET_SCHEME_HTTP.

3. Detect the scheme in lpszUrl and manually over-ride UrlComponents.nScheme
after calling InternetCrackUrlW() if the INTERNET_FLAG_SECURE flag is set, then
detect whether a port-specifier exists in lpszUrl and if not adjust
UrlComponents.nPort.

4. Massage the original lpszUrl (make a copy) *before* the call to
InternetCrackUrlW() to alter the scheme string to "https" (if it isn't already)
when the INTERNET_FLAG_SECURE is set.

Personally, I prefer option 4 since it is least invasive and simplest to code.