mattjohnsonpint / DotNetOpenAuth.GoogleOAuth2

DotNetOpenAuth OAuth2 Client for Google
23 stars 18 forks source link

Service login url query string parameter values are not being url encoded in mono #6

Closed farleigh closed 10 years ago

farleigh commented 10 years ago

I'm seeing an issue in Mono (mono 3.4.0, env: Ubuntu 14.04, nginx 1.6.0, mono-fastcgi-server4) where Google does not return the name of the provider when redirects to the returnUrl provided by DotNetOpenAuth.GoogleOAuth2

I traced the issue down to a difference in how Mono vs. the Microsoft .Net runtime seems to be url-encoding values in HttpValueCollection.ToString().

DotNetOpenAuth.GoogleOAuth2.GoogleOAuth2Client.GetServiceLoginUrl calls static method BuildUri. BuildUri() creates a new HttpValueCollection via HttpUtility.ParseQueryString(string.Empty), adds parameters to it, and then calls a ToString on it. At this point, the difference occurs.

To show this difference:

static void Main(string[] args)
{
    var queryStringNameValues = HttpUtility.ParseQueryString(string.Empty);
    queryStringNameValues.Add("ReturnUrl", @"http://localhost/login/authenticate?ReturnUrl=http://localhost/secured_area&__provider__=google");
    System.Console.WriteLine("HttpValueCollection to string = {0}", queryStringNameValues.ToString());
}

Running in Microsoft .Net runtime outputs:

HttpValueCollection to string = ReturnUrl=http%3a%2f%2flocalhost%2flogin%2fauthe
nticate%3fReturnUrl%3dhttp%3a%2f%2flocalhost%2fsecured_area%26__provider__%3dgoo
gle

Running in Mono (3.4.0) runtime outputs:

HttpValueCollection to string = ReturnUrl=http://localhost/login/authenticate?ReturnUrl=http://localhost/secured_area&__provider__=google

I filed a bug for the mono team (bug 22557), but may also be worth fixing here also.

mattjohnsonpint commented 10 years ago

It would seems that mono's HttpQSCollection class doesn't URL-encode parameter values, while Microsoft's implementation does.

Mono should probably fix their implementation. I can't just URL-encode things on my side, or it will double the encoding effect. I suppose I could rewrite BuildUri to not use HttpUtility.

mattjohnsonpint commented 10 years ago

Deployed in version 1.1.2