Skip to content

Commit

Permalink
Flesh out ByteRangeStream methods to make it always limit reads (#213)
Browse files Browse the repository at this point in the history
* Flesh out `ByteRangeStream` methods to make it always limit reads
- #206
- `Seek(...)` was a no-op, `Position` was incorrect if `_lowerbounds != 0`, `ReadAsync(...)` read entire inner `Stream`
  - rewrite `ByteRangeStreamTest` to cover new `ByteRangeStream` members and hold fewer resources
- remove `DelegatingStream.CopyToAsync(...)` override because it copied entire inner `Stream` and was not needed
  - base implementation invokes `ReadAsync(...)`
  • Loading branch information
dougbu authored Nov 28, 2018
1 parent cfda5e5 commit 39d3064
Show file tree
Hide file tree
Showing 5 changed files with 582 additions and 78 deletions.
94 changes: 80 additions & 14 deletions src/System.Net.Http.Formatting/Internal/ByteRangeStream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,22 @@

using System.IO;
using System.Net.Http.Headers;
using System.Threading;
using System.Threading.Tasks;
using System.Web.Http;

namespace System.Net.Http.Internal
{
/// <summary>
/// Stream which only exposes a read-only only range view of an
/// Stream which only exposes a read-only only range view of an
/// inner stream.
/// </summary>
internal class ByteRangeStream : DelegatingStream
{
// The offset stream position at which the range starts.
private readonly long _lowerbounds;

// The total number of bytes within the range.
// The total number of bytes within the range.
private readonly long _totalCount;

// The current number of bytes read into the range
Expand Down Expand Up @@ -92,6 +94,23 @@ public override bool CanWrite
get { return false; }
}

public override long Position
{
get
{
return _currentCount;
}
set
{
if (value < 0)
{
throw Error.ArgumentMustBeGreaterThanOrEqualTo("value", value, 0L);
}

_currentCount = value;
}
}

public override IAsyncResult BeginRead(byte[] buffer, int offset, int count, AsyncCallback callback, object state)
{
return base.BeginRead(buffer, offset, PrepareStreamForRangeRead(count), callback, state);
Expand All @@ -102,16 +121,47 @@ public override int Read(byte[] buffer, int offset, int count)
return base.Read(buffer, offset, PrepareStreamForRangeRead(count));
}

public override Task<int> ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
{
return base.ReadAsync(buffer, offset, PrepareStreamForRangeRead(count), cancellationToken);
}

public override int ReadByte()
{
int effectiveCount = PrepareStreamForRangeRead(1);
if (effectiveCount <= 0)
{
return -1;
}

return base.ReadByte();
}

public override long Seek(long offset, SeekOrigin origin)
{
switch (origin)
{
case SeekOrigin.Begin:
_currentCount = offset;
break;
case SeekOrigin.Current:
_currentCount = _currentCount + offset;
break;
case SeekOrigin.End:
_currentCount = _totalCount + offset;
break;
default:
throw Error.InvalidEnumArgument("origin", (int)origin, typeof(SeekOrigin));
}

if (_currentCount < 0L)
{
throw new IOException(Properties.Resources.ByteRangeStreamInvalidOffset);
}

return _currentCount;
}

public override void SetLength(long value)
{
throw Error.NotSupported(Properties.Resources.ByteRangeStreamReadOnly);
Expand All @@ -132,33 +182,49 @@ public override void EndWrite(IAsyncResult asyncResult)
throw Error.NotSupported(Properties.Resources.ByteRangeStreamReadOnly);
}

public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
{
throw Error.NotSupported(Properties.Resources.ByteRangeStreamReadOnly);
}

public override void WriteByte(byte value)
{
throw Error.NotSupported(Properties.Resources.ByteRangeStreamReadOnly);
}

/// <summary>
/// Gets the
/// Gets the correct count for the next read operation.
/// </summary>
/// <param name="count">The count requested to be read by the caller.</param>
/// <returns>The remaining bytes to read within the range defined for this stream.</returns>
private int PrepareStreamForRangeRead(int count)
{
long effectiveCount = Math.Min(count, _totalCount - _currentCount);
if (effectiveCount > 0)
// A negative count causes base.Raad* methods to throw an ArgumentOutOfRangeException.
if (count <= 0)
{
// Check if we should update the stream position
long position = InnerStream.Position;
if (_lowerbounds + _currentCount != position)
{
InnerStream.Position = _lowerbounds + _currentCount;
}
return count;
}

// Update current number of bytes read
_currentCount += effectiveCount;
// Reading past the end simply does nothing.
if (_currentCount >= _totalCount)
{
return 0;
}

// Effective count can never be bigger than int
long effectiveCount = Math.Min(count, _totalCount - _currentCount);

// Check if we should update the inner stream's position.
var newPosition = _lowerbounds + _currentCount;
var position = InnerStream.Position;
if (newPosition != position)
{
InnerStream.Position = newPosition;
}

// Update current number of bytes read.
_currentCount += effectiveCount;

// Effective count can never be bigger than int.
return (int)effectiveCount;
}
}
Expand Down
9 changes: 2 additions & 7 deletions src/System.Net.Http.Formatting/Internal/DelegatingStream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@
namespace System.Net.Http.Internal
{
/// <summary>
/// Stream that delegates to inner stream.
/// Stream that delegates to inner stream.
/// This is taken from System.Net.Http
/// </summary>
internal abstract class DelegatingStream : Stream
{
private Stream _innerStream;
private readonly Stream _innerStream;

protected DelegatingStream(Stream innerStream)
{
Expand Down Expand Up @@ -119,11 +119,6 @@ public override void Flush()
_innerStream.Flush();
}

public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken)
{
return _innerStream.CopyToAsync(destination, bufferSize, cancellationToken);
}

public override Task FlushAsync(CancellationToken cancellationToken)
{
return _innerStream.FlushAsync(cancellationToken);
Expand Down
11 changes: 10 additions & 1 deletion src/System.Net.Http.Formatting/Properties/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/System.Net.Http.Formatting/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -339,4 +339,7 @@
<data name="RemoteStreamInfoCannotBeNull" xml:space="preserve">
<value>The '{0}' method in '{1}' returned null. It must return a RemoteStreamResult instance containing a writable stream and a valid URL.</value>
</data>
<data name="ByteRangeStreamInvalidOffset" xml:space="preserve">
<value>An attempt was made to move the position before the beginning of the stream.</value>
</data>
</root>
Loading

0 comments on commit 39d3064

Please sign in to comment.