Skip to content

Commit efcd007

Browse files
[Mono.Android] Do not dispose request content stream in AndroidMessageHandler (#8764)
Fixes: #2901 Fixes: #4476 Fixes: #7086 Neither the `SocketsHttpHandler` nor the `iOS/macOS` `NSUrlSessionHandler` dispose the content stream, let's follow suit.
1 parent 8c7cf91 commit efcd007

File tree

2 files changed

+48
-24
lines changed

2 files changed

+48
-24
lines changed

src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -536,30 +536,29 @@ protected virtual async Task WriteRequestContentToOutput (HttpRequestMessage req
536536
if (request.Content is null)
537537
return;
538538

539-
using (var stream = await request.Content.ReadAsStreamAsync ().ConfigureAwait (false)) {
540-
await stream.CopyToAsync(httpConnection.OutputStream!, 4096, cancellationToken).ConfigureAwait(false);
541-
542-
//
543-
// Rewind the stream to beginning in case the HttpContent implementation
544-
// will be accessed again (e.g. after redirect) and it keeps its stream
545-
// open behind the scenes instead of recreating it on the next call to
546-
// ReadAsStreamAsync. If we don't rewind it, the ReadAsStreamAsync
547-
// call above will throw an exception as we'd be attempting to read an
548-
// already "closed" stream (that is one whose Position is set to its
549-
// end).
550-
//
551-
// This is not a perfect solution since the HttpContent may do weird
552-
// things in its implementation, but it's better than copying the
553-
// content into a buffer since we have no way of knowing how the data is
554-
// read or generated and also we don't want to keep potentially large
555-
// amounts of data in memory (which would happen if we read the content
556-
// into a byte[] buffer and kept it cached for re-use on redirect).
557-
//
558-
// See https://bugzilla.xamarin.com/show_bug.cgi?id=55477
559-
//
560-
if (stream.CanSeek)
561-
stream.Seek (0, SeekOrigin.Begin);
562-
}
539+
var stream = await request.Content.ReadAsStreamAsync ().ConfigureAwait (false);
540+
await stream.CopyToAsync(httpConnection.OutputStream!, 4096, cancellationToken).ConfigureAwait(false);
541+
542+
//
543+
// Rewind the stream to beginning in case the HttpContent implementation
544+
// will be accessed again (e.g. after redirect) and it keeps its stream
545+
// open behind the scenes instead of recreating it on the next call to
546+
// ReadAsStreamAsync. If we don't rewind it, the ReadAsStreamAsync
547+
// call above will throw an exception as we'd be attempting to read an
548+
// already "closed" stream (that is one whose Position is set to its
549+
// end).
550+
//
551+
// This is not a perfect solution since the HttpContent may do weird
552+
// things in its implementation, but it's better than copying the
553+
// content into a buffer since we have no way of knowing how the data is
554+
// read or generated and also we don't want to keep potentially large
555+
// amounts of data in memory (which would happen if we read the content
556+
// into a byte[] buffer and kept it cached for re-use on redirect).
557+
//
558+
// See https://bugzilla.xamarin.com/show_bug.cgi?id=55477
559+
//
560+
if (stream.CanSeek)
561+
stream.Seek (0, SeekOrigin.Begin);
563562
}
564563

565564
internal Task WriteRequestContentToOutputInternal (HttpRequestMessage request, HttpURLConnection httpConnection, CancellationToken cancellationToken)

tests/Mono.Android-Tests/Xamarin.Android.Net/AndroidMessageHandlerTests.cs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,31 @@ async Task<bool> DoDecompression (string urlPath, string encoding, string jsonFi
9898
return true;
9999
}
100100

101+
[Test]
102+
public async Task DoesNotDisposeContentStream()
103+
{
104+
using var listener = new HttpListener ();
105+
listener.Prefixes.Add ("http://+:47663/");
106+
listener.Start ();
107+
listener.BeginGetContext (ar => {
108+
var ctx = listener.EndGetContext (ar);
109+
ctx.Response.StatusCode = 204;
110+
ctx.Response.ContentLength64 = 0;
111+
ctx.Response.Close ();
112+
}, null);
113+
114+
var jsonContent = new StringContent ("hello");
115+
var request = new HttpRequestMessage (HttpMethod.Post, "http://localhost:47663/") { Content = jsonContent };
116+
117+
var response = await new HttpClient (new AndroidMessageHandler ()).SendAsync (request);
118+
Assert.True (response.IsSuccessStatusCode);
119+
120+
var contentValue = await jsonContent.ReadAsStringAsync ();
121+
Assert.AreEqual ("hello", contentValue);
122+
123+
listener.Close ();
124+
}
125+
101126
[Test]
102127
public async Task ServerCertificateCustomValidationCallback_ApproveRequest ()
103128
{

0 commit comments

Comments
 (0)