From ad8338e1e8a24682927ac305fd24da613b47ba43 Mon Sep 17 00:00:00 2001 From: Jass Bagga Date: Tue, 6 Jun 2017 11:22:31 -0700 Subject: [PATCH] Refactor RangeHelper (#200) Addresses #196 --- .../RangeHelper.cs | 101 +++------- .../StaticFileContext.cs | 55 +++-- .../StaticFileMiddleware.cs | 7 +- .../RangeHelperTests.cs | 189 +++--------------- 4 files changed, 88 insertions(+), 264 deletions(-) diff --git a/shared/Microsoft.AspNetCore.RangeHelper.Sources/RangeHelper.cs b/shared/Microsoft.AspNetCore.RangeHelper.Sources/RangeHelper.cs index 07f9905490..64f0e2a607 100644 --- a/shared/Microsoft.AspNetCore.RangeHelper.Sources/RangeHelper.cs +++ b/shared/Microsoft.AspNetCore.RangeHelper.Sources/RangeHelper.cs @@ -2,8 +2,8 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Collections.Generic; using System.Diagnostics; +using System.Linq; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Headers; using Microsoft.Extensions.Primitives; @@ -17,19 +17,25 @@ namespace Microsoft.AspNetCore.Internal internal static class RangeHelper { /// - /// Returns the requested range if the Range Header in the is valid. + /// Returns the normalized form of the requested range if the Range Header in the is valid. /// /// The associated with the request. /// The associated with the given . - /// The representation of the last modified date of the file. - /// The provided in the . - /// A collection of containing the ranges parsed from the . - public static ICollection ParseRange(HttpContext context, RequestHeaders requestHeaders, DateTimeOffset? lastModified = null, EntityTagHeaderValue etag = null) + /// The total length of the file representation requested. + /// A boolean value which represents if the contain a single valid + /// range request. A which represents the normalized form of the + /// range parsed from the or null if it cannot be normalized. + /// If the Range header exists but cannot be parsed correctly, or if the provided length is 0, then the range request cannot be satisfied (status 416). + /// This results in (true,null) return values. + public static (bool, RangeItemHeaderValue) ParseRange( + HttpContext context, + RequestHeaders requestHeaders, + long length) { var rawRangeHeader = context.Request.Headers[HeaderNames.Range]; if (StringValues.IsNullOrEmpty(rawRangeHeader)) { - return null; + return (false, null); } // Perf: Check for a single entry before parsing it @@ -38,98 +44,44 @@ namespace Microsoft.AspNetCore.Internal // The spec allows for multiple ranges but we choose not to support them because the client may request // very strange ranges (e.g. each byte separately, overlapping ranges, etc.) that could negatively // impact the server. Ignore the header and serve the response normally. - return null; + return (false, null); } var rangeHeader = requestHeaders.Range; if (rangeHeader == null) { // Invalid - return null; + return (false, null); } // Already verified above Debug.Assert(rangeHeader.Ranges.Count == 1); - // 14.27 If-Range - var ifRangeHeader = requestHeaders.IfRange; - if (ifRangeHeader != null) - { - // If the validator given in the If-Range header field matches the - // current validator for the selected representation of the target - // resource, then the server SHOULD process the Range header field as - // requested. If the validator does not match, the server MUST ignore - // the Range header field. - bool ignoreRangeHeader = false; - if (ifRangeHeader.LastModified.HasValue) - { - if (lastModified.HasValue && lastModified > ifRangeHeader.LastModified) - { - ignoreRangeHeader = true; - } - } - else if (etag != null && ifRangeHeader.EntityTag != null && !ifRangeHeader.EntityTag.Compare(etag, useStrongComparison: true)) - { - ignoreRangeHeader = true; - } - - if (ignoreRangeHeader) - { - return null; - } - } - - return rangeHeader.Ranges; - } - - /// - /// A helper method to normalize a collection of s. - /// - /// A collection of to normalize. - /// The total length of the file representation requested. - /// A normalized list of s. - // 14.35.1 Byte Ranges - If a syntactically valid byte-range-set includes at least one byte-range-spec whose - // first-byte-pos is less than the current length of the entity-body, or at least one suffix-byte-range-spec - // with a non-zero suffix-length, then the byte-range-set is satisfiable. - // Adjusts ranges to be absolute and within bounds. - public static IList NormalizeRanges(ICollection ranges, long length) - { + var ranges = rangeHeader.Ranges; if (ranges == null) { - return null; + return (false, null); } if (ranges.Count == 0) { - return Array.Empty(); + return (true, null); } if (length == 0) { - return Array.Empty(); + return (true, null); } - var normalizedRanges = new List(ranges.Count); - foreach (var range in ranges) - { - var normalizedRange = NormalizeRange(range, length); + // Normalize the ranges + var range = NormalizeRange(ranges.SingleOrDefault(), length); - if (normalizedRange != null) - { - normalizedRanges.Add(normalizedRange); - } - } - - return normalizedRanges; + // Return the single range + return (true, range); } - /// - /// A helper method to normalize a . - /// - /// The to normalize. - /// The total length of the file representation requested. - /// A normalized . - public static RangeItemHeaderValue NormalizeRange(RangeItemHeaderValue range, long length) + // Internal for testing + internal static RangeItemHeaderValue NormalizeRange(RangeItemHeaderValue range, long length) { var start = range.From; var end = range.To; @@ -161,8 +113,7 @@ namespace Microsoft.AspNetCore.Internal end = start + bytes - 1; } - var normalizedRange = new RangeItemHeaderValue(start, end); - return normalizedRange; + return new RangeItemHeaderValue(start, end); } } } diff --git a/src/Microsoft.AspNetCore.StaticFiles/StaticFileContext.cs b/src/Microsoft.AspNetCore.StaticFiles/StaticFileContext.cs index 7c6e8f81b3..1f04360dd5 100644 --- a/src/Microsoft.AspNetCore.StaticFiles/StaticFileContext.cs +++ b/src/Microsoft.AspNetCore.StaticFiles/StaticFileContext.cs @@ -2,8 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Collections.Generic; -using System.Diagnostics; using System.IO; using System.Linq; using System.Threading; @@ -49,7 +47,8 @@ namespace Microsoft.AspNetCore.StaticFiles private PreconditionState _ifModifiedSinceState; private PreconditionState _ifUnmodifiedSinceState; - private IList _ranges; + private RangeItemHeaderValue _range; + private bool _isRangeRequest; public StaticFileContext(HttpContext context, StaticFileOptions options, PathString matchUrl, ILogger logger, IFileProvider fileProvider, IContentTypeProvider contentTypeProvider) { @@ -77,7 +76,8 @@ namespace Microsoft.AspNetCore.StaticFiles _ifNoneMatchState = PreconditionState.Unspecified; _ifModifiedSinceState = PreconditionState.Unspecified; _ifUnmodifiedSinceState = PreconditionState.Unspecified; - _ranges = null; + _range = null; + _isRangeRequest = false; } internal enum PreconditionState @@ -85,7 +85,7 @@ namespace Microsoft.AspNetCore.StaticFiles Unspecified, NotModified, ShouldProcess, - PreconditionFailed, + PreconditionFailed } public bool IsHeadMethod @@ -95,7 +95,7 @@ namespace Microsoft.AspNetCore.StaticFiles public bool IsRangeRequest { - get { return _ranges != null; } + get { return _isRangeRequest; } } public string SubPath @@ -162,6 +162,8 @@ namespace Microsoft.AspNetCore.StaticFiles ComputeIfModifiedSince(); ComputeRange(); + + ComputeIfRange(); } private void ComputeIfMatch() @@ -218,6 +220,31 @@ namespace Microsoft.AspNetCore.StaticFiles } } + private void ComputeIfRange() + { + // 14.27 If-Range + var ifRangeHeader = _requestHeaders.IfRange; + if (ifRangeHeader != null) + { + // If the validator given in the If-Range header field matches the + // current validator for the selected representation of the target + // resource, then the server SHOULD process the Range header field as + // requested. If the validator does not match, the server MUST ignore + // the Range header field. + if (ifRangeHeader.LastModified.HasValue) + { + if (_lastModified !=null && _lastModified > ifRangeHeader.LastModified) + { + _isRangeRequest = false; + } + } + else if (_etag != null && ifRangeHeader.EntityTag != null && !ifRangeHeader.EntityTag.Compare(_etag, useStrongComparison: true)) + { + _isRangeRequest = false; + } + } + } + private void ComputeRange() { // 14.35 Range @@ -230,8 +257,7 @@ namespace Microsoft.AspNetCore.StaticFiles return; } - var parsedRange = RangeHelper.ParseRange(_context, _requestHeaders, _lastModified, _etag); - _ranges = RangeHelper.NormalizeRanges(parsedRange, _length); + (_isRangeRequest, _range) = RangeHelper.ParseRange(_context, _requestHeaders, _length); } public void ApplyResponseHeaders(int statusCode) @@ -322,13 +348,7 @@ namespace Microsoft.AspNetCore.StaticFiles // When there is only a single range the bytes are sent directly in the body. internal async Task SendRangeAsync() { - bool rangeNotSatisfiable = false; - if (_ranges.Count == 0) - { - rangeNotSatisfiable = true; - } - - if (rangeNotSatisfiable) + if (_range == null) { // 14.16 Content-Range - A server sending a response with status code 416 (Requested range not satisfiable) // SHOULD include a Content-Range field with a byte-range-resp-spec of "*". The instance-length specifies @@ -340,11 +360,8 @@ namespace Microsoft.AspNetCore.StaticFiles return; } - // Multi-range is not supported. - Debug.Assert(_ranges.Count == 1); - long start, length; - _responseHeaders.ContentRange = ComputeContentRange(_ranges[0], out start, out length); + _responseHeaders.ContentRange = ComputeContentRange(_range, out start, out length); _response.ContentLength = length; ApplyResponseHeaders(Constants.Status206PartialContent); diff --git a/src/Microsoft.AspNetCore.StaticFiles/StaticFileMiddleware.cs b/src/Microsoft.AspNetCore.StaticFiles/StaticFileMiddleware.cs index d2cf35ac83..d8910bcdbf 100644 --- a/src/Microsoft.AspNetCore.StaticFiles/StaticFileMiddleware.cs +++ b/src/Microsoft.AspNetCore.StaticFiles/StaticFileMiddleware.cs @@ -70,7 +70,7 @@ namespace Microsoft.AspNetCore.StaticFiles public Task Invoke(HttpContext context) { var fileContext = new StaticFileContext(context, _options, _matchUrl, _logger, _fileProvider, _contentTypeProvider); - + if (!fileContext.ValidateMethod()) { _logger.LogRequestMethodNotSupported(context.Request.Method); @@ -88,10 +88,9 @@ namespace Microsoft.AspNetCore.StaticFiles _logger.LogFileNotFound(fileContext.SubPath); } else - { + { // If we get here, we can try to serve the file fileContext.ComprehendRequestHeaders(); - switch (fileContext.GetPreconditionState()) { case StaticFileContext.PreconditionState.Unspecified: @@ -104,10 +103,8 @@ namespace Microsoft.AspNetCore.StaticFiles { return fileContext.SendRangeAsync(); } - _logger.LogFileServed(fileContext.SubPath, fileContext.PhysicalPath); return fileContext.SendAsync(); - case StaticFileContext.PreconditionState.NotModified: _logger.LogPathNotModified(fileContext.SubPath); return fileContext.SendStatusAsync(Constants.Status304NotModified); diff --git a/test/Microsoft.AspNetCore.RangeHelper.Sources.Test/RangeHelperTests.cs b/test/Microsoft.AspNetCore.RangeHelper.Sources.Test/RangeHelperTests.cs index 976e0bb7be..c1a4a3d282 100644 --- a/test/Microsoft.AspNetCore.RangeHelper.Sources.Test/RangeHelperTests.cs +++ b/test/Microsoft.AspNetCore.RangeHelper.Sources.Test/RangeHelperTests.cs @@ -1,8 +1,6 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; -using System.Collections.Generic; using Microsoft.AspNetCore.Http; using Microsoft.Net.Http.Headers; using Xunit; @@ -11,134 +9,50 @@ namespace Microsoft.AspNetCore.Internal { public class RangeHelperTests { - [Fact] - public void NormalizeRanges_ReturnsEmptyArrayWhenRangeCountZero() - { - // Arrange - var ranges = new List(); - - // Act - var normalizedRanges = RangeHelper.NormalizeRanges(ranges, 2); - - // Assert - Assert.Empty(normalizedRanges); - } - - [Fact] - public void NormalizeRanges_ReturnsEmptyArrayWhenLengthZero() - { - // Arrange - var ranges = new[] - { - new RangeItemHeaderValue(0, 0), - }; - - // Act - var normalizedRanges = RangeHelper.NormalizeRanges(ranges, 0); - - // Assert - Assert.Empty(normalizedRanges); - } - [Theory] [InlineData(1, 2)] [InlineData(2, 3)] - public void NormalizeRanges_SkipsItemWhenRangeStartEqualOrGreaterThanLength(long start, long end) + public void NormalizeRange_ReturnsNullWhenRangeStartEqualsOrGreaterThanLength(long start, long end) { - // Arrange - var ranges = new[] - { - new RangeItemHeaderValue(start, end), - }; - - // Act - var normalizedRanges = RangeHelper.NormalizeRanges(ranges, 1); + // Arrange & Act + var normalizedRange = RangeHelper.NormalizeRange(new RangeItemHeaderValue(start, end), 1); // Assert - Assert.Empty(normalizedRanges); + Assert.Null(normalizedRange); } [Fact] - public void NormalizeRanges_SkipsItemWhenRangeEndEqualsZero() + public void NormalizeRange_ReturnsNullWhenRangeEndEqualsZero() { - // Arrange - var ranges = new[] - { - new RangeItemHeaderValue(null, 0), - }; - - // Act - var normalizedRanges = RangeHelper.NormalizeRanges(ranges, 1); + // Arrange & Act + var normalizedRange = RangeHelper.NormalizeRange(new RangeItemHeaderValue(null, 0), 1); // Assert - Assert.Empty(normalizedRanges); + Assert.Null(normalizedRange); } [Theory] [InlineData(0, null, 0, 2)] [InlineData(0, 0, 0, 0)] - public void NormalizeRanges_ReturnsNormalizedRange(long? start, long? end, long? normalizedStart, long? normalizedEnd) + public void NormalizeRange_ReturnsNormalizedRange(long? start, long? end, long? normalizedStart, long? normalizedEnd) { - // Arrange - var ranges = new[] - { - new RangeItemHeaderValue(start, end), - }; - - // Act - var normalizedRanges = RangeHelper.NormalizeRanges(ranges, 3); + // Arrange & Act + var normalizedRange = RangeHelper.NormalizeRange(new RangeItemHeaderValue(start, end), 3); // Assert - var range = Assert.Single(normalizedRanges); - Assert.Equal(normalizedStart, range.From); - Assert.Equal(normalizedEnd, range.To); + Assert.Equal(normalizedStart, normalizedRange.From); + Assert.Equal(normalizedEnd, normalizedRange.To); } [Fact] - public void NormalizeRanges_ReturnsRangeWithNoChange() + public void NormalizeRange_ReturnsRangeWithNoChange() { - // Arrange - var ranges = new[] - { - new RangeItemHeaderValue(1, 3), - }; - - // Act - var normalizedRanges = RangeHelper.NormalizeRanges(ranges, 4); + // Arrange & Act + var normalizedRange = RangeHelper.NormalizeRange(new RangeItemHeaderValue(1, 3), 4); // Assert - var range = Assert.Single(normalizedRanges); - Assert.Equal(1, range.From); - Assert.Equal(3, range.To); - } - - [Theory] - [InlineData(0, null, 0, 2)] - [InlineData(0, 0, 0, 0)] - public void NormalizeRanges_MultipleRanges_ReturnsNormalizedRange(long? start, long? end, long? normalizedStart, long? normalizedEnd) - { - // Arrange - var ranges = new[] - { - new RangeItemHeaderValue(start, end), - new RangeItemHeaderValue(1, 2), - }; - - // Act - var normalizedRanges = RangeHelper.NormalizeRanges(ranges, 3); - - // Assert - Assert.Collection(normalizedRanges, - range => - { - Assert.Equal(normalizedStart, range.From); - Assert.Equal(normalizedEnd, range.To); - }, - range => - { - Assert.Equal(1, range.From); - Assert.Equal(2, range.To); - }); + Assert.Equal(1, normalizedRange.From); + Assert.Equal(3, normalizedRange.To); } [Theory] @@ -151,9 +65,10 @@ namespace Microsoft.AspNetCore.Internal httpContext.Request.Headers[HeaderNames.Range] = range; // Act - var parsedRangeResult = RangeHelper.ParseRange(httpContext, httpContext.Request.GetTypedHeaders(), new DateTimeOffset(), null); + var (isRangeRequest, parsedRangeResult) = RangeHelper.ParseRange(httpContext, httpContext.Request.GetTypedHeaders(), 10); // Assert + Assert.False(isRangeRequest); Assert.Null(parsedRangeResult); } @@ -167,43 +82,10 @@ namespace Microsoft.AspNetCore.Internal httpContext.Request.Headers[HeaderNames.Range] = range; // Act - var parsedRangeResult = RangeHelper.ParseRange(httpContext, httpContext.Request.GetTypedHeaders(), new DateTimeOffset(), null); - - // Assert - Assert.Null(parsedRangeResult); - } - - [Fact] - public void ParseRange_ReturnsNullWhenLastModifiedGreaterThanIfRangeHeaderLastModified() - { - // Arrange - var httpContext = new DefaultHttpContext(); - var range = new RangeHeaderValue(1, 2); - httpContext.Request.Headers[HeaderNames.Range] = range.ToString(); - var lastModified = new RangeConditionHeaderValue(DateTime.Now); - httpContext.Request.Headers[HeaderNames.IfRange] = lastModified.ToString(); - - // Act - var parsedRangeResult = RangeHelper.ParseRange(httpContext, httpContext.Request.GetTypedHeaders(), DateTime.Now.AddMilliseconds(2), null); - - // Assert - Assert.Null(parsedRangeResult); - } - - [Fact] - public void ParseRange_ReturnsNullWhenETagNotEqualToIfRangeHeaderEntityTag() - { - // Arrange - var httpContext = new DefaultHttpContext(); - var range = new RangeHeaderValue(1, 2); - httpContext.Request.Headers[HeaderNames.Range] = range.ToString(); - var etag = new RangeConditionHeaderValue("\"tag\""); - httpContext.Request.Headers[HeaderNames.IfRange] = etag.ToString(); - - // Act - var parsedRangeResult = RangeHelper.ParseRange(httpContext, httpContext.Request.GetTypedHeaders(), DateTime.Now, new EntityTagHeaderValue("\"etag\"")); + var (isRangeRequest, parsedRangeResult) = RangeHelper.ParseRange(httpContext, httpContext.Request.GetTypedHeaders(), 10); // Assert + Assert.False(isRangeRequest); Assert.Null(parsedRangeResult); } @@ -214,35 +96,12 @@ namespace Microsoft.AspNetCore.Internal var httpContext = new DefaultHttpContext(); var range = new RangeHeaderValue(1, 2); httpContext.Request.Headers[HeaderNames.Range] = range.ToString(); - var lastModified = new RangeConditionHeaderValue(DateTime.Now); - httpContext.Request.Headers[HeaderNames.IfRange] = lastModified.ToString(); - var etag = new RangeConditionHeaderValue("\"etag\""); - httpContext.Request.Headers[HeaderNames.IfRange] = etag.ToString(); // Act - var parsedRangeResult = RangeHelper.ParseRange(httpContext, httpContext.Request.GetTypedHeaders(), DateTime.Now, new EntityTagHeaderValue("\"etag\"")); + var (isRangeRequest, parsedRange) = RangeHelper.ParseRange(httpContext, httpContext.Request.GetTypedHeaders(), 4); // Assert - var parsedRange = Assert.Single(parsedRangeResult); - Assert.Equal(1, parsedRange.From); - Assert.Equal(2, parsedRange.To); - } - - [Fact] - public void ParseRange_ReturnsRangeWhenLastModifiedAndEtagNull() - { - // Arrange - var httpContext = new DefaultHttpContext(); - var range = new RangeHeaderValue(1, 2); - httpContext.Request.Headers[HeaderNames.Range] = range.ToString(); - var lastModified = new RangeConditionHeaderValue(DateTime.Now); - httpContext.Request.Headers[HeaderNames.IfRange] = lastModified.ToString(); - - // Act - var parsedRangeResult = RangeHelper.ParseRange(httpContext, httpContext.Request.GetTypedHeaders()); - - // Assert - var parsedRange = Assert.Single(parsedRangeResult); + Assert.True(isRangeRequest); Assert.Equal(1, parsedRange.From); Assert.Equal(2, parsedRange.To); }