Deduplicate Range header parsing (#117304)

modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainerRetriesTests.java

@@ -41,6 +41,7 @@ import org.elasticsearch.repositories.blobstore.AbstractBlobContainerRetriesTest
 import org.elasticsearch.repositories.blobstore.ESMockAPIBasedRepositoryIntegTestCase;
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.rest.RestUtils;
+import org.elasticsearch.test.fixture.HttpHeaderParser;
 import org.threeten.bp.Duration;
 
 import java.io.IOException;
@@ -177,9 +178,9 @@ public class GoogleCloudStorageBlobContainerRetriesTests extends AbstractBlobCon
         httpServer.createContext(downloadStorageEndpoint(blobContainer, "large_blob_retries"), exchange -> {
             Streams.readFully(exchange.getRequestBody());
             exchange.getResponseHeaders().add("Content-Type", "application/octet-stream");
-            final Tuple<Long, Long> range = getRange(exchange);
+            final HttpHeaderParser.Range range = getRange(exchange);
-            final int offset = Math.toIntExact(range.v1());
+            final int offset = Math.toIntExact(range.start());
-            final byte[] chunk = Arrays.copyOfRange(bytes, offset, Math.toIntExact(Math.min(range.v2() + 1, bytes.length)));
+            final byte[] chunk = Arrays.copyOfRange(bytes, offset, Math.toIntExact(Math.min(range.end() + 1, bytes.length)));
             exchange.sendResponseHeaders(RestStatus.OK.getStatus(), chunk.length);
             if (randomBoolean() && countDown.decrementAndGet() >= 0) {
                 exchange.getResponseBody().write(chunk, 0, chunk.length - 1);

test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureHttpHandler.java

@@ -22,6 +22,7 @@ import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.SuppressForbidden;
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.rest.RestUtils;
+import org.elasticsearch.test.fixture.HttpHeaderParser;
 import org.elasticsearch.xcontent.XContentBuilder;
 import org.elasticsearch.xcontent.XContentType;
 
@@ -42,8 +43,6 @@ import java.util.Objects;
 import java.util.Set;
 import java.util.UUID;
 import java.util.function.Predicate;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 
 import static fixture.azure.MockAzureBlobStore.failTestWithAssertionError;
 import static org.elasticsearch.repositories.azure.AzureFixtureHelper.assertValidBlockId;
@@ -54,7 +53,6 @@ import static org.elasticsearch.repositories.azure.AzureFixtureHelper.assertVali
 @SuppressForbidden(reason = "Uses a HttpServer to emulate an Azure endpoint")
 public class AzureHttpHandler implements HttpHandler {
     private static final Logger logger = LogManager.getLogger(AzureHttpHandler.class);
-    private static final Pattern RANGE_HEADER_PATTERN = Pattern.compile("^bytes=([0-9]+)-([0-9]+)$");
     static final String X_MS_LEASE_ID = "x-ms-lease-id";
     static final String X_MS_PROPOSED_LEASE_ID = "x-ms-proposed-lease-id";
     static final String X_MS_LEASE_DURATION = "x-ms-lease-duration";
@@ -232,29 +230,26 @@ public class AzureHttpHandler implements HttpHandler {
                 final BytesReference responseContent;
                 final RestStatus successStatus;
                 // see Constants.HeaderConstants.STORAGE_RANGE_HEADER
-                final String range = exchange.getRequestHeaders().getFirst("x-ms-range");
+                final String rangeHeader = exchange.getRequestHeaders().getFirst("x-ms-range");
-                if (range != null) {
+                if (rangeHeader != null) {
-                    final Matcher matcher = RANGE_HEADER_PATTERN.matcher(range);
+                    final HttpHeaderParser.Range range = HttpHeaderParser.parseRangeHeader(rangeHeader);
-                    if (matcher.matches() == false) {
+                    if (range == null) {
                         throw new MockAzureBlobStore.BadRequestException(
                             "InvalidHeaderValue",
-                            "Range header does not match expected format: " + range
+                            "Range header does not match expected format: " + rangeHeader
                         );
                     }
 
-                    final long start = Long.parseLong(matcher.group(1));
-                    final long end = Long.parseLong(matcher.group(2));
-
                     final BytesReference blobContents = blob.getContents();
-                    if (blobContents.length() <= start) {
+                    if (blobContents.length() <= range.start()) {
                         exchange.getResponseHeaders().add("Content-Type", "application/octet-stream");
                         exchange.sendResponseHeaders(RestStatus.REQUESTED_RANGE_NOT_SATISFIED.getStatus(), -1);
                         return;
                     }
 
                     responseContent = blobContents.slice(
-                        Math.toIntExact(start),
+                        Math.toIntExact(range.start()),
-                        Math.toIntExact(Math.min(end - start + 1, blobContents.length() - start))
+                        Math.toIntExact(Math.min(range.end() - range.start() + 1, blobContents.length() - range.start()))
                     );
                     successStatus = RestStatus.PARTIAL_CONTENT;
                 } else {

test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java

@@ -24,6 +24,7 @@ import org.elasticsearch.core.SuppressForbidden;
 import org.elasticsearch.core.Tuple;
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.rest.RestUtils;
+import org.elasticsearch.test.fixture.HttpHeaderParser;
 
 import java.io.BufferedReader;
 import java.io.IOException;
@@ -58,8 +59,6 @@ public class GoogleCloudStorageHttpHandler implements HttpHandler {
 
     private static final Logger logger = LogManager.getLogger(GoogleCloudStorageHttpHandler.class);
 
-    private static final Pattern RANGE_MATCHER = Pattern.compile("bytes=([0-9]*)-([0-9]*)");
-
     private final ConcurrentMap<String, BytesReference> blobs;
     private final String bucket;
 
@@ -131,19 +130,19 @@ public class GoogleCloudStorageHttpHandler implements HttpHandler {
                 // Download Object https://cloud.google.com/storage/docs/request-body
                 BytesReference blob = blobs.get(exchange.getRequestURI().getPath().replace("/download/storage/v1/b/" + bucket + "/o/", ""));
                 if (blob != null) {
-                    final String range = exchange.getRequestHeaders().getFirst("Range");
+                    final String rangeHeader = exchange.getRequestHeaders().getFirst("Range");
                     final long offset;
                     final long end;
-                    if (range == null) {
+                    if (rangeHeader == null) {
                         offset = 0L;
                         end = blob.length() - 1;
                     } else {
-                        Matcher matcher = RANGE_MATCHER.matcher(range);
+                        final HttpHeaderParser.Range range = HttpHeaderParser.parseRangeHeader(rangeHeader);
-                        if (matcher.find() == false) {
+                        if (range == null) {
-                            throw new AssertionError("Range bytes header does not match expected format: " + range);
+                            throw new AssertionError("Range bytes header does not match expected format: " + rangeHeader);
                         }
-                        offset = Long.parseLong(matcher.group(1));
+                        offset = range.start();
-                        end = Long.parseLong(matcher.group(2));
+                        end = range.end();
                     }
 
                     if (offset >= blob.length()) {

test/fixtures/s3-fixture/build.gradle

@@ -15,5 +15,5 @@ dependencies {
   api("junit:junit:${versions.junit}") {
     transitive = false
   }
-  testImplementation project(':test:framework')
+  implementation project(':test:framework')
 }

test/fixtures/s3-fixture/src/main/java/fixture/s3/S3HttpHandler.java

@@ -28,6 +28,7 @@ import org.elasticsearch.logging.LogManager;
 import org.elasticsearch.logging.Logger;
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.rest.RestUtils;
+import org.elasticsearch.test.fixture.HttpHeaderParser;
 
 import java.io.IOException;
 import java.io.InputStreamReader;
@@ -269,8 +270,8 @@ public class S3HttpHandler implements HttpHandler {
                     exchange.sendResponseHeaders(RestStatus.NOT_FOUND.getStatus(), -1);
                     return;
                 }
-                final String range = exchange.getRequestHeaders().getFirst("Range");
+                final String rangeHeader = exchange.getRequestHeaders().getFirst("Range");
-                if (range == null) {
+                if (rangeHeader == null) {
                     exchange.getResponseHeaders().add("Content-Type", "application/octet-stream");
                     exchange.sendResponseHeaders(RestStatus.OK.getStatus(), blob.length());
                     blob.writeTo(exchange.getResponseBody());
@@ -281,17 +282,12 @@ public class S3HttpHandler implements HttpHandler {
                 // requests with a header value like "Range: bytes=start-end" where both {@code start} and {@code end} are always defined
                 // (sometimes to very high value for {@code end}). It would be too tedious to fully support the RFC so S3HttpHandler only
                 // supports when both {@code start} and {@code end} are defined to match the SDK behavior.
-                final Matcher matcher = Pattern.compile("^bytes=([0-9]+)-([0-9]+)$").matcher(range);
+                final HttpHeaderParser.Range range = HttpHeaderParser.parseRangeHeader(rangeHeader);
-                if (matcher.matches() == false) {
+                if (range == null) {
-                    throw new AssertionError("Bytes range does not match expected pattern: " + range);
+                    throw new AssertionError("Bytes range does not match expected pattern: " + rangeHeader);
                 }
-                var groupStart = matcher.group(1);
+                long start = range.start();
-                var groupEnd = matcher.group(2);
+                long end = range.end();
-                if (groupStart == null || groupEnd == null) {
-                    throw new AssertionError("Bytes range does not match expected pattern: " + range);
-                }
-                long start = Long.parseLong(groupStart);
-                long end = Long.parseLong(groupEnd);
                 if (end < start) {
                     exchange.getResponseHeaders().add("Content-Type", "application/octet-stream");
                     exchange.sendResponseHeaders(RestStatus.OK.getStatus(), blob.length());

test/fixtures/url-fixture/src/main/java/fixture/url/URLFixture.java

@@ -10,6 +10,7 @@ package fixture.url;
 
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.test.fixture.AbstractHttpFixture;
+import org.elasticsearch.test.fixture.HttpHeaderParser;
 import org.junit.rules.TemporaryFolder;
 import org.junit.rules.TestRule;
 
@@ -21,15 +22,12 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 
 /**
  * This {@link URLFixture} exposes a filesystem directory over HTTP. It is used in repository-url
  * integration tests to expose a directory created by a regular FS repository.
  */
 public class URLFixture extends AbstractHttpFixture implements TestRule {
-    private static final Pattern RANGE_PATTERN = Pattern.compile("bytes=(\\d+)-(\\d+)$");
     private final TemporaryFolder temporaryFolder;
     private Path repositoryDir;
 
@@ -60,19 +58,19 @@ public class URLFixture extends AbstractHttpFixture implements TestRule {
 
         if (normalizedPath.startsWith(normalizedRepositoryDir)) {
             if (Files.exists(normalizedPath) && Files.isReadable(normalizedPath) && Files.isRegularFile(normalizedPath)) {
-                final String range = request.getHeader("Range");
+                final String rangeHeader = request.getHeader("Range");
                 final Map<String, String> headers = new HashMap<>(contentType("application/octet-stream"));
-                if (range == null) {
+                if (rangeHeader == null) {
                     byte[] content = Files.readAllBytes(normalizedPath);
                     headers.put("Content-Length", String.valueOf(content.length));
                     return new Response(RestStatus.OK.getStatus(), headers, content);
                 } else {
-                    final Matcher matcher = RANGE_PATTERN.matcher(range);
+                    final HttpHeaderParser.Range range = HttpHeaderParser.parseRangeHeader(rangeHeader);
-                    if (matcher.matches() == false) {
+                    if (range == null) {
                         return new Response(RestStatus.REQUESTED_RANGE_NOT_SATISFIED.getStatus(), TEXT_PLAIN_CONTENT_TYPE, EMPTY_BYTE);
                     } else {
-                        long start = Long.parseLong(matcher.group(1));
+                        long start = range.start();
-                        long end = Long.parseLong(matcher.group(2));
+                        long end = range.end();
                         long rangeLength = end - start + 1;
                         final long fileSize = Files.size(normalizedPath);
                         if (start >= fileSize || start > end || rangeLength > fileSize) {

test/framework/src/main/java/org/elasticsearch/repositories/blobstore/AbstractBlobContainerRetriesTestCase.java

@@ -23,9 +23,9 @@ import org.elasticsearch.common.util.concurrent.CountDown;
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.SuppressForbidden;
 import org.elasticsearch.core.TimeValue;
-import org.elasticsearch.core.Tuple;
 import org.elasticsearch.mocksocket.MockHttpServer;
 import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.fixture.HttpHeaderParser;
 import org.junit.After;
 import org.junit.Before;
 
@@ -40,8 +40,6 @@ import java.util.Locale;
 import java.util.OptionalInt;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicLong;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 
 import static org.elasticsearch.repositories.blobstore.BlobStoreTestUtil.randomPurpose;
 import static org.elasticsearch.test.NeverMatcher.never;
@@ -371,28 +369,24 @@ public abstract class AbstractBlobContainerRetriesTestCase extends ESTestCase {
         return randomByteArrayOfLength(randomIntBetween(minSize, frequently() ? 512 : 1 << 20)); // rarely up to 1mb
     }
 
-    private static final Pattern RANGE_PATTERN = Pattern.compile("^bytes=([0-9]+)-([0-9]+)$");
+    protected static HttpHeaderParser.Range getRange(HttpExchange exchange) {
-
-    protected static Tuple<Long, Long> getRange(HttpExchange exchange) {
         final String rangeHeader = exchange.getRequestHeaders().getFirst("Range");
         if (rangeHeader == null) {
-            return Tuple.tuple(0L, MAX_RANGE_VAL);
+            return new HttpHeaderParser.Range(0L, MAX_RANGE_VAL);
         }
 
-        final Matcher matcher = RANGE_PATTERN.matcher(rangeHeader);
+        final HttpHeaderParser.Range range = HttpHeaderParser.parseRangeHeader(rangeHeader);
-        assertTrue(rangeHeader + " matches expected pattern", matcher.matches());
+        assertNotNull(rangeHeader + " matches expected pattern", range);
-        long rangeStart = Long.parseLong(matcher.group(1));
+        assertThat(range.start(), lessThanOrEqualTo(range.end()));
-        long rangeEnd = Long.parseLong(matcher.group(2));
+        return range;
-        assertThat(rangeStart, lessThanOrEqualTo(rangeEnd));
-        return Tuple.tuple(rangeStart, rangeEnd);
     }
 
     protected static int getRangeStart(HttpExchange exchange) {
-        return Math.toIntExact(getRange(exchange).v1());
+        return Math.toIntExact(getRange(exchange).start());
     }
 
     protected static OptionalInt getRangeEnd(HttpExchange exchange) {
-        final long rangeEnd = getRange(exchange).v2();
+        final long rangeEnd = getRange(exchange).end();
         if (rangeEnd == MAX_RANGE_VAL) {
             return OptionalInt.empty();
         }

test/framework/src/main/java/org/elasticsearch/test/fixture/HttpHeaderParser.java

@@ -0,0 +1,42 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+package org.elasticsearch.test.fixture;
+
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+public enum HttpHeaderParser {
+    ;
+
+    private static final Pattern RANGE_HEADER_PATTERN = Pattern.compile("bytes=([0-9]+)-([0-9]+)");
+
+    /**
+     * Parse a "Range" header
+     *
+     * Note: only a single bounded range is supported (e.g. <code>Range: bytes={range_start}-{range_end}</code>)
+     *
+     * @see <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Range">MDN: Range header</a>
+     * @param rangeHeaderValue The header value as a string
+     * @return a {@link Range} instance representing the parsed value, or null if the header is malformed
+     */
+    public static Range parseRangeHeader(String rangeHeaderValue) {
+        final Matcher matcher = RANGE_HEADER_PATTERN.matcher(rangeHeaderValue);
+        if (matcher.matches()) {
+            try {
+                return new Range(Long.parseLong(matcher.group(1)), Long.parseLong(matcher.group(2)));
+            } catch (NumberFormatException e) {
+                return null;
+            }
+        }
+        return null;
+    }
+
+    public record Range(long start, long end) {}
+}

test/framework/src/test/java/org/elasticsearch/http/HttpHeaderParserTests.java

@@ -0,0 +1,53 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+package org.elasticsearch.http;
+
+import org.elasticsearch.common.Strings;
+import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.fixture.HttpHeaderParser;
+
+import java.math.BigInteger;
+
+public class HttpHeaderParserTests extends ESTestCase {
+
+    public void testParseRangeHeader() {
+        final long start = randomLongBetween(0, 10_000);
+        final long end = randomLongBetween(start, start + 10_000);
+        assertEquals(new HttpHeaderParser.Range(start, end), HttpHeaderParser.parseRangeHeader("bytes=" + start + "-" + end));
+    }
+
+    public void testParseRangeHeaderInvalidLong() {
+        final BigInteger longOverflow = BigInteger.valueOf(Long.MAX_VALUE).add(BigInteger.ONE).add(randomBigInteger());
+        assertNull(HttpHeaderParser.parseRangeHeader("bytes=123-" + longOverflow));
+        assertNull(HttpHeaderParser.parseRangeHeader("bytes=" + longOverflow + "-123"));
+    }
+
+    public void testParseRangeHeaderMultipleRangesNotMatched() {
+        assertNull(
+            HttpHeaderParser.parseRangeHeader(
+                Strings.format(
+                    "bytes=%d-%d,%d-%d",
+                    randomIntBetween(0, 99),
+                    randomIntBetween(100, 199),
+                    randomIntBetween(200, 299),
+                    randomIntBetween(300, 399)
+                )
+            )
+        );
+    }
+
+    public void testParseRangeHeaderEndlessRangeNotMatched() {
+        assertNull(HttpHeaderParser.parseRangeHeader(Strings.format("bytes=%d-", randomLongBetween(0, Long.MAX_VALUE))));
+    }
+
+    public void testParseRangeHeaderSuffixLengthNotMatched() {
+        assertNull(HttpHeaderParser.parseRangeHeader(Strings.format("bytes=-%d", randomLongBetween(0, Long.MAX_VALUE))));
+    }
+}