From f893057a01bfd1260b9d5693840ea47a5673acbc Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Fri, 26 May 2017 12:34:40 +0200 Subject: [PATCH] Rewind POST content stream before using it again If a POST request is redirected the redirection happens after the data has already been read from the request content object and thus the stream it was read from is positioned at its end. Since redirect reuses the request content object it can end up using the same Stream instance as in the previous request (the request content object might be caching it) and would end up getting an exception since the stream would be considered "read" or "closed. There are two possible fixes for this issue. One is to cache the POST content in a local buffer/stream and the other is to rewind the stream to beginning after reading from it in the original request. The first fix should be avoided in our scenario as the handler runs on memory constrained devices and should not use too much memory, and that may very well happen if the POST is sending a large file to the service. The second fix will fail if the stream isn't seekable, but hopefully this will happen few and far between, if at all. The advantage of this fix is that it is compatible with any way the HttpContent used in request handles its internal streams. For these reasons this commit implements the second fix. Additionally, this commit moves the code to a virtual method that can be overriden by anyone who needs to implement the POST data copying in a different way than implemented by us. Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=55477 --- .../AndroidClientHandler.cs | 36 +++++++++++++++---- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/src/Mono.Android/Xamarin.Android.Net/AndroidClientHandler.cs b/src/Mono.Android/Xamarin.Android.Net/AndroidClientHandler.cs index e1049090891..036634954fb 100644 --- a/src/Mono.Android/Xamarin.Android.Net/AndroidClientHandler.cs +++ b/src/Mono.Android/Xamarin.Android.Net/AndroidClientHandler.cs @@ -287,6 +287,34 @@ Task ConnectAsync (HttpURLConnection httpConnection, CancellationToken ct) }, ct); } + protected virtual async Task WriteRequestContentToOutput (HttpRequestMessage request, HttpURLConnection httpConnection, CancellationToken cancellationToken) + { + using (var stream = await request.Content.ReadAsStreamAsync ().ConfigureAwait (false)) { + await stream.CopyToAsync(httpConnection.OutputStream, 4096, cancellationToken).ConfigureAwait(false); + + // + // Rewind the stream to beginning in case the HttpContent implementation + // will be accessed again (e.g. after redirect) and it keeps its stream + // open behind the scenes instead of recreating it on the next call to + // ReadAsStreamAsync. If we don't rewind it, the ReadAsStreamAsync + // call above will throw an exception as we'd be attempting to read an + // already "closed" stream (that is one whose Position is set to its + // end). + // + // This is not a perfect solution since the HttpContent may do weird + // things in its implementation, but it's better than copying the + // content into a buffer since we have no way of knowing how the data is + // read or generated and also we don't want to keep potentially large + // amounts of data in memory (which would happen if we read the content + // into a byte[] buffer and kept it cached for re-use on redirect). + // + // See https://bugzilla.xamarin.com/show_bug.cgi?id=55477 + // + if (stream.CanSeek) + stream.Seek (0, SeekOrigin.Begin); + } + } + async Task DoProcessRequest (HttpRequestMessage request, URL javaUrl, HttpURLConnection httpConnection, CancellationToken cancellationToken, RequestRedirectionState redirectState) { if (Logger.LogNet) @@ -333,12 +361,8 @@ Task ConnectAsync (HttpURLConnection httpConnection, CancellationToken ct) }, TaskScheduler.Default); }, useSynchronizationContext: false); - if (httpConnection.DoOutput) { - using (var stream = await request.Content.ReadAsStreamAsync ().ConfigureAwait (false)) { - await stream.CopyToAsync(httpConnection.OutputStream, 4096, cancellationToken) - .ConfigureAwait(false); - } - } + if (httpConnection.DoOutput) + await WriteRequestContentToOutput (request, httpConnection, cancellationToken); statusCode = await Task.Run (() => (HttpStatusCode)httpConnection.ResponseCode).ConfigureAwait (false); connectionUri = new Uri (httpConnection.URL.ToString ());