bittercoder / DevDefined.OAuth

An OAuth Consumer and Provider implemented for .Net
http://code.google.com/p/devdefined-tools/wiki/OAuth
162 stars 70 forks source link

WebException.GetResponseStream always throws #3

Closed bittercoder closed 11 years ago

bittercoder commented 14 years ago

What steps will reproduce the problem?

  1. Use 2 way auth with X509 certificate
  2. Make an invalid request to a server with valid credentials
  3. Server returns error 400 with a response body (in my case xml describing the error)
  4. In the catch section, attempt to use the following code
if (ex.Response != null)
{
    using (StreamReader streamReader = new StreamReader(ex.Response.GetResponseStream()))
    {
        Response = streamReader.ReadToEnd();
    }

    if (ex.Response is HttpWebResponse)
    {
        StatusCode = ((HttpWebResponse)ex.Response).StatusCode;
    }
}

What is the expected output? What do you see instead?

I expect the ability to read from the stream. I get an exception instead

What version of the product are you using? On what operating system? Latest trunk

Please provide any additional information below.

This appears to be a bug from the implementation of WebExceptionHelper.cs. The helper reads the content stream before the exception is re-thrown to the caller. As a result, the stream has been read to the end, and I'm unable to retrieve the data I need. I'll be modifying the exception helper to resolve this issue.

Reported by todd@spidertracks.co.nz, Mar 31, 2010

bittercoder commented 14 years ago

Todd:

I've attached the patch file. This wraps WebException, but I don't think this is ideal. Perhaps a better solution would be to expose a public API that allows users to pass a string in and be returned an OAuthException. This way uses can still have the choice of standard web exceptions, or using the internal API's to wrap WebExceptions with more specific oAuth help.

Patch is here: http://devdefined-tools.googlecode.com/issues/attachment?aid=2966971684363880305&name=exception-wrapping.patch&token=43ad6f4455741fbbd7a5739d7383ce9f

bittercoder commented 14 years ago

Thanks for the patch!

I wonder if perhaps this logic shouldn't be contained in some kind of "ExceptionHandlingPolicy" that can be configured just once and then used for all subsequent requests/responses - as I suspect it might not be a "one-size-fits-all" situation, like you say. Especially if they're working against a less then ideal OAuth server-side implementation (i.e. if it's not implementing OAuth error reporting).

What do you think?