Skip to content

Deduplicate Range header parsing #126104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
elasticsearchmachine merged 3 commits into from
Apr 4, 2025
Merged

Deduplicate Range header parsing #126104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d9bb380
Deduplicate Range header parsing
nicktindall Nov 30, 2024
019daf6
Merge branch '8.x' into 2025/04/02/backport-117304-range-header-parsing
DaveCTurner Apr 4, 2025
f3db334
[CI] Auto commit changes from spotless
Apr 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions .../java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainerRetriesTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
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;
Expand Down Expand Up @@ -178,9 +179,9 @@ public void testReadLargeBlobWithRetries() throws Exception {
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 int offset = Math.toIntExact(range.v1());
final byte[] chunk = Arrays.copyOfRange(bytes, offset, Math.toIntExact(Math.min(range.v2() + 1, bytes.length)));
final HttpHeaderParser.Range range = getRange(exchange);
final int offset = Math.toIntExact(range.start());
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);
Expand Down
24 changes: 13 additions & 11 deletions test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureHttpHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
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;

Expand All @@ -37,8 +38,6 @@
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Predicate;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import static org.elasticsearch.repositories.azure.AzureFixtureHelper.assertValidBlockId;

Expand Down Expand Up @@ -180,22 +179,25 @@ public void handle(final HttpExchange exchange) throws IOException {
}

// see Constants.HeaderConstants.STORAGE_RANGE_HEADER
final String range = exchange.getRequestHeaders().getFirst("x-ms-range");
final Matcher matcher = Pattern.compile("^bytes=([0-9]+)-([0-9]+)$").matcher(range);
if (matcher.matches() == false) {
throw new AssertionError("Range header does not match expected format: " + range);
final String rangeHeader = exchange.getRequestHeaders().getFirst("x-ms-range");
if (rangeHeader == null) {
throw new AssertionError("Range header is required");
}
final HttpHeaderParser.Range range = HttpHeaderParser.parseRangeHeader(rangeHeader);
if (range == null) {
throw new AssertionError("Range header does not match expected format: " + rangeHeader);
}

final long start = Long.parseLong(matcher.group(1));
final long end = Long.parseLong(matcher.group(2));

if (blob.length() <= start) {
if (blob.length() <= range.start()) {
exchange.getResponseHeaders().add("Content-Type", "application/octet-stream");
exchange.sendResponseHeaders(RestStatus.REQUESTED_RANGE_NOT_SATISFIED.getStatus(), -1);
return;
}

var responseBlob = blob.slice(Math.toIntExact(start), Math.toIntExact(Math.min(end - start + 1, blob.length() - start)));
var responseBlob = blob.slice(
Math.toIntExact(range.start()),
Math.toIntExact(Math.min(range.end() - range.start() + 1, blob.length() - range.start()))
);

exchange.getResponseHeaders().add("Content-Type", "application/octet-stream");
exchange.getResponseHeaders().add("x-ms-blob-content-length", String.valueOf(responseBlob.length()));
Expand Down
17 changes: 8 additions & 9 deletions test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -131,19 +130,19 @@ public void handle(final HttpExchange exchange) throws IOException {
// 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);
if (matcher.find() == false) {
throw new AssertionError("Range bytes header does not match expected format: " + range);
final HttpHeaderParser.Range range = HttpHeaderParser.parseRangeHeader(rangeHeader);
if (range == null) {
throw new AssertionError("Range bytes header does not match expected format: " + rangeHeader);
}
offset = Long.parseLong(matcher.group(1));
end = Long.parseLong(matcher.group(2));
offset = range.start();
end = range.end();
}

if (offset >= blob.length()) {
Expand Down
20 changes: 8 additions & 12 deletions test/fixtures/s3-fixture/src/main/java/fixture/s3/S3HttpHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.elasticsearch.logging.LogManager;
import org.elasticsearch.logging.Logger;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.test.fixture.HttpHeaderParser;

import java.io.IOException;
import java.io.InputStreamReader;
Expand Down Expand Up @@ -256,8 +257,8 @@ public void handle(final HttpExchange exchange) throws IOException {
exchange.sendResponseHeaders(RestStatus.NOT_FOUND.getStatus(), -1);
return;
}
final String range = exchange.getRequestHeaders().getFirst("Range");
if (range == null) {
final String rangeHeader = exchange.getRequestHeaders().getFirst("Range");
if (rangeHeader == null) {
exchange.getResponseHeaders().add("Content-Type", "application/octet-stream");
exchange.sendResponseHeaders(RestStatus.OK.getStatus(), blob.length());
blob.writeTo(exchange.getResponseBody());
Expand All @@ -268,17 +269,12 @@ public void handle(final HttpExchange exchange) throws IOException {
// 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);
if (matcher.matches() == false) {
throw new AssertionError("Bytes range does not match expected pattern: " + range);
}
var groupStart = matcher.group(1);
var groupEnd = matcher.group(2);
if (groupStart == null || groupEnd == null) {
throw new AssertionError("Bytes range does not match expected pattern: " + range);
final HttpHeaderParser.Range range = HttpHeaderParser.parseRangeHeader(rangeHeader);
if (range == null) {
throw new AssertionError("Bytes range does not match expected pattern: " + rangeHeader);
}
long start = Long.parseLong(groupStart);
long end = Long.parseLong(groupEnd);
long start = range.start();
long end = range.end();
if (end < start) {
exchange.getResponseHeaders().add("Content-Type", "application/octet-stream");
exchange.sendResponseHeaders(RestStatus.OK.getStatus(), blob.length());
Expand Down
16 changes: 7 additions & 9 deletions test/fixtures/url-fixture/src/main/java/fixture/url/URLFixture.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

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;

Expand All @@ -21,15 +22,12 @@
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;

Expand Down Expand Up @@ -60,19 +58,19 @@ private AbstractHttpFixture.Response handleGetRequest(Request request) throws IO

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);
if (matcher.matches() == false) {
final HttpHeaderParser.Range range = HttpHeaderParser.parseRangeHeader(rangeHeader);
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 end = Long.parseLong(matcher.group(2));
long start = range.start();
long end = range.end();
long rangeLength = end - start + 1;
final long fileSize = Files.size(normalizedPath);
if (start >= fileSize || start > end || rangeLength > fileSize) {
Expand Down
24 changes: 9 additions & 15 deletions ...n/java/org/elasticsearch/repositories/blobstore/AbstractBlobContainerRetriesTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
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;

Expand All @@ -40,8 +40,6 @@
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;
Expand Down Expand Up @@ -373,28 +371,24 @@ protected static byte[] randomBlobContent(int minSize) {
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 Tuple<Long, Long> getRange(HttpExchange exchange) {
protected static HttpHeaderParser.Range 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);
assertTrue(rangeHeader + " matches expected pattern", matcher.matches());
long rangeStart = Long.parseLong(matcher.group(1));
long rangeEnd = Long.parseLong(matcher.group(2));
assertThat(rangeStart, lessThanOrEqualTo(rangeEnd));
return Tuple.tuple(rangeStart, rangeEnd);
final HttpHeaderParser.Range range = HttpHeaderParser.parseRangeHeader(rangeHeader);
assertNotNull(rangeHeader + " matches expected pattern", range);
assertThat(range.start(), lessThanOrEqualTo(range.end()));
return range;
}

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();
}
Expand Down