Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

import org.opensearch.common.UUIDs;

import java.util.regex.Pattern;

/**
* Common utility methods for request handling.
*
Expand All @@ -19,6 +21,21 @@ public final class RequestUtils {

private RequestUtils() {}

/** Matches ASCII control characters (0x00-0x1F and 0x7F). */
private static final Pattern CONTROL_CHARS = Pattern.compile("\\p{Cntrl}");

/**
* Removes control characters from a client-provided header value so it is safe to emit in
* single-line log output and response headers. A {@code null} input is returned unchanged;
* well-formed values (UUIDs, hexadecimal or alphanumeric identifiers) are unaffected.
*/
public static String sanitizeHeaderValue(String value) {
if (value == null) {
return null;
}
return CONTROL_CHARS.matcher(value).replaceAll("");
}

/**
* Generates a new ID field for new documents.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import org.opensearch.common.lease.Releasables;
import org.opensearch.common.network.CloseableChannel;
import org.opensearch.common.util.BigArrays;
import org.opensearch.common.util.RequestUtils;
import org.opensearch.common.util.concurrent.ThreadContext;
import org.opensearch.core.action.ActionListener;
import org.opensearch.core.common.bytes.BytesArray;
Expand Down Expand Up @@ -147,12 +148,13 @@ public void sendResponse(RestResponse restResponse) {

corsHandler.setCorsResponseHeaders(httpRequest, httpResponse);

opaque = request.header(X_OPAQUE_ID);
opaque = RequestUtils.sanitizeHeaderValue(request.header(X_OPAQUE_ID));
if (opaque != null) {
setHeaderField(httpResponse, X_OPAQUE_ID, opaque);
}
if (request.header(X_REQUEST_ID) != null) {
setHeaderField(httpResponse, X_REQUEST_ID, request.header(X_REQUEST_ID));
final String requestId = RequestUtils.sanitizeHeaderValue(request.header(X_REQUEST_ID));
if (requestId != null) {
setHeaderField(httpResponse, X_REQUEST_ID, requestId);
}

// Add all custom headers
Expand Down
11 changes: 8 additions & 3 deletions server/src/main/java/org/opensearch/rest/RestController.java
Original file line number Diff line number Diff line change
Expand Up @@ -440,10 +440,15 @@ private void tryAllHandlers(final RestRequest request, final RestChannel channel
);
return;
} else {
threadContext.putHeader(name, String.join(",", distinctHeaderValues));
String headerValue = String.join(",", distinctHeaderValues);
// Normalize client-provided correlation headers so downstream log output stays single-line
if (Task.X_REQUEST_ID.equals(name) || Task.X_OPAQUE_ID.equals(name)) {
headerValue = RequestUtils.sanitizeHeaderValue(headerValue);
}
threadContext.putHeader(name, headerValue);
// Validate request-id header if present
if (Task.X_REQUEST_ID.equals(restHeader.getName())) {
RequestUtils.validateRequestId(distinctHeaderValues.getFirst(), requestIdMaxLength);
if (Task.X_REQUEST_ID.equals(name)) {
RequestUtils.validateRequestId(headerValue, requestIdMaxLength);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this previously passed distinctHeaderValues.getFirst(); it now passes the joined headerValue. These are equivalent for X-Request-Id: it's registered as single-valued (new RestHeaderDefinition(Task.X_REQUEST_ID, false)), so the guard above (L433) returns 400 for more than one value before this branch is reached — meaning distinctHeaderValues always has exactly one element here, and String.join(",", …) equals getFirst(). Validating headerValue directly is intentional: it validates the exact value stored in the ThreadContext (no chance of the validated and stored values drifting) and stays correct if the header were ever made multi-valued.

}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,31 @@ public void testValidateRequestIdCustomMaxLength() {
);
assertEquals("X-Request-Id length [33] exceeds maximum allowed length [32]", exception.getMessage());
}

public void testSanitizeHeaderValueNullReturnedUnchanged() {
assertNull(RequestUtils.sanitizeHeaderValue(null));
}

public void testSanitizeHeaderValueEmptyReturnedUnchanged() {
assertEquals("", RequestUtils.sanitizeHeaderValue(""));
}

public void testSanitizeHeaderValueWellFormedUnchanged() {
assertEquals("a1b2c3d4e5f67890abcdef1234567890", RequestUtils.sanitizeHeaderValue("a1b2c3d4e5f67890abcdef1234567890"));
assertEquals("a1b2c3d4-e5f6-7890-abcd-ef1234567890", RequestUtils.sanitizeHeaderValue("a1b2c3d4-e5f6-7890-abcd-ef1234567890"));
assertEquals("my-trace-id-123", RequestUtils.sanitizeHeaderValue("my-trace-id-123"));
}

public void testSanitizeHeaderValueStripsControlCharacters() {
// Control characters (newline, carriage return, tab, NUL) are removed.
assertEquals("abcdef", RequestUtils.sanitizeHeaderValue("abc\r\ndef"));
assertEquals("tabbed", RequestUtils.sanitizeHeaderValue("tab\tbed"));
assertEquals("nul", RequestUtils.sanitizeHeaderValue("n\u0000ul"));
// Spaces (0x20) are not control characters and are preserved.
assertEquals("has space", RequestUtils.sanitizeHeaderValue("has space\n"));
}

public void testSanitizeHeaderValueControlCharactersOnlyBecomesEmpty() {
assertEquals("", RequestUtils.sanitizeHeaderValue("\n\r\t"));
}
}
Loading