1

Closed

Invalid handling of http proxy server response

description

Hi,

I did find a problem when testing the assembly with our squid 3.0 proxy. I receive the message, that Result cannot be called on a failed match. Therefore I've extended the code in Session.cs a little bit:
            if (statusCode == 200 && string.IsNullOrEmpty(response))
            {
                //  Once all HTTP header information is read, exit
                break;
            }
            else
            {
                string reasonPhrase = string.Empty;

                try
                {
                    if (match.Success)
                        reasonPhrase = match.Result("${reasonPhrase}");
                }
                catch (Exception ex)
                {
                    reasonPhrase = string.Empty;
                }
Now the following line

throw new ProxyException(string.Format("HTTP: Status code {0}, Reason \"{1}\"", statusCode, reasonPhrase));

Does at least return the error code (not yet the fault reason; in this case ERR_ACCESS_DENIED)

In fact the code received a 403 status code from the proxy:

SshNet.Logging Verbose: 1 : Initiating connect to 'yyy.xxx.zzz.ww:22'.

HTTP/1.0 403 Forbidden
Server: squid/3.0.STABLE16-RC1
Mime-Version: 1.0
Date: Wed, 13 Feb 2013 14:15:06 GMT
Content-Type: text/html
Content-Length: 1008
Expires: Wed, 13 Feb 2013 14:15:06 GMT
X-Squid-Error: ERR_ACCESS_DENIED 0
X-Cache: MISS from app-proxy-3.xxxx.xxx
Via: 1.0 app-proxy-3.xxxx.xxx (squid/3.0.STABLE16-RC1)
Proxy-Connection: close

The thread 0xa2c has exited with code 0 (0x0).

Kind regards

Johannes

file attachments

Closed Apr 6 at 2:56 PM by drieseng

comments

JohRest wrote Feb 13, 2013 at 2:17 PM

Hi,

what do you think about this alternative implementation?

Just as a discussion Point....

Regards

Johannes

olegkap wrote Feb 14, 2013 at 1:39 PM

Hi,

Thanks for pointing this out.
Can you probably provide more information under which .Result can be called prior calling .Match method?
After checking my code I noticed that I always test if match was Success before calling Result method.

May be there is something I overlooked.

Thanks,
Oleg

JohRest wrote Feb 15, 2013 at 1:06 PM

Hi Oleg,

I was just checking the code while connecting to our squit http proxy. (3.0). During connection communication I noticed that there was a 403 coming back over the wire. So I found in Session.cs (copied from codeplex directly):

// Read response body if specified
            if (string.IsNullOrEmpty(response) && contentLength > 0)
            {
                var contentBody = new byte[contentLength];
                this.SocketRead(contentLength, ref contentBody);
            }

            if (statusCode == 200 && string.IsNullOrEmpty(response))
            {
                //  Once all HTTP header information is read, exit
                break;
            }
            else
            {
                var reasonPhrase = match.Result("${reasonPhrase}");
                throw new ProxyException(string.Format("HTTP: Status code {0}, Reason \"{1}\"", statusCode, reasonPhrase));
            }
In the last else branch you're trying to match

var reasonPhrase = match.Result("${reasonPhrase}");

on a response that by that time is empty and therefore I suggested the code change. At that point in time the method already "knows" that there's an error condition (!= 200) and thus the error description is clear and there (in my opinion) is no need to use regex to try to fiddle it out of the server response.

The method just has to translate the error code 403 into something readable to return it to the user. At the moment the user just receives a "A result cannot be called on a failed match" as exception, what is not the "real" exception and could confuse the caller (as it did when I first received that exception)

Kind regards

Johannes

olegkap wrote Mar 2, 2013 at 1:00 PM

I applied this fix in 23335 commit, with some minor modification, I did it actually a while ago but forgot to commit it so can you please take a look and see if it works?

Thanks,
Oleg