From 38f5a2a57f3294dcee91772deb10f2740d94c1ba Mon Sep 17 00:00:00 2001 From: Finn Carroll Date: Wed, 29 Apr 2026 08:17:52 -0700 Subject: [PATCH 1/2] Remove X-Request-Id format restrictions and make size configurable (#21048) Introduces dynamic http.request_id.max_length setting for configuring maximum length for X-Request-Id headers. Removes the alpha-numeric validation of X-Request-Id. Signed-off-by: Finn Carroll --- .../opensearch/rest/Netty4RequestIdIT.java | 61 +++++++++++++++++ .../org/opensearch/action/ActionModule.java | 6 ++ .../common/settings/ClusterSettings.java | 1 + .../opensearch/common/util/RequestUtils.java | 19 ++---- .../http/HttpTransportSettings.java | 9 +++ .../org/opensearch/rest/RestController.java | 10 ++- .../common/util/RequestUtilsTests.java | 67 +++++++++++-------- 7 files changed, 132 insertions(+), 41 deletions(-) create mode 100644 modules/transport-netty4/src/javaRestTest/java/org/opensearch/rest/Netty4RequestIdIT.java diff --git a/modules/transport-netty4/src/javaRestTest/java/org/opensearch/rest/Netty4RequestIdIT.java b/modules/transport-netty4/src/javaRestTest/java/org/opensearch/rest/Netty4RequestIdIT.java new file mode 100644 index 0000000000000..926c32a71d1b5 --- /dev/null +++ b/modules/transport-netty4/src/javaRestTest/java/org/opensearch/rest/Netty4RequestIdIT.java @@ -0,0 +1,61 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.rest; + +import org.opensearch.client.Request; +import org.opensearch.client.RequestOptions; +import org.opensearch.client.Response; +import org.opensearch.client.ResponseException; +import org.opensearch.test.rest.OpenSearchRestTestCase; + +import java.io.IOException; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; + +public class Netty4RequestIdIT extends OpenSearchRestTestCase { + + private Response requestWithId(String requestId) throws IOException { + Request request = new Request("GET", "/_cluster/health"); + RequestOptions.Builder options = request.getOptions().toBuilder(); + options.addHeader("X-Request-Id", requestId); + request.setOptions(options); + return client().performRequest(request); + } + + public void testRequestIdExactlyAtMax() throws IOException { + assertThat(requestWithId("a".repeat(128)).getStatusLine().getStatusCode(), equalTo(200)); + } + + public void testRequestIdTooLong() { + ResponseException e = expectThrows(ResponseException.class, () -> requestWithId("a".repeat(129))); + assertThat(e.getResponse().getStatusLine().getStatusCode(), equalTo(400)); + assertThat(e.getMessage(), containsString("exceeds maximum allowed length")); + } + + public void testRequestIdAfterSettingUpdate() throws IOException { + int newMax = 20; + + // Expect request is valid under default + assertThat(requestWithId("a".repeat(128)).getStatusLine().getStatusCode(), equalTo(200)); + + // Update setting + Request updateSettings = new Request("PUT", "/_cluster/settings"); + updateSettings.setJsonEntity("{\"transient\": {\"http.request_id.max_length\": " + newMax + "}}"); + client().performRequest(updateSettings); + + // Was valid under default, now too long + ResponseException e = expectThrows(ResponseException.class, () -> requestWithId("a".repeat(129))); + assertThat(e.getResponse().getStatusLine().getStatusCode(), equalTo(400)); + assertThat(e.getMessage(), containsString("exceeds maximum allowed length [" + newMax + "]")); + + // ID at new size passes + assertThat(requestWithId("a".repeat(newMax)).getStatusLine().getStatusCode(), equalTo(200)); + } +} diff --git a/server/src/main/java/org/opensearch/action/ActionModule.java b/server/src/main/java/org/opensearch/action/ActionModule.java index 1ddb988dc6508..6a60e5c57ab4c 100644 --- a/server/src/main/java/org/opensearch/action/ActionModule.java +++ b/server/src/main/java/org/opensearch/action/ActionModule.java @@ -330,6 +330,7 @@ import org.opensearch.extensions.action.ExtensionProxyTransportAction; import org.opensearch.extensions.rest.RestInitializeExtensionAction; import org.opensearch.extensions.rest.RestSendToExtensionAction; +import org.opensearch.http.HttpTransportSettings; import org.opensearch.identity.IdentityService; import org.opensearch.index.seqno.RetentionLeaseActions; import org.opensearch.indices.SystemIndices; @@ -613,6 +614,11 @@ public ActionModule( ); restController = new RestController(headers, restWrapper, nodeClient, circuitBreakerService, usageService); + restController.setRequestIdMaxLength(HttpTransportSettings.SETTING_HTTP_REQUEST_ID_MAX_LENGTH.get(settings)); + clusterSettings.addSettingsUpdateConsumer( + HttpTransportSettings.SETTING_HTTP_REQUEST_ID_MAX_LENGTH, + restController::setRequestIdMaxLength + ); responseLimitSettings = new ResponseLimitSettings(clusterSettings, settings); } diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index b10396f083111..2bb517e385ef3 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -428,6 +428,7 @@ public void apply(Settings value, Settings current, Settings previous) { HttpTransportSettings.SETTING_HTTP_TRACE_LOG_INCLUDE, HttpTransportSettings.SETTING_HTTP_TRACE_LOG_EXCLUDE, HttpTransportSettings.SETTING_HTTP_HTTP3_ENABLED, + HttpTransportSettings.SETTING_HTTP_REQUEST_ID_MAX_LENGTH, HierarchyCircuitBreakerService.USE_REAL_MEMORY_USAGE_SETTING, HierarchyCircuitBreakerService.TOTAL_CIRCUIT_BREAKER_LIMIT_SETTING, HierarchyCircuitBreakerService.FIELDDATA_CIRCUIT_BREAKER_LIMIT_SETTING, diff --git a/server/src/main/java/org/opensearch/common/util/RequestUtils.java b/server/src/main/java/org/opensearch/common/util/RequestUtils.java index dae4404b527e0..03f9d77f2de24 100644 --- a/server/src/main/java/org/opensearch/common/util/RequestUtils.java +++ b/server/src/main/java/org/opensearch/common/util/RequestUtils.java @@ -27,22 +27,17 @@ public static String generateID() { } /** - * Validate whether X-Request-id is valid or not. + * Validate whether X-Request-Id is valid or not. + * The request ID must be non-empty and not exceed the configured maximum length. */ - public static void validateRequestId(String requestId) { + public static void validateRequestId(String requestId, int maxLength) { if (requestId == null || requestId.isBlank()) { throw new IllegalArgumentException("X-Request-Id should not be null or empty"); } - - if (requestId.length() != 32) { - throw new IllegalArgumentException("Invalid X-Request-Id passed. Should be 32 hexadecimal characters: " + requestId); - } - - for (int i = 0; i < requestId.length(); i++) { - char c = requestId.charAt(i); - if (!((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F'))) { - throw new IllegalArgumentException("Invalid X-Request-Id passed: " + requestId); - } + if (requestId.length() > maxLength) { + throw new IllegalArgumentException( + "X-Request-Id length [" + requestId.length() + "] exceeds maximum allowed length [" + maxLength + "]" + ); } } diff --git a/server/src/main/java/org/opensearch/http/HttpTransportSettings.java b/server/src/main/java/org/opensearch/http/HttpTransportSettings.java index 2405798009764..75257faee4de3 100644 --- a/server/src/main/java/org/opensearch/http/HttpTransportSettings.java +++ b/server/src/main/java/org/opensearch/http/HttpTransportSettings.java @@ -259,6 +259,15 @@ public final class HttpTransportSettings { Setting.Property.NodeScope ); + public static final Setting SETTING_HTTP_REQUEST_ID_MAX_LENGTH = intSetting( + "http.request_id.max_length", + 128, + 16, + 1024, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); + // Enable HTTP/3 protocol if supported by the operating system and architecture // The HTTP/3 transport is still experimental and should be used with caution. public static final Setting SETTING_HTTP_HTTP3_ENABLED = Setting.boolSetting( diff --git a/server/src/main/java/org/opensearch/rest/RestController.java b/server/src/main/java/org/opensearch/rest/RestController.java index 872bddf2f107c..ad148b07a9966 100644 --- a/server/src/main/java/org/opensearch/rest/RestController.java +++ b/server/src/main/java/org/opensearch/rest/RestController.java @@ -40,6 +40,7 @@ import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.common.logging.DeprecationLogger; import org.opensearch.common.path.PathTrie; +import org.opensearch.common.settings.Settings; import org.opensearch.common.util.RequestUtils; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.common.util.io.Streams; @@ -55,6 +56,7 @@ import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.http.HttpChunk; import org.opensearch.http.HttpServerTransport; +import org.opensearch.http.HttpTransportSettings; import org.opensearch.tasks.Task; import org.opensearch.transport.client.node.NodeClient; import org.opensearch.usage.UsageService; @@ -98,6 +100,8 @@ public class RestController implements HttpServerTransport.Dispatcher { private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestController.class); private static final String OPENSEARCH_PRODUCT_ORIGIN_HTTP_HEADER = "X-opensearch-product-origin"; + private volatile int requestIdMaxLength = HttpTransportSettings.SETTING_HTTP_REQUEST_ID_MAX_LENGTH.getDefault(Settings.EMPTY); + private static final BytesReference FAVICON_RESPONSE; static { @@ -145,6 +149,10 @@ public RestController( ); } + public void setRequestIdMaxLength(int maxLength) { + this.requestIdMaxLength = maxLength; + } + /** * Returns an iterator over registered REST method handlers. * @return {@link Iterator} of {@link MethodHandlers} @@ -435,7 +443,7 @@ private void tryAllHandlers(final RestRequest request, final RestChannel channel threadContext.putHeader(name, String.join(",", distinctHeaderValues)); // Validate request-id header if present if (Task.X_REQUEST_ID.equals(restHeader.getName())) { - RequestUtils.validateRequestId(distinctHeaderValues.getFirst()); + RequestUtils.validateRequestId(distinctHeaderValues.getFirst(), requestIdMaxLength); } } } diff --git a/server/src/test/java/org/opensearch/common/util/RequestUtilsTests.java b/server/src/test/java/org/opensearch/common/util/RequestUtilsTests.java index ef05091cd045a..32a716856f0e6 100644 --- a/server/src/test/java/org/opensearch/common/util/RequestUtilsTests.java +++ b/server/src/test/java/org/opensearch/common/util/RequestUtilsTests.java @@ -8,73 +8,84 @@ package org.opensearch.common.util; +import org.opensearch.common.settings.Settings; import org.opensearch.core.common.Strings; +import org.opensearch.http.HttpTransportSettings; import org.opensearch.test.OpenSearchTestCase; public class RequestUtilsTests extends OpenSearchTestCase { + private static final int DEFAULT_MAX_LENGTH = HttpTransportSettings.SETTING_HTTP_REQUEST_ID_MAX_LENGTH.getDefault(Settings.EMPTY); + public void testGenerateID() { assertTrue(Strings.hasText(RequestUtils.generateID())); } public void testValidateRequestIdValid() { - RequestUtils.validateRequestId("a1b2c3d4e5f67890abcdef1234567890"); - RequestUtils.validateRequestId("ABCDEF1234567890abcdef1234567890"); - RequestUtils.validateRequestId("00000000000000000000000000000000"); - RequestUtils.validateRequestId("ffffffffffffffffffffffffffffffff"); - RequestUtils.validateRequestId("FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF"); + RequestUtils.validateRequestId("a1b2c3d4e5f67890abcdef1234567890", DEFAULT_MAX_LENGTH); + RequestUtils.validateRequestId("ABCDEF1234567890abcdef1234567890", DEFAULT_MAX_LENGTH); + RequestUtils.validateRequestId("00000000000000000000000000000000", DEFAULT_MAX_LENGTH); + RequestUtils.validateRequestId("ffffffffffffffffffffffffffffffff", DEFAULT_MAX_LENGTH); + RequestUtils.validateRequestId("FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF", DEFAULT_MAX_LENGTH); } public void testValidateRequestIdNull() { - IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> RequestUtils.validateRequestId(null)); + IllegalArgumentException exception = expectThrows( + IllegalArgumentException.class, + () -> RequestUtils.validateRequestId(null, DEFAULT_MAX_LENGTH) + ); assertEquals("X-Request-Id should not be null or empty", exception.getMessage()); } public void testValidateRequestIdEmpty() { - IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> RequestUtils.validateRequestId("")); + IllegalArgumentException exception = expectThrows( + IllegalArgumentException.class, + () -> RequestUtils.validateRequestId("", DEFAULT_MAX_LENGTH) + ); assertEquals("X-Request-Id should not be null or empty", exception.getMessage()); } public void testValidateRequestIdBlank() { - IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> RequestUtils.validateRequestId(" ")); - assertEquals("X-Request-Id should not be null or empty", exception.getMessage()); - } - - public void testValidateRequestIdTooShort() { IllegalArgumentException exception = expectThrows( IllegalArgumentException.class, - () -> RequestUtils.validateRequestId("a1b2c3d4e5f67890") + () -> RequestUtils.validateRequestId(" ", DEFAULT_MAX_LENGTH) ); - assertEquals("Invalid X-Request-Id passed. Should be 32 hexadecimal characters: a1b2c3d4e5f67890", exception.getMessage()); + assertEquals("X-Request-Id should not be null or empty", exception.getMessage()); } public void testValidateRequestIdTooLong() { + String tooLong = "a".repeat(DEFAULT_MAX_LENGTH + 1); IllegalArgumentException exception = expectThrows( IllegalArgumentException.class, - () -> RequestUtils.validateRequestId("a1b2c3d4e5f67890abcdef1234567890extra") + () -> RequestUtils.validateRequestId(tooLong, DEFAULT_MAX_LENGTH) ); assertEquals( - "Invalid X-Request-Id passed. Should be 32 hexadecimal characters: a1b2c3d4e5f67890abcdef1234567890extra", + "X-Request-Id length [" + (DEFAULT_MAX_LENGTH + 1) + "] exceeds maximum allowed length [" + DEFAULT_MAX_LENGTH + "]", exception.getMessage() ); } - public void testValidateRequestIdInvalidCharacters() { - IllegalArgumentException exception = expectThrows( - IllegalArgumentException.class, - () -> RequestUtils.validateRequestId("g1b2c3d4e5f67890abcdef1234567890") - ); - assertEquals("Invalid X-Request-Id passed: g1b2c3d4e5f67890abcdef1234567890", exception.getMessage()); + public void testValidateRequestIdNonHexCharactersAllowed() { + // Previously rejected, now allowed + RequestUtils.validateRequestId("g1b2c3d4e5f67890abcdef1234567890", DEFAULT_MAX_LENGTH); } - public void testValidateRequestIdWithSpecialCharacters() { + public void testValidateRequestIdWithSpecialCharactersAllowed() { + // UUID with dashes - previously rejected, now allowed + RequestUtils.validateRequestId("a1b2c3d4-e5f6-7890-abcd-ef1234567890", DEFAULT_MAX_LENGTH); + } + + public void testValidateRequestIdExactlyAtMaxLength() { + RequestUtils.validateRequestId("a".repeat(DEFAULT_MAX_LENGTH), DEFAULT_MAX_LENGTH); + } + + public void testValidateRequestIdCustomMaxLength() { + RequestUtils.validateRequestId("a".repeat(256), 256); + IllegalArgumentException exception = expectThrows( IllegalArgumentException.class, - () -> RequestUtils.validateRequestId("a1b2c3d4-e5f6-7890-abcd-ef1234567890") - ); - assertEquals( - "Invalid X-Request-Id passed. Should be 32 hexadecimal characters: a1b2c3d4-e5f6-7890-abcd-ef1234567890", - exception.getMessage() + () -> RequestUtils.validateRequestId("a".repeat(33), 32) ); + assertEquals("X-Request-Id length [33] exceeds maximum allowed length [32]", exception.getMessage()); } } From 629706ff98daea7f0aff0cb3f9848344dc9b14f8 Mon Sep 17 00:00:00 2001 From: Finn Carroll Date: Wed, 29 Apr 2026 08:32:28 -0700 Subject: [PATCH 2/2] Changelog. Signed-off-by: Finn Carroll --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0216c216b100c..bb7033d896d67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,6 +65,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Harden detection of HTTP/3 support by ensuring Quic native libraries are available for the target platform ([#20680](https://github.com/opensearch-project/OpenSearch/pull/20680)) - Fix the regression of terms agg optimization at high cardinality ([#20623](https://github.com/opensearch-project/OpenSearch/pull/20623)) - Fix TLS cert hot-reload for Arrow Flight transport ([#20732](https://github.com/opensearch-project/OpenSearch/pull/20732)) +- Remove X-Request-Id format restrictions and make size configurable ([#21048](https://github.com/opensearch-project/OpenSearch/pull/21048)) ### Dependencies - Bump `com.google.auth:google-auth-library-oauth2-http` from 1.38.0 to 1.41.0 ([#20183](https://github.com/opensearch-project/OpenSearch/pull/20183))