HttpRange validates requested ranges

Issue: SPR-17318

Author: rstoyanchev

spring-web/src/main/java/org/springframework/http/HttpRange.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -44,6 +44,9 @@
  */
 public abstract class HttpRange {
 
+	/** Maximum ranges per request. */
+	private static final int MAX_RANGES = 100;
+
 	private static final String BYTE_RANGE_PREFIX = "bytes=";
 
 
@@ -59,16 +62,22 @@ public ResourceRegion toResourceRegion(Resource resource) {
 		// Note: custom InputStreamResource subclasses could provide a pre-calculated content length!
 		Assert.isTrue(resource.getClass() != InputStreamResource.class,
 				"Cannot convert an InputStreamResource to a ResourceRegion");
+		long contentLength = getLengthFor(resource);
+		long start = getRangeStart(contentLength);
+		long end = getRangeEnd(contentLength);
+		return new ResourceRegion(resource, start, end - start + 1);
+	}
+
+	private static long getLengthFor(Resource resource) {
+		long contentLength;
 		try {
-			long contentLength = resource.contentLength();
+			contentLength = resource.contentLength();
 			Assert.isTrue(contentLength > 0, "Resource content length should be > 0");
-			long start = getRangeStart(contentLength);
-			long end = getRangeEnd(contentLength);
-			return new ResourceRegion(resource, start, end - start + 1);
 		}
 		catch (IOException ex) {
-			throw new IllegalArgumentException("Failed to convert Resource to ResourceRegion", ex);
+			throw new IllegalArgumentException("Failed to obtain Resource content length", ex);
 		}
+		return contentLength;
 	}
 
 	/**
@@ -122,7 +131,8 @@ public static HttpRange createSuffixRange(long suffixLength) {
 	 * <p>This method can be used to parse an {@code Range} header.
 	 * @param ranges the string to parse
 	 * @return the list of ranges
-	 * @throws IllegalArgumentException if the string cannot be parsed
+	 * @throws IllegalArgumentException if the string cannot be parsed, or if
+	 * the number of ranges is greater than 100.
 	 */
 	public static List<HttpRange> parseRanges(@Nullable String ranges) {
 		if (!StringUtils.hasLength(ranges)) {
@@ -134,6 +144,7 @@ public static List<HttpRange> parseRanges(@Nullable String ranges) {
 		ranges = ranges.substring(BYTE_RANGE_PREFIX.length());
 
 		String[] tokens = StringUtils.tokenizeToStringArray(ranges, ",");
+		Assert.isTrue(tokens.length <= MAX_RANGES, () -> "Too many ranges " + tokens.length);
 		List<HttpRange> result = new ArrayList<>(tokens.length);
 		for (String token : tokens) {
 			result.add(parseRange(token));
@@ -169,6 +180,8 @@ else if (dashIdx == 0) {
 	 * @param ranges the list of ranges
 	 * @param resource the resource to select the regions from
 	 * @return the list of regions for the given resource
+	 * @throws IllegalArgumentException if the sum of all ranges exceeds the
+	 * resource length.
 	 * @since 4.3
 	 */
 	public static List<ResourceRegion> toResourceRegions(List<HttpRange> ranges, Resource resource) {
@@ -179,6 +192,13 @@ public static List<ResourceRegion> toResourceRegions(List<HttpRange> ranges, Res
 		for (HttpRange range : ranges) {
 			regions.add(range.toResourceRegion(resource));
 		}
+		if (ranges.size() > 1) {
+			long length = getLengthFor(resource);
+			long total = regions.stream().map(ResourceRegion::getCount).reduce(0L, (count, sum) -> sum + count);
+			Assert.isTrue(total < length,
+					() -> "The sum of all ranges (" + total + ") " +
+							"should be less than the resource length (" + length + ")");
+		}
 		return regions;
 	}
 

spring-web/src/test/java/org/springframework/http/HttpRangeTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -19,7 +19,9 @@
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
+import java.util.stream.Stream;
 
 import org.junit.Test;
 
@@ -100,6 +102,31 @@ public void parseRanges() {
 		assertEquals(999, ranges.get(2).getRangeEnd(1000));
 	}
 
+	@Test
+	public void parseRangesValidations() {
+
+		// 1. At limit..
+		StringBuilder sb = new StringBuilder("bytes=0-0");
+		for (int i=0; i < 99; i++) {
+			sb.append(",").append(i).append("-").append(i + 1);
+		}
+		List<HttpRange> ranges = HttpRange.parseRanges(sb.toString());
+		assertEquals(100, ranges.size());
+
+		// 2. Above limit..
+		sb = new StringBuilder("bytes=0-0");
+		for (int i=0; i < 100; i++) {
+			sb.append(",").append(i).append("-").append(i + 1);
+		}
+		try {
+			HttpRange.parseRanges(sb.toString());
+			fail();
+		}
+		catch (IllegalArgumentException ex) {
+			// Expected
+		}
+	}
+
 	@Test
 	public void rangeToString() {
 		List<HttpRange> ranges = new ArrayList<>();
@@ -144,4 +171,25 @@ public void toResourceRegionExceptionLength() throws IOException {
 		range.toResourceRegion(resource);
 	}
 
+	@Test
+	public void toResourceRegionsValidations() {
+		byte[] bytes = "12345".getBytes(StandardCharsets.UTF_8);
+		ByteArrayResource resource = new ByteArrayResource(bytes);
+
+		// 1. Below length
+		List<HttpRange> ranges = HttpRange.parseRanges("bytes=0-1,2-3");
+		List<ResourceRegion> regions = HttpRange.toResourceRegions(ranges, resource);
+		assertEquals(2, regions.size());
+
+		// 2. At length
+		ranges = HttpRange.parseRanges("bytes=0-1,2-4");
+		try {
+			HttpRange.toResourceRegions(ranges, resource);
+			fail();
+		}
+		catch (IllegalArgumentException ex) {
+			// Expected..
+		}
+	}
+
 }