Skip to content

Commit eae75d6

Browse files
dougbuTratcher
andauthored
Correct ValidateStreamForReading(...) (#390)
- fix #389 - check `Stream` before first read attempt - check `CanRead` (not `CanSeek`) before second or later read attempts - add lots of tests of related scenarios - nit: correct a recent `HttpContentMessageExtensions` comment Apply suggestions from code review - specifically, correct an `HttpMessageContent` comment Co-authored-by: Chris Ross <Tratcher@Outlook.com>
1 parent 30c4ece commit eae75d6

File tree

3 files changed

+283
-29
lines changed

3 files changed

+283
-29
lines changed

src/System.Net.Http.Formatting/HttpContentMessageExtensions.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,8 @@ private static HttpContent CreateHeaderFields(HttpHeaders source, HttpHeaders de
467467
}
468468
}
469469

470-
// If we have content headers then create an HttpContent for this Response
470+
// If we have content headers then create an HttpContent for this request or response. Otherwise,
471+
// provide a HttpContent instance to overwrite the null or EmptyContent value.
471472
if (contentHeaders != null)
472473
{
473474
// Need to rewind the input stream to be at the position right after the HTTP header

src/System.Net.Http.Formatting/HttpMessageContent.cs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,12 +359,21 @@ private byte[] SerializeHeader()
359359

360360
private void ValidateStreamForReading(Stream stream)
361361
{
362+
// Stream is null case should be an extreme, incredibly unlikely corner case. Every HttpContent from
363+
// the framework (see dotnet/runtime or .NET Framework reference source) provides a non-null Stream
364+
// in the ReadAsStreamAsync task's return value. Likely need a poorly-designed derived HttpContent
365+
// to hit this. Mostly ignoring the fact this message doesn't make much sense for the case.
366+
if (stream is null || !stream.CanRead)
367+
{
368+
throw Error.NotSupported(Properties.Resources.NotSupported_UnreadableStream);
369+
}
370+
362371
// If the content needs to be written to a target stream a 2nd time, then the stream must support
363372
// seeking (e.g. a FileStream), otherwise the stream can't be copied a second time to a target
364373
// stream (e.g. a NetworkStream).
365374
if (_contentConsumed)
366375
{
367-
if (stream != null && stream.CanRead)
376+
if (stream.CanSeek)
368377
{
369378
stream.Position = 0;
370379
}

0 commit comments

Comments
 (0)