Skip to content

Commit da557e7

Browse files
committed
Avoid expensive assertions in HttpRange
Closes gh-22742
1 parent 95232d5 commit da557e7

File tree

2 files changed

+25
-28
lines changed

2 files changed

+25
-28
lines changed

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

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -68,18 +68,6 @@ public ResourceRegion toResourceRegion(Resource resource) {
6868
return new ResourceRegion(resource, start, end - start + 1);
6969
}
7070

71-
private static long getLengthFor(Resource resource) {
72-
long contentLength;
73-
try {
74-
contentLength = resource.contentLength();
75-
Assert.isTrue(contentLength > 0, "Resource content length should be > 0");
76-
}
77-
catch (IOException ex) {
78-
throw new IllegalArgumentException("Failed to obtain Resource content length", ex);
79-
}
80-
return contentLength;
81-
}
82-
8371
/**
8472
* Return the start of the range given the total length of a representation.
8573
* @param length the length of the representation
@@ -131,8 +119,8 @@ public static HttpRange createSuffixRange(long suffixLength) {
131119
* <p>This method can be used to parse an {@code Range} header.
132120
* @param ranges the string to parse
133121
* @return the list of ranges
134-
* @throws IllegalArgumentException if the string cannot be parsed, or if
135-
* the number of ranges is greater than 100.
122+
* @throws IllegalArgumentException if the string cannot be parsed
123+
* or if the number of ranges is greater than 100
136124
*/
137125
public static List<HttpRange> parseRanges(@Nullable String ranges) {
138126
if (!StringUtils.hasLength(ranges)) {
@@ -144,7 +132,9 @@ public static List<HttpRange> parseRanges(@Nullable String ranges) {
144132
ranges = ranges.substring(BYTE_RANGE_PREFIX.length());
145133

146134
String[] tokens = StringUtils.tokenizeToStringArray(ranges, ",");
147-
Assert.isTrue(tokens.length <= MAX_RANGES, () -> "Too many ranges " + tokens.length);
135+
if (tokens.length > MAX_RANGES) {
136+
throw new IllegalArgumentException("Too many ranges: " + tokens.length);
137+
}
148138
List<HttpRange> result = new ArrayList<>(tokens.length);
149139
for (String token : tokens) {
150140
result.add(parseRange(token));
@@ -158,7 +148,7 @@ private static HttpRange parseRange(String range) {
158148
if (dashIdx > 0) {
159149
long firstPos = Long.parseLong(range.substring(0, dashIdx));
160150
if (dashIdx < range.length() - 1) {
161-
Long lastPos = Long.parseLong(range.substring(dashIdx + 1, range.length()));
151+
Long lastPos = Long.parseLong(range.substring(dashIdx + 1));
162152
return new ByteRange(firstPos, lastPos);
163153
}
164154
else {
@@ -195,13 +185,25 @@ public static List<ResourceRegion> toResourceRegions(List<HttpRange> ranges, Res
195185
if (ranges.size() > 1) {
196186
long length = getLengthFor(resource);
197187
long total = regions.stream().map(ResourceRegion::getCount).reduce(0L, (count, sum) -> sum + count);
198-
Assert.isTrue(total < length,
199-
() -> "The sum of all ranges (" + total + ") " +
200-
"should be less than the resource length (" + length + ")");
188+
if (total >= length) {
189+
throw new IllegalArgumentException("The sum of all ranges (" + total +
190+
") should be less than the resource length (" + length + ")");
191+
}
201192
}
202193
return regions;
203194
}
204195

196+
private static long getLengthFor(Resource resource) {
197+
try {
198+
long contentLength = resource.contentLength();
199+
Assert.isTrue(contentLength > 0, "Resource content length should be > 0");
200+
return contentLength;
201+
}
202+
catch (IOException ex) {
203+
throw new IllegalArgumentException("Failed to obtain Resource content length", ex);
204+
}
205+
}
206+
205207
/**
206208
* Return a string representation of the given list of {@code HttpRange} objects.
207209
* <p>This method can be used to for an {@code Range} header.

spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -53,9 +53,8 @@
5353
/**
5454
* {@code HttpMessageWriter} that can write a {@link Resource}.
5555
*
56-
* <p>Also an implementation of {@code HttpMessageWriter} with support
57-
* for writing one or more {@link ResourceRegion}'s based on the HTTP ranges
58-
* specified in the request.
56+
* <p>Also an implementation of {@code HttpMessageWriter} with support for writing one
57+
* or more {@link ResourceRegion}'s based on the HTTP ranges specified in the request.
5958
*
6059
* <p>For reading to a Resource, use {@link ResourceDecoder} wrapped with
6160
* {@link DecoderHttpMessageReader}.
@@ -187,7 +186,6 @@ private static Optional<Mono<Void>> zeroCopy(Resource resource, @Nullable Resour
187186
// Server-side only: single Resource or sub-regions...
188187

189188
@Override
190-
@SuppressWarnings("unchecked")
191189
public Mono<Void> write(Publisher<? extends Resource> inputStream, @Nullable ResolvableType actualType,
192190
ResolvableType elementType, @Nullable MediaType mediaType, ServerHttpRequest request,
193191
ServerHttpResponse response, Map<String, Object> hints) {
@@ -205,15 +203,12 @@ public Mono<Void> write(Publisher<? extends Resource> inputStream, @Nullable Res
205203
}
206204

207205
return Mono.from(inputStream).flatMap(resource -> {
208-
209206
if (ranges.isEmpty()) {
210207
return writeResource(resource, elementType, mediaType, response, hints);
211208
}
212-
213209
response.setStatusCode(HttpStatus.PARTIAL_CONTENT);
214210
List<ResourceRegion> regions = HttpRange.toResourceRegions(ranges, resource);
215211
MediaType resourceMediaType = getResourceMediaType(mediaType, resource, hints);
216-
217212
if (regions.size() == 1){
218213
ResourceRegion region = regions.get(0);
219214
headers.setContentType(resourceMediaType);

0 commit comments

Comments
 (0)