From da557e741519f6d110bccd6f10e1f808c40d661b Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 5 Apr 2019 12:47:02 +0200 Subject: [PATCH] Avoid expensive assertions in HttpRange Closes gh-22742 --- .../org/springframework/http/HttpRange.java | 42 ++++++++++--------- .../http/codec/ResourceHttpMessageWriter.java | 11 ++--- 2 files changed, 25 insertions(+), 28 deletions(-) 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 c1025ac1df6..f1a7d33f513 100644 --- a/spring-web/src/main/java/org/springframework/http/HttpRange.java +++ b/spring-web/src/main/java/org/springframework/http/HttpRange.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -68,18 +68,6 @@ public abstract class HttpRange { return new ResourceRegion(resource, start, end - start + 1); } - private static long getLengthFor(Resource resource) { - long contentLength; - try { - contentLength = resource.contentLength(); - Assert.isTrue(contentLength > 0, "Resource content length should be > 0"); - } - catch (IOException ex) { - throw new IllegalArgumentException("Failed to obtain Resource content length", ex); - } - return contentLength; - } - /** * Return the start of the range given the total length of a representation. * @param length the length of the representation @@ -131,8 +119,8 @@ public abstract class HttpRange { *

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, or if - * the number of ranges is greater than 100. + * @throws IllegalArgumentException if the string cannot be parsed + * or if the number of ranges is greater than 100 */ public static List parseRanges(@Nullable String ranges) { if (!StringUtils.hasLength(ranges)) { @@ -144,7 +132,9 @@ public abstract class HttpRange { ranges = ranges.substring(BYTE_RANGE_PREFIX.length()); String[] tokens = StringUtils.tokenizeToStringArray(ranges, ","); - Assert.isTrue(tokens.length <= MAX_RANGES, () -> "Too many ranges " + tokens.length); + if (tokens.length > MAX_RANGES) { + throw new IllegalArgumentException("Too many ranges: " + tokens.length); + } List result = new ArrayList<>(tokens.length); for (String token : tokens) { result.add(parseRange(token)); @@ -158,7 +148,7 @@ public abstract class HttpRange { if (dashIdx > 0) { long firstPos = Long.parseLong(range.substring(0, dashIdx)); if (dashIdx < range.length() - 1) { - Long lastPos = Long.parseLong(range.substring(dashIdx + 1, range.length())); + Long lastPos = Long.parseLong(range.substring(dashIdx + 1)); return new ByteRange(firstPos, lastPos); } else { @@ -195,13 +185,25 @@ public abstract class HttpRange { 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 + ")"); + if (total >= length) { + throw new IllegalArgumentException("The sum of all ranges (" + total + + ") should be less than the resource length (" + length + ")"); + } } return regions; } + private static long getLengthFor(Resource resource) { + try { + long contentLength = resource.contentLength(); + Assert.isTrue(contentLength > 0, "Resource content length should be > 0"); + return contentLength; + } + catch (IOException ex) { + throw new IllegalArgumentException("Failed to obtain Resource content length", ex); + } + } + /** * Return a string representation of the given list of {@code HttpRange} objects. *

This method can be used to for an {@code Range} header. diff --git a/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java b/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java index 1d0bd920bed..4376d39d06c 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java +++ b/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -53,9 +53,8 @@ import org.springframework.util.MimeTypeUtils; /** * {@code HttpMessageWriter} that can write a {@link Resource}. * - *

Also an implementation of {@code HttpMessageWriter} with support - * for writing one or more {@link ResourceRegion}'s based on the HTTP ranges - * specified in the request. + *

Also an implementation of {@code HttpMessageWriter} with support for writing one + * or more {@link ResourceRegion}'s based on the HTTP ranges specified in the request. * *

For reading to a Resource, use {@link ResourceDecoder} wrapped with * {@link DecoderHttpMessageReader}. @@ -187,7 +186,6 @@ public class ResourceHttpMessageWriter implements HttpMessageWriter { // Server-side only: single Resource or sub-regions... @Override - @SuppressWarnings("unchecked") public Mono write(Publisher inputStream, @Nullable ResolvableType actualType, ResolvableType elementType, @Nullable MediaType mediaType, ServerHttpRequest request, ServerHttpResponse response, Map hints) { @@ -205,15 +203,12 @@ public class ResourceHttpMessageWriter implements HttpMessageWriter { } return Mono.from(inputStream).flatMap(resource -> { - if (ranges.isEmpty()) { return writeResource(resource, elementType, mediaType, response, hints); } - response.setStatusCode(HttpStatus.PARTIAL_CONTENT); List regions = HttpRange.toResourceRegions(ranges, resource); MediaType resourceMediaType = getResourceMediaType(mediaType, resource, hints); - if (regions.size() == 1){ ResourceRegion region = regions.get(0); headers.setContentType(resourceMediaType);