Skip to content

Commit

Permalink
fix: optimize handling of requests containing Range header (#9484)
Browse files Browse the repository at this point in the history 
More efficient parsing of the header value. Also, range count is capped at 16
(additional will be ignored) and overlapping ranges at 2 (request will be denied
if above).
  • Loading branch information
Johannes Eriksson committed Dec 17, 2020
1 parent f029101 commit faf9975
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 6 deletions.
60 changes: 54 additions & 6 deletions flow-server/src/main/java/com/vaadin/flow/internal/ResponseWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
import java.net.MalformedURLException;
import java.net.URL;
import java.net.URLConnection;
import java.util.ArrayList;
import java.util.List;
import java.util.Stack;
import java.util.UUID;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand All @@ -51,8 +51,20 @@
public class ResponseWriter implements Serializable {
private static final int DEFAULT_BUFFER_SIZE = 32 * 1024;

private static final Pattern RANGE_HEADER_PATTERN = Pattern.compile("^bytes=(([0-9]*-[0-9]*,?\\s*)+)$");
private static final Pattern BYTE_RANGE_PATTERN = Pattern.compile("([0-9]*)-([0-9]*)");
private static final Pattern RANGE_HEADER_PATTERN = Pattern.compile(
"^bytes=((\\d*-\\d*\\s*,\\s*)*\\d*-\\d*\\s*)$");
private static final Pattern BYTE_RANGE_PATTERN = Pattern.compile(
"(\\d*)-(\\d*)");

/**
* Maximum number of ranges accepted in a single Range header. Remaining ranges will be ignored.
*/
private static final int MAX_RANGE_COUNT = 16;

/**
* Maximum number of overlapping ranges allowed. The request will be denied if above this threshold.
*/
private static final int MAX_OVERLAPPING_RANGE_COUNT = 2;

private final int bufferSize;
private final boolean brotliEnabled;
Expand Down Expand Up @@ -196,13 +208,14 @@ private void writeRangeContents(String range, HttpServletResponse response,
long resourceLength = connection.getContentLengthLong();
Matcher rangeMatcher = BYTE_RANGE_PATTERN.matcher(byteRanges);

List<Pair<Long, Long>> ranges = new ArrayList<>();
while (rangeMatcher.find()) {
Stack<Pair<Long, Long>> ranges = new Stack<>();
while (rangeMatcher.find() && ranges.size() < MAX_RANGE_COUNT) {
String startGroup = rangeMatcher.group(1);
String endGroup = rangeMatcher.group(2);
if (startGroup.isEmpty() && endGroup.isEmpty()) {
response.setContentLengthLong(0L);
response.setStatus(416); // Range Not Satisfiable
getLogger().info("received a malformed range: '{}'", rangeMatcher.group());
return;
}
long start = startGroup.isEmpty() ? 0L : Long.parseLong(startGroup);
Expand All @@ -211,11 +224,20 @@ private void writeRangeContents(String range, HttpServletResponse response,
if (end < start
|| (resourceLength >= 0 && start >= resourceLength)) {
// illegal range -> 416
getLogger().info("received an illegal range '{}' for resource '{}'",
rangeMatcher.group(), resourceURL);
response.setContentLengthLong(0L);
response.setStatus(416);
return;
}
ranges.add(new Pair<>(start, end));
ranges.push(new Pair<>(start, end));

if (!verifyRangeLimits(ranges)) {
ranges.pop();
getLogger().info("serving only {} ranges for resource '{}' even though more were requested",
ranges.size(), resourceURL);
break;
}
}

response.setStatus(206);
Expand Down Expand Up @@ -314,6 +336,32 @@ private void setContentLength(HttpServletResponse response,
}
}

/**
* Returns true if the number of ranges in <code>ranges</code> is less than the
* upper limit and the number that overlap (= have at least one byte in common)
* with the range <code>[start, end]</code> are less than the upper limit.
*/
private boolean verifyRangeLimits(List<Pair<Long, Long>> ranges) {
if (ranges.size() > MAX_RANGE_COUNT) {
getLogger().info("more than {} ranges requested", MAX_RANGE_COUNT);
return false;
}
int count = 0;
for (int i = 0; i < ranges.size(); i++) {
for (int j = i + 1; j < ranges.size(); j++) {
if (ranges.get(i).getFirst() <= ranges.get(j).getSecond()
&& ranges.get(j).getFirst() <= ranges.get(i).getSecond()) {
count++;
}
}
}
if (count > MAX_OVERLAPPING_RANGE_COUNT) {
getLogger().info("more than {} overlapping ranges requested", MAX_OVERLAPPING_RANGE_COUNT);
return false;
}
return true;
}

private URL getResource(HttpServletRequest request, String resource)
throws MalformedURLException {
URL url = request.getServletContext().getResource(resource);
Expand Down
49 changes: 49 additions & 0 deletions flow-server/src/test/java/com/vaadin/flow/internal/ResponseWriterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,44 @@ public void writeByteRangeMultiPartOverlapping() throws IOException {
assertStatus(206);
}

@Test
public void writeByteRangeMultiPartTooManyRequested() throws IOException {
makePathsAvailable(PATH_JS);
mockRequestHeaders(new Pair<>("Range", "bytes=0-0, 0-0, 1-1, 2-2, 3-3, 4-4, 5-5, 6-6, 7-7, 8-8, 9-9, 10-10, 11-11, 12-12, 13-13, 14-14, 15-15, 16-16"));
// "File.js contents"
// ^0123456789ABCDEF^
assertMultipartResponse(PATH_JS, Arrays.asList(
new Pair<>(new String[]{"Content-Range: bytes 0-0/16"}, "F".getBytes()),
new Pair<>(new String[]{"Content-Range: bytes 0-0/16"}, "F".getBytes()),
new Pair<>(new String[]{"Content-Range: bytes 1-1/16"}, "i".getBytes()),
new Pair<>(new String[]{"Content-Range: bytes 2-2/16"}, "l".getBytes()),
new Pair<>(new String[]{"Content-Range: bytes 3-3/16"}, "e".getBytes()),
new Pair<>(new String[]{"Content-Range: bytes 4-4/16"}, ".".getBytes()),
new Pair<>(new String[]{"Content-Range: bytes 5-5/16"}, "j".getBytes()),
new Pair<>(new String[]{"Content-Range: bytes 6-6/16"}, "s".getBytes()),
new Pair<>(new String[]{"Content-Range: bytes 7-7/16"}, " ".getBytes()),
new Pair<>(new String[]{"Content-Range: bytes 8-8/16"}, "c".getBytes()),
new Pair<>(new String[]{"Content-Range: bytes 9-9/16"}, "o".getBytes()),
new Pair<>(new String[]{"Content-Range: bytes 10-10/16"}, "n".getBytes()),
new Pair<>(new String[]{"Content-Range: bytes 11-11/16"}, "t".getBytes()),
new Pair<>(new String[]{"Content-Range: bytes 12-12/16"}, "e".getBytes()),
new Pair<>(new String[]{"Content-Range: bytes 13-13/16"}, "n".getBytes()),
new Pair<>(new String[]{"Content-Range: bytes 14-14/16"}, "t".getBytes())));
assertStatus(206);
}

@Test
public void writeByteRangeMultiPartTooManyOverlappingRequested() throws IOException {
makePathsAvailable(PATH_JS);
mockRequestHeaders(new Pair<>("Range", "bytes=2-4, 0-4, 3-14"));
// "File.js contents"
// ^0123456789ABCDEF^
assertMultipartResponse(PATH_JS, Arrays.asList(
new Pair<>(new String[]{"Content-Range: bytes 2-4/16"}, "le.".getBytes()),
new Pair<>(new String[]{"Content-Range: bytes 0-4/16"}, "File.".getBytes())));
assertStatus(206);
}

private void assertResponse(byte[] expectedResponse) throws IOException {
assertResponse(PATH_JS, expectedResponse);
}
Expand Down Expand Up @@ -542,6 +580,17 @@ private void assertMultipartResponse(String path,
byte[] bytes = outputStream.toByteArray();
Assert.assertArrayEquals(expectedBytes, bytes);
}

// check that there are no excess parts
try {
mps.readHeaders();
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
mps.readBodyData(outputStream);
Assert.assertTrue("excess bytes in multipart response",
outputStream.toByteArray().length == 0);
} catch (IOException ioe) {
// all is well, stream ended
}
}

private void assertStatus(int status) {
Expand Down

0 comments on commit faf9975

Please sign in to comment.