diff --git a/spring-web/src/main/java/org/springframework/http/HttpRange.java b/spring-web/src/main/java/org/springframework/http/HttpRange.java index bfee87f6e7..f10405ab77 100644 --- a/spring-web/src/main/java/org/springframework/http/HttpRange.java +++ b/spring-web/src/main/java/org/springframework/http/HttpRange.java @@ -27,7 +27,7 @@ import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; /** - * Represents an HTTP (byte) range, as used in the {@code Range} header. + * Represents an HTTP (byte) range for use with the HTTP {@code "Range"} header. * * @author Arjen Poutsma * @see HTTP/1.1: Range Requests @@ -41,10 +41,24 @@ public abstract class HttpRange { /** - * Creates a {@code HttpRange} that ranges from the given position to the end of the - * representation. + * Return the start of the range given the total length of a representation. + * @param length the length of the representation + * @return the start of this range for the representation + */ + public abstract long getRangeStart(long length); + + /** + * Return the end of the range (inclusive) given the total length of a representation. + * @param length the length of the representation + * @return the end of the range for the representation + */ + public abstract long getRangeEnd(long length); + + + /** + * Create an {@code HttpRange} from the given position to the end. * @param firstBytePos the first byte position - * @return a byte range that ranges from {@code firstBytePos} till the end + * @return a byte range that ranges from {@code firstPos} till the end * @see Byte Ranges */ public static HttpRange createByteRange(long firstBytePos) { @@ -52,22 +66,19 @@ public abstract class HttpRange { } /** - * Creates a {@code HttpRange} that ranges from the given fist position to the given - * last position. + * Create a {@code HttpRange} from the given fist to last position. * @param firstBytePos the first byte position * @param lastBytePos the last byte position - * @return a byte range that ranges from {@code firstBytePos} till {@code lastBytePos} + * @return a byte range that ranges from {@code firstPos} till {@code lastPos} * @see Byte Ranges */ public static HttpRange createByteRange(long firstBytePos, long lastBytePos) { - Assert.isTrue(firstBytePos <= lastBytePos, - "\"firstBytePost\" should be " + "less then or equal to \"lastBytePos\""); return new ByteRange(firstBytePos, lastBytePos); } /** - * Creates a {@code HttpRange} that ranges over the last given number of bytes. - * @param suffixLength the number of bytes + * Create an {@code HttpRange} that ranges over the last given number of bytes. + * @param suffixLength the number of bytes for the range * @return a byte range that ranges over the last {@code suffixLength} number of bytes * @see Byte Ranges */ @@ -75,23 +86,6 @@ public abstract class HttpRange { return new SuffixByteRange(suffixLength); } - - /** - * Return the start of this range, given the total length of the representation. - * @param length the length of the representation. - * @return the start of this range - */ - public abstract long getRangeStart(long length); - - /** - * Return the end of this range (inclusive), given the total length of the - * representation. - * @param length the length of the representation. - * @return the end of this range - */ - public abstract long getRangeEnd(long length); - - /** * Parse the given, comma-separated string into a list of {@code HttpRange} objects. *

This method can be used to parse an {@code Range} header. @@ -104,8 +98,7 @@ public abstract class HttpRange { return Collections.emptyList(); } if (!ranges.startsWith(BYTE_RANGE_PREFIX)) { - throw new IllegalArgumentException("Range \"" + ranges + "\" does not " + - "start with \"" + BYTE_RANGE_PREFIX + "\""); + throw new IllegalArgumentException("Range '" + ranges + "' does not start with 'bytes='"); } ranges = ranges.substring(BYTE_RANGE_PREFIX.length()); @@ -118,36 +111,25 @@ public abstract class HttpRange { } private static HttpRange parseRange(String range) { - if (range == null) { - return null; - } + Assert.notNull(range); int dashIdx = range.indexOf('-'); - if (dashIdx < 0) { - throw new IllegalArgumentException("Range '\"" + range + "\" does not" + - "contain \"-\""); - } - else if (dashIdx > 0) { - // standard byte range, i.e. "bytes=0-500" + if (dashIdx > 0) { long firstPos = Long.parseLong(range.substring(0, dashIdx)); - ByteRange byteRange; if (dashIdx < range.length() - 1) { - long lastPos = - Long.parseLong(range.substring(dashIdx + 1, range.length())); - byteRange = new ByteRange(firstPos, lastPos); + Long lastPos = Long.parseLong(range.substring(dashIdx + 1, range.length())); + return new ByteRange(firstPos, lastPos); } else { - byteRange = new ByteRange(firstPos, null); + return new ByteRange(firstPos, null); } - if (!byteRange.validate()) { - throw new IllegalArgumentException("Invalid Range \"" + range + "\""); - } - return byteRange; } - else { // dashIdx == 0 - // suffix byte range, i.e. "bytes=-500" + else if (dashIdx == 0) { long suffixLength = Long.parseLong(range.substring(1)); return new SuffixByteRange(suffixLength); } + else { + throw new IllegalArgumentException("Range '" + range + "' does not contain \"-\""); + } } /** @@ -157,6 +139,7 @@ public abstract class HttpRange { * @return the string representation */ public static String toString(Collection ranges) { + Assert.notNull(ranges); StringBuilder builder = new StringBuilder(BYTE_RANGE_PREFIX); for (Iterator iterator = ranges.iterator(); iterator.hasNext(); ) { HttpRange range = iterator.next(); @@ -177,6 +160,7 @@ public abstract class HttpRange { abstract void appendTo(StringBuilder builder); + /** * Represents an HTTP/1.1 byte range, with a first and optional last position. * @see Byte Ranges @@ -189,11 +173,23 @@ public abstract class HttpRange { private final Long lastPos; + private ByteRange(long firstPos, Long lastPos) { + assertPositions(firstPos, lastPos); this.firstPos = firstPos; this.lastPos = lastPos; } + private void assertPositions(long firstBytePos, Long lastBytePos) { + if (firstBytePos < 0) { + throw new IllegalArgumentException("Invalid firstPos=" + firstBytePos); + } + if (lastBytePos != null && lastBytePos < firstBytePos) { + throw new IllegalArgumentException("firstPost= " + firstBytePos + + " should be less then or equal to lastBytePosition=" + lastBytePos); + } + } + @Override public long getRangeStart(long length) { return this.firstPos; @@ -206,7 +202,6 @@ public abstract class HttpRange { } else { return length - 1; - } } @@ -219,18 +214,6 @@ public abstract class HttpRange { } } - boolean validate() { - if (this.firstPos < 0) { - return false; - } - if (this.lastPos == null) { - return true; - } - else { - return this.firstPos <= this.lastPos; - } - } - @Override public boolean equals(Object o) { if (this == o) { @@ -239,11 +222,8 @@ public abstract class HttpRange { if (!(o instanceof ByteRange)) { return false; } - ByteRange other = (ByteRange) o; - - return this.firstPos == other.firstPos && - ObjectUtils.nullSafeEquals(this.lastPos, other.lastPos); + return this.firstPos == other.firstPos && ObjectUtils.nullSafeEquals(this.lastPos, other.lastPos); } @Override @@ -252,7 +232,6 @@ public abstract class HttpRange { hashCode = 31 * hashCode + ObjectUtils.nullSafeHashCode(this.lastPos); return hashCode; } - } /** @@ -264,15 +243,14 @@ public abstract class HttpRange { private final long suffixLength; + private SuffixByteRange(long suffixLength) { + if (suffixLength < 0) { + throw new IllegalArgumentException("Invalid suffixLength=" + suffixLength); + } this.suffixLength = suffixLength; } - @Override - void appendTo(StringBuilder builder) { - builder.append('-'); - builder.append(this.suffixLength); - } @Override public long getRangeStart(long length) { @@ -289,6 +267,12 @@ public abstract class HttpRange { return length - 1; } + @Override + void appendTo(StringBuilder builder) { + builder.append('-'); + builder.append(this.suffixLength); + } + @Override public boolean equals(Object o) { if (this == o) { diff --git a/spring-web/src/test/java/org/springframework/http/HttpHeadersTests.java b/spring-web/src/test/java/org/springframework/http/HttpHeadersTests.java index 7eb8fb52aa..c7328fce20 100644 --- a/spring-web/src/test/java/org/springframework/http/HttpHeadersTests.java +++ b/spring-web/src/test/java/org/springframework/http/HttpHeadersTests.java @@ -36,6 +36,7 @@ import org.junit.Before; import org.junit.Test; /** + * Unit tests for {@link org.springframework.http.HttpHeaders}. * @author Arjen Poutsma */ public class HttpHeadersTests { @@ -266,16 +267,4 @@ public class HttpHeadersTests { assertThat(headers.getAllow(), Matchers.emptyCollectionOf(HttpMethod.class)); } - @Test - public void range() { - List ranges = new ArrayList<>(); - ranges.add(HttpRange.createByteRange(0, 499)); - ranges.add(HttpRange.createByteRange(9500)); - ranges.add(HttpRange.createSuffixRange(500)); - - headers.setRange(ranges); - assertEquals("Invalid Range header", ranges, headers.getRange()); - assertEquals("Invalid Range header", "bytes=0-499, 9500-, -500", headers.getFirst("Range")); - } - } diff --git a/spring-web/src/test/java/org/springframework/http/HttpRangeTests.java b/spring-web/src/test/java/org/springframework/http/HttpRangeTests.java new file mode 100644 index 0000000000..fa71022f2b --- /dev/null +++ b/spring-web/src/test/java/org/springframework/http/HttpRangeTests.java @@ -0,0 +1,103 @@ +/* + * Copyright 2002-2015 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.http; + +import static org.junit.Assert.*; + +import java.util.ArrayList; +import java.util.List; + +import org.junit.Test; + +/** + * Unit tests for {@link org.springframework.http.HttpRange}. + * @author Rossen Stoyanchev + */ +public class HttpRangeTests { + + + @Test(expected = IllegalArgumentException.class) + public void invalidFirstPosition() throws Exception { + HttpRange.createByteRange(-1); + } + + @Test(expected = IllegalArgumentException.class) + public void invalidLastLessThanFirst() throws Exception { + HttpRange.createByteRange(10, 9); + } + + @Test(expected = IllegalArgumentException.class) + public void invalidSuffixLength() throws Exception { + HttpRange.createSuffixRange(-1); + } + + @Test + public void byteRange() throws Exception { + HttpRange range = HttpRange.createByteRange(0, 499); + assertEquals(0, range.getRangeStart(1000)); + assertEquals(499, range.getRangeEnd(1000)); + } + + @Test + public void byteRangeWithoutLastPosition() throws Exception { + HttpRange range = HttpRange.createByteRange(9500); + assertEquals(9500, range.getRangeStart(10000)); + assertEquals(9999, range.getRangeEnd(10000)); + } + + @Test + public void byteRangeOfZeroLength() throws Exception { + HttpRange range = HttpRange.createByteRange(9500, 9500); + assertEquals(9500, range.getRangeStart(10000)); + assertEquals(9500, range.getRangeEnd(10000)); + } + + @Test + public void suffixRange() throws Exception { + HttpRange range = HttpRange.createSuffixRange(500); + assertEquals(500, range.getRangeStart(1000)); + assertEquals(999, range.getRangeEnd(1000)); + } + + @Test + public void suffixRangeShorterThanRepresentation() throws Exception { + HttpRange range = HttpRange.createSuffixRange(500); + assertEquals(0, range.getRangeStart(350)); + assertEquals(349, range.getRangeEnd(350)); + } + + @Test + public void parseRanges() throws Exception { + List ranges = HttpRange.parseRanges("bytes=0-0,500-,-1"); + assertEquals(3, ranges.size()); + assertEquals(0, ranges.get(0).getRangeStart(1000)); + assertEquals(0, ranges.get(0).getRangeEnd(1000)); + assertEquals(500, ranges.get(1).getRangeStart(1000)); + assertEquals(999, ranges.get(1).getRangeEnd(1000)); + assertEquals(999, ranges.get(2).getRangeStart(1000)); + assertEquals(999, ranges.get(2).getRangeEnd(1000)); + } + + @Test + public void rangeToString() { + List ranges = new ArrayList<>(); + ranges.add(HttpRange.createByteRange(0, 499)); + ranges.add(HttpRange.createByteRange(9500)); + ranges.add(HttpRange.createSuffixRange(500)); + assertEquals("Invalid Range header", "bytes=0-499, 9500-, -500", HttpRange.toString(ranges)); + } + +} diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java index 8520cf9160..b2e721f04d 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java @@ -246,7 +246,7 @@ public class ResourceHttpRequestHandler extends WebContentGenerator implements H return; } - if (request.getHeader("Range") == null) { + if (request.getHeader(HttpHeaders.RANGE) == null) { setHeaders(response, resource, mediaType); writeContent(response, resource); } @@ -408,6 +408,8 @@ public class ResourceHttpRequestHandler extends WebContentGenerator implements H if (resource instanceof EncodedResource) { response.setHeader(CONTENT_ENCODING, ((EncodedResource) resource).getContentEncoding()); } + + response.setHeader(HttpHeaders.ACCEPT_RANGES, "bytes"); } /** @@ -433,26 +435,25 @@ public class ResourceHttpRequestHandler extends WebContentGenerator implements H } /** - * Write partial content out to the given servlet response, - * streaming parts of the resource's content, as indicated by the request's - * {@code Range} header. + * Write parts of the resource as indicated by the request {@code Range} header. * @param request current servlet request * @param response current servlet response * @param resource the identified resource (never {@code null}) * @param contentType the content type * @throws IOException in case of errors while writing the content */ - protected void writePartialContent(HttpServletRequest request, - HttpServletResponse response, Resource resource, MediaType contentType) throws IOException { - long resourceLength = resource.contentLength(); + protected void writePartialContent(HttpServletRequest request, HttpServletResponse response, + Resource resource, MediaType contentType) throws IOException { + + long length = resource.contentLength(); List ranges; try { - HttpHeaders requestHeaders = - new ServletServerHttpRequest(request).getHeaders(); - ranges = requestHeaders.getRange(); - } catch (IllegalArgumentException ex) { - response.addHeader("Content-Range", "bytes */" + resourceLength); + HttpHeaders headers = new ServletServerHttpRequest(request).getHeaders(); + ranges = headers.getRange(); + } + catch (IllegalArgumentException ex) { + response.addHeader("Content-Range", "bytes */" + length); response.sendError(HttpServletResponse.SC_REQUESTED_RANGE_NOT_SATISFIABLE); return; } @@ -462,20 +463,17 @@ public class ResourceHttpRequestHandler extends WebContentGenerator implements H if (ranges.size() == 1) { HttpRange range = ranges.get(0); - long rangeStart = range.getRangeStart(resourceLength); - long rangeEnd = range.getRangeEnd(resourceLength); - long rangeLength = rangeEnd - rangeStart + 1; + long start = range.getRangeStart(length); + long end = range.getRangeEnd(length); + long rangeLength = end - start + 1; setHeaders(response, resource, contentType); - response.addHeader("Content-Range", "bytes " - + rangeStart + "-" - + rangeEnd + "/" - + resourceLength); + response.addHeader("Content-Range", "bytes " + start + "-" + end + "/" + length); response.setContentLength((int) rangeLength); InputStream in = resource.getInputStream(); try { - copyRange(in, response.getOutputStream(), rangeStart, rangeEnd); + copyRange(in, response.getOutputStream(), start, end); } finally { try { @@ -493,8 +491,8 @@ public class ResourceHttpRequestHandler extends WebContentGenerator implements H ServletOutputStream out = response.getOutputStream(); for (HttpRange range : ranges) { - long rangeStart = range.getRangeStart(resourceLength); - long rangeEnd = range.getRangeEnd(resourceLength); + long start = range.getRangeStart(length); + long end = range.getRangeEnd(length); InputStream in = resource.getInputStream(); @@ -504,27 +502,23 @@ public class ResourceHttpRequestHandler extends WebContentGenerator implements H if (contentType != null) { out.println("Content-Type: " + contentType); } - out.println("Content-Range: bytes " + rangeStart + "-" + - rangeEnd + "/" + resourceLength); + out.println("Content-Range: bytes " + start + "-" + end + "/" + length); out.println(); // Printing content - copyRange(in, out, rangeStart, rangeEnd); - + copyRange(in, out, start, end); } out.println(); out.print("--" + boundaryString + "--"); } } - private void copyRange(InputStream in, OutputStream out, long start, long end) - throws IOException { + private void copyRange(InputStream in, OutputStream out, long start, long end) throws IOException { long skipped = in.skip(start); if (skipped < start) { - throw new IOException("Could only skip " + skipped + " bytes out of " + - start + " required"); + throw new IOException("Skipped only " + skipped + " bytes out of " + start + " required."); } long bytesToCopy = end - start + 1; @@ -546,6 +540,7 @@ public class ResourceHttpRequestHandler extends WebContentGenerator implements H } } + @Override public String toString() { return "ResourceHttpRequestHandler [locations=" + diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java index 470b3e31c7..b7129b7501 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java @@ -286,8 +286,7 @@ public class ResourceHttpRequestHandlerTests { assertEquals(206, this.response.getStatus()); assertEquals("text/plain", this.response.getContentType()); assertEquals(2, this.response.getContentLength()); - assertEquals("bytes 0-1/10", - this.response.getHeader("Content-Range")); + assertEquals("bytes 0-1/10", this.response.getHeader("Content-Range")); assertEquals("So", this.response.getContentAsString()); } @@ -300,8 +299,7 @@ public class ResourceHttpRequestHandlerTests { assertEquals(206, this.response.getStatus()); assertEquals("text/plain", this.response.getContentType()); assertEquals(1, this.response.getContentLength()); - assertEquals("bytes 9-9/10", - this.response.getHeader("Content-Range")); + assertEquals("bytes 9-9/10", this.response.getHeader("Content-Range")); assertEquals(".", this.response.getContentAsString()); } @@ -314,8 +312,7 @@ public class ResourceHttpRequestHandlerTests { assertEquals(206, this.response.getStatus()); assertEquals("text/plain", this.response.getContentType()); assertEquals(1, this.response.getContentLength()); - assertEquals("bytes 9-9/10", - this.response.getHeader("Content-Range")); + assertEquals("bytes 9-9/10", this.response.getHeader("Content-Range")); assertEquals(".", this.response.getContentAsString()); } @@ -328,8 +325,7 @@ public class ResourceHttpRequestHandlerTests { assertEquals(206, this.response.getStatus()); assertEquals("text/plain", this.response.getContentType()); assertEquals(1, this.response.getContentLength()); - assertEquals("bytes 9-9/10", - this.response.getHeader("Content-Range")); + assertEquals("bytes 9-9/10", this.response.getHeader("Content-Range")); assertEquals(".", this.response.getContentAsString()); } @@ -342,21 +338,18 @@ public class ResourceHttpRequestHandlerTests { assertEquals(206, this.response.getStatus()); assertEquals("text/plain", this.response.getContentType()); assertEquals(10, this.response.getContentLength()); - assertEquals("bytes 0-9/10", - this.response.getHeader("Content-Range")); + assertEquals("bytes 0-9/10", this.response.getHeader("Content-Range")); assertEquals("Some text.", this.response.getContentAsString()); } @Test public void partialContentInvalidRangeHeader() throws Exception { this.request.addHeader("Range", "bytes= foo bar"); - this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, - "foo.txt"); + this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "foo.txt"); this.handler.handleRequest(this.request, this.response); assertEquals(416, this.response.getStatus()); - assertEquals("bytes */10", - this.response.getHeader("Content-Range")); + assertEquals("bytes */10", this.response.getHeader("Content-Range")); } @Test @@ -366,13 +359,12 @@ public class ResourceHttpRequestHandlerTests { this.handler.handleRequest(this.request, this.response); assertEquals(206, this.response.getStatus()); - assertTrue(this.response.getContentType() - .startsWith("multipart/byteranges; boundary=")); + assertTrue(this.response.getContentType().startsWith("multipart/byteranges; boundary=")); String boundary = "--" + this.response.getContentType().substring(31); - String[] ranges = StringUtils.tokenizeToStringArray(this.response.getContentAsString(), - "\r\n", false, true); + String content = this.response.getContentAsString(); + String[] ranges = StringUtils.tokenizeToStringArray(content, "\r\n", false, true); assertEquals(boundary, ranges[0]); assertEquals("Content-Type: text/plain", ranges[1]); @@ -391,8 +383,6 @@ public class ResourceHttpRequestHandlerTests { } - - private long headerAsLong(String responseHeaderName) { return Long.valueOf(this.response.getHeader(responseHeaderName)); }