Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redirect to relative url doesn't work with AndroidClientHandler #1923

Closed
dominik-weber opened this issue Jul 4, 2018 · 6 comments
Closed
Assignees
Labels
bug Component does not function as intended. vs-sync For internal use only; creates a VSTS "mirror" issue.
Milestone

Comments

@dominik-weber
Copy link
Contributor

dominik-weber commented Jul 4, 2018

Steps to Reproduce

  1. Execute this code:
var client = new HttpClient(new AndroidClientHandler());
var res = await client.GetStringAsync("https://nghttp2.org/httpbin/redirect-to?url=/httpbin/get");

Expected Behavior

The initial request returns a 302 redirect to relative url: /httpbin/get. This endpoint returns a JSON string.

Actual Behavior

Instead of following the redirect, an exception is thrown:
System.InvalidCastException: Unable to convert instance of type 'Java.Net.URLConnectionInvoker' to type 'Java.Net.HttpURLConnection'.
Please note that the code above works fine when using HttpClientHandler instead of AndroidClientHandler

Version Information

Microsoft Visual Studio Enterprise 2017
Version 15.7.4
VisualStudio.15.Release/15.7.4+27703.2035
Microsoft .NET Framework
Version 4.7.03056

Installed Version: Enterprise

Xamarin 4.10.10.2 (35a01d8dc)
Visual Studio extension to enable development for Xamarin.iOS and Xamarin.Android.

Xamarin Designer 4.12.1 (f3257e429)
Visual Studio extension to enable Xamarin Designer tools in Visual Studio.

Xamarin.Android SDK 8.3.3.2 (HEAD/dffc59120)
Xamarin.Android Reference Assemblies and MSBuild support.

Xamarin.iOS and Xamarin.Mac SDK 11.12.0.4 (64fece5)
Xamarin.iOS and Xamarin.Mac Reference Assemblies and MSBuild support.

VS bug #742257

@dominik-weber
Copy link
Contributor Author

Based on our testing, the issue seems to be caused by the handling of the redirect uri here: https://github.com/xamarin/xamarin-android/blob/master/src/Mono.Android/Xamarin.Android.Net/AndroidClientHandler.cs#L570

Please note that according to the mentioned rfc https://tools.ietf.org/html/rfc3986#section-5 (example in section 5.4) relative uris (starting with /) should be appended to the base uri (ie. the original request uri) following multiple rules, only some of which are currently implemented in AndroidClientHandler.

Please see this reference source here: https://github.com/dotnet/corefx/blob/master/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs#L110
Note that in this implementation the Uri(Uri baseUri, string relativeUri) constructor is used to create the final redirect uri. This constructor seems to handle all of the cases in the RFC correctly (also including the already supported case where the redirect uri starts with //, ie. the protocol of the base uri should be used). See https://msdn.microsoft.com/en-us/library/9hst1w91(v=vs.110).aspx for more info.

Please also note that this issue causes real world problems with the following links not being able to load:

Dropbox share links:
Dropbox uses relative uris to redirect to for file download. You may use this link for testing:
https://www.dropbox.com/s/f1md5z13sgs351k/test-image.jpg?dl=1

Sharepoint online links:
Same as Dropbox, uses relative uris for redirect.
https://crmclient-my.sharepoint.com/:i:/g/personal/d_biecuszek_openasapp_net/ER0uRnZwruZDotjiEypDaN0Bodq_AKCLiUsH583vsE6TAg?download=1

@justintoth
Copy link

+1, seeing the same issue in our mobile app. It only started happening in our latest release, so it seems like something changed recently in the AndroidClientHandler code.

@scriptam
Copy link

scriptam commented Jul 12, 2018

+1, The issue appeared recently when working with services that redirects Json responses in my case. Issue seems to be related to AndroidClientHandler indeed and is observed consistently. Specifically at Xamarin.Android.Net.AndroidClientHandler.SetupRequestInternal

The issue persists on the latest preview release as well.

For anyone else who is experiencing the issue, changing the HttpClientHandler to Managed is a workaround. Not ideal, but a workaround until the issue is resolved.

If you are using the default constructor for HttpClient, you can change the behaviour throughout:
Project Options > Build > Android Build > General > HttpClient implementation = Managed (HttpClientHandler) instead of AndroidClientHandler.

Alternatively, you may switch to Managed handler explicitly, and only when required, instead of setting it project-wide: new HttpClient(new HttpClientHandler())

@dominik-weber
Copy link
Contributor Author

@jonpryor @grendello anybody here?

@raspyweather
Copy link

I had a similar issue recently and was abled to resolve it by using the Managed version of HttpClient and using this nuget https://github.com/tranb3r/secure-httpclient to initialize it

@Nico04
Copy link

Nico04 commented Oct 9, 2018

I've got the exact same issue with url "https://picsum.photos/400/200/?random".
I need to explicitly use HttpClientHandler but it's a workaround...

@JonDouglas JonDouglas added this to the d16-0 milestone Oct 30, 2018
@JonDouglas JonDouglas added the bug Component does not function as intended. label Oct 30, 2018
grendello added a commit to grendello/xamarin-android that referenced this issue Nov 6, 2018
Fixes: dotnet#1923

When a Location header contains an URL that is relative or lacks schema/host, we
now use the original request URL to construct the destination URL. Additionally,
if the original URL contains a fragment but the destination does not, we append
the fragment to the destination.
grendello added a commit to grendello/xamarin-android that referenced this issue Nov 6, 2018
Fixes: dotnet#1923

When a Location header contains an URL that is relative or lacks schema/host, we
now use the original request URL to construct the destination URL. Additionally,
if the original URL contains a fragment but the destination does not, we append
the fragment to the destination.
grendello added a commit to grendello/xamarin-android that referenced this issue Nov 21, 2018
Fixes: dotnet#1923

When a Location header contains an URL that is relative or lacks schema/host, we
now use the original request URL to construct the destination URL. Additionally,
if the original URL contains a fragment but the destination does not, we append
the fragment to the destination.
jonpryor pushed a commit that referenced this issue Nov 28, 2018
Fixes: #1923

Consider this code:

	var client = new HttpClient(new AndroidClientHandler());
	var res = await client.GetStringAsync("https://nghttp2.org/httpbin/redirect-to?url=/httpbin/get");

Accessing `https://nghttp2.org/httpbin/redirect-to?url=/httpbin/get`
results in an HTTP-302 (redirect) to the location `/httpbin/get`:

	$ curl -D - https://nghttp2.org/httpbin/redirect-to?url=/httpbin/get
	HTTP/2 302 
	date: Wed, 28 Nov 2018 20:52:01 GMT
	content-type: text/html; charset=utf-8
	content-length: 0
	location: /httpbin/get
	...

Expected behavior within Xamarin.Android is that this should work:
the HTTP-302 should be followed, with `httpClient.GetStringAsync()`
returning the contents of `https://nghttp2.org/httpbin/get`.

What instead happens is that an exception is thrown:

	System.InvalidCastException: Unable to convert instance of type 'Java.Net.URLConnectionInvoker' to type 'Java.Net.HttpURLConnection'

Oops.

Update `AndroidClientHandler.HandleRedirect()` to improve its support
for the `Location` header, such that if the `Location` header value
contains a URL that is relative or lacks schema/host, we now use the
original request URL to construct the destination URL.  Additionally,
if the original URL contains a fragment but the destination does not,
we append the fragment to the destination.
jonpryor pushed a commit that referenced this issue Dec 6, 2018
Fixes: #1923

Consider this code:

	var client = new HttpClient(new AndroidClientHandler());
	var res = await client.GetStringAsync("https://nghttp2.org/httpbin/redirect-to?url=/httpbin/get");

Accessing `https://nghttp2.org/httpbin/redirect-to?url=/httpbin/get`
results in an HTTP-302 (redirect) to the location `/httpbin/get`:

	$ curl -D - https://nghttp2.org/httpbin/redirect-to?url=/httpbin/get
	HTTP/2 302 
	date: Wed, 28 Nov 2018 20:52:01 GMT
	content-type: text/html; charset=utf-8
	content-length: 0
	location: /httpbin/get
	...

Expected behavior within Xamarin.Android is that this should work:
the HTTP-302 should be followed, with `httpClient.GetStringAsync()`
returning the contents of `https://nghttp2.org/httpbin/get`.

What instead happens is that an exception is thrown:

	System.InvalidCastException: Unable to convert instance of type 'Java.Net.URLConnectionInvoker' to type 'Java.Net.HttpURLConnection'

Oops.

Update `AndroidClientHandler.HandleRedirect()` to improve its support
for the `Location` header, such that if the `Location` header value
contains a URL that is relative or lacks schema/host, we now use the
original request URL to construct the destination URL.  Additionally,
if the original URL contains a fragment but the destination does not,
we append the fragment to the destination.
@pjcollins pjcollins added the vs-sync For internal use only; creates a VSTS "mirror" issue. label Dec 6, 2018
@xamarin-release-manager xamarin-release-manager modified the milestones: d16-0, d15-9 Dec 7, 2018
marcelltoth added a commit to marcelltoth/NeptunLight that referenced this issue Jan 7, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Jun 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Component does not function as intended. vs-sync For internal use only; creates a VSTS "mirror" issue.
Projects
None yet
Development

No branches or pull requests

9 participants