Refactor RangeHelper (#200)

Addresses #196

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
     {
         /// <summary>
-        /// Returns the requested range if the Range Header in the <see cref="HttpContext.Request"/> is valid.
+        /// Returns the normalized form of the requested range if the Range Header in the <see cref="HttpContext.Request"/> is valid.
         /// </summary>
         /// <param name="context">The <see cref="HttpContext"/> associated with the request.</param>
         /// <param name="requestHeaders">The <see cref="RequestHeaders"/> associated with the given <paramref name="context"/>.</param>
-        /// <param name="lastModified">The <see cref="DateTimeOffset"/> representation of the last modified date of the file.</param>
+        /// <param name="length">The total length of the file representation requested.</param>
-        /// <param name="etag">The <see cref="EntityTagHeaderValue"/> provided in the <see cref="HttpContext.Request"/>.</param>
+        /// <returns>A boolean value which represents if the <paramref name="requestHeaders"/> contain a single valid 
-        /// <returns>A collection of <see cref="RangeItemHeaderValue"/> containing the ranges parsed from the <paramref name="requestHeaders"/>.</returns>
+        /// range request. A <see cref="RangeItemHeaderValue"/> which represents the normalized form of the 
-        public static ICollection<RangeItemHeaderValue> ParseRange(HttpContext context, RequestHeaders requestHeaders, DateTimeOffset? lastModified = null, EntityTagHeaderValue etag = null)
+        /// range parsed from the <paramref name="requestHeaders"/> or <c>null</c> if it cannot be normalized.</returns>
+        /// <remark>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 (<c>true</c>,<c>null</c>) return values.</remark>
+        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 ranges = rangeHeader.Ranges;
-            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;
-        }
-
-        /// <summary>
-        /// A helper method to normalize a collection of <see cref="RangeItemHeaderValue"/>s.
-        /// </summary>
-        /// <param name="ranges">A collection of <see cref="RangeItemHeaderValue"/> to normalize.</param>
-        /// <param name="length">The total length of the file representation requested.</param>
-        /// <returns>A normalized list of <see cref="RangeItemHeaderValue"/>s.</returns>
-        // 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<RangeItemHeaderValue> NormalizeRanges(ICollection<RangeItemHeaderValue> ranges, long length)
-        {
             if (ranges == null)
             {
-                return null;
+                return (false, null);
             }
 
             if (ranges.Count == 0)
             {
-                return Array.Empty<RangeItemHeaderValue>();
+                return (true, null);
             }
 
             if (length == 0)
             {
-                return Array.Empty<RangeItemHeaderValue>();
+                return (true, null);
             }
 
-            var normalizedRanges = new List<RangeItemHeaderValue>(ranges.Count);
+            // Normalize the ranges
-            foreach (var range in ranges)
+            var range = NormalizeRange(ranges.SingleOrDefault(), length);
-            {
-                var normalizedRange = NormalizeRange(range, length);
 
-                if (normalizedRange != null)
+            // Return the single range
-                {
+            return (true, range);
-                    normalizedRanges.Add(normalizedRange);
-                }
-            }
-
-            return normalizedRanges;
         }
 
-        /// <summary>
+        // Internal for testing
-        /// A helper method to normalize a <see cref="RangeItemHeaderValue"/>.
+        internal static RangeItemHeaderValue NormalizeRange(RangeItemHeaderValue range, long length)
-        /// </summary>
-        /// <param name="range">The <see cref="RangeItemHeaderValue"/> to normalize.</param>
-        /// <param name="length">The total length of the file representation requested.</param>
-        /// <returns>A normalized <see cref="RangeItemHeaderValue"/>.</returns>
-        public 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 new RangeItemHeaderValue(start, end);
-            return normalizedRange;
         }
     }
 }

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<RangeItemHeaderValue> _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);            
+            (_isRangeRequest, _range) = RangeHelper.ParseRange(_context, _requestHeaders, _length);
-            _ranges = RangeHelper.NormalizeRanges(parsedRange, _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 (_range == null)
-            if (_ranges.Count == 0)
-            {
-                rangeNotSatisfiable = true;
-            }
-
-            if (rangeNotSatisfiable)
             {
                 // 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);
 

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);

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<RangeItemHeaderValue>();
-
-            // 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
+            // Arrange & Act
-            var ranges = new[]
+            var normalizedRange = RangeHelper.NormalizeRange(new RangeItemHeaderValue(start, end), 1);
-            {
-                new RangeItemHeaderValue(start, end),
-            };
-
-            // Act
-            var normalizedRanges = RangeHelper.NormalizeRanges(ranges, 1);
 
             // Assert
-            Assert.Empty(normalizedRanges);
+            Assert.Null(normalizedRange);
         }
 
         [Fact]
-        public void NormalizeRanges_SkipsItemWhenRangeEndEqualsZero()
+        public void NormalizeRange_ReturnsNullWhenRangeEndEqualsZero()
         {
-            // Arrange
+            // Arrange & Act
-            var ranges = new[]
+            var normalizedRange = RangeHelper.NormalizeRange(new RangeItemHeaderValue(null, 0), 1);
-            {
-                new RangeItemHeaderValue(null, 0),
-            };
-
-            // Act
-            var normalizedRanges = RangeHelper.NormalizeRanges(ranges, 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
+            // Arrange & Act
-            var ranges = new[]
+            var normalizedRange = RangeHelper.NormalizeRange(new RangeItemHeaderValue(start, end), 3);
-            {
-                new RangeItemHeaderValue(start, end),
-            };
-
-            // Act
-            var normalizedRanges = RangeHelper.NormalizeRanges(ranges, 3);
 
             // Assert
-            var range = Assert.Single(normalizedRanges);
+            Assert.Equal(normalizedStart, normalizedRange.From);
-            Assert.Equal(normalizedStart, range.From);
+            Assert.Equal(normalizedEnd, normalizedRange.To);
-            Assert.Equal(normalizedEnd, range.To);
         }
 
         [Fact]
-        public void NormalizeRanges_ReturnsRangeWithNoChange()
+        public void NormalizeRange_ReturnsRangeWithNoChange()
         {
-            // Arrange
+            // Arrange & Act
-            var ranges = new[]
+            var normalizedRange = RangeHelper.NormalizeRange(new RangeItemHeaderValue(1, 3), 4);
-            {
-                new RangeItemHeaderValue(1, 3),
-            };
-
-            // Act
-            var normalizedRanges = RangeHelper.NormalizeRanges(ranges, 4);
 
             // Assert
-            var range = Assert.Single(normalizedRanges);
+            Assert.Equal(1, normalizedRange.From);
-            Assert.Equal(1, range.From);
+            Assert.Equal(3, normalizedRange.To);
-            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);
-                });
         }
 
         [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);
+            var (isRangeRequest, parsedRangeResult) = RangeHelper.ParseRange(httpContext, httpContext.Request.GetTypedHeaders(), 10);
-
-            // 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\""));
 
             // 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.True(isRangeRequest);
-            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.Equal(1, parsedRange.From);
             Assert.Equal(2, parsedRange.To);
         }