Skip to content

Return bad request for invalid URI #2127

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
fool1280 merged 3 commits into from
May 18, 2026
+155 −57
Merged

Return bad request for invalid URI #2127

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
fcdc419
Harden path normalization in ClientRequestReceiver
fool1280 May 18, 2026
dbb5dcf
update
fool1280 May 18, 2026
6fa096c
change bad_uri to bad_uri_reason
fool1280 May 18, 2026
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
1 change: 1 addition & 0 deletions zuul-core/src/main/java/com/netflix/zuul/context/CommonContextKeys.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public class CommonContextKeys {
SessionContext.newKey("origin_status_category");
public static final SessionContext.Key<String> ORIGIN_STATUS_CATEGORY_REASON =
SessionContext.newKey("origin_status_category_reason");
public static final SessionContext.Key<String> BAD_URI_REASON = SessionContext.newKey("bad_uri_reason");
public static final SessionContext.Key<Integer> ORIGIN_STATUS = SessionContext.newKey("origin_status");
public static final SessionContext.Key<RequestAttempts> REQUEST_ATTEMPTS =
SessionContext.newKey("request_attempts");
Expand Down
108 changes: 56 additions & 52 deletions zuul-core/src/main/java/com/netflix/zuul/netty/server/ClientRequestReceiver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,9 @@
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.net.ssl.SSLException;
import lombok.NonNull;
import org.slf4j.Logger;
Expand All @@ -96,10 +95,6 @@ public class ClientRequestReceiver extends ChannelDuplexHandler {
private static final String SCHEME_HTTP = "http";
private static final String SCHEME_HTTPS = "https";

// via @stephenhay https://mathiasbynens.be/demo/url-regex, groups added
// group 1: scheme, group 2: domain, group 3: path+query
private static final Pattern URL_REGEX = Pattern.compile("^(https?)://([^\\s/$.?#].[^\\s/]*)([^\\s]*)$");

private final SessionContextDecorator decorator;

private HttpRequestMessage zuulRequest;
Expand Down Expand Up @@ -158,6 +153,27 @@ private void channelReadInternal(ChannelHandlerContext ctx, Object msg) {
RejectionUtils.rejectByClosingConnection(
ctx, ZuulStatusCategory.FAILURE_CLIENT_BAD_REQUEST, "decodefailure", clientRequest, null);
return;
} else if (zuulRequest.getContext().containsKey(CommonContextKeys.BAD_URI_REASON)) {
String clientIp = Objects.requireNonNullElse(getClientIp(ctx.channel()), "unknown");
String badUriReason = zuulRequest.getContext().get(CommonContextKeys.BAD_URI_REASON);
LOG.warn(
"Invalid URI in request. reason = {}, clientRequest = {}, clientIp = {}, info = {}",
badUriReason,
clientRequest,
clientIp,
ChannelUtils.channelInfoForLogging(ctx.channel()));
StatusCategoryUtils.setStatusCategory(
zuulRequest.getContext(), ZuulStatusCategory.FAILURE_CLIENT_BAD_REQUEST, badUriReason);
RejectionUtils.sendRejectionResponse(
ctx,
ZuulStatusCategory.FAILURE_CLIENT_BAD_REQUEST,
"invaliduri",
clientRequest,
null,
HttpResponseStatus.BAD_REQUEST,
badUriReason,
Map.of());
return;
} else if (zuulRequest.hasBody() && zuulRequest.getBodyLength() > zuulRequest.getMaxBodySize()) {
String errorMsg = "Request too large. "
+ "clientRequest = " + clientRequest.toString()
Expand Down Expand Up @@ -363,7 +379,13 @@ private HttpRequestMessage buildZuulHttpRequest(HttpRequest nativeRequest, Chann
}

// Strip off the query from the path.
String path = parsePath(nativeRequest.uri());
String path;
try {
path = parsePath(preProcessPath(nativeRequest.uri()));
} catch (URISyntaxException ex) {
path = nativeRequest.uri();
context.put(CommonContextKeys.BAD_URI_REASON, ex.getReason());
}

// Setup the req/resp message objects.
HttpRequestMessage request = new HttpRequestMessageImpl(
Expand Down Expand Up @@ -409,56 +431,38 @@ protected String getClientIp(Channel channel) {
return channel.attr(SourceAddressChannelHandler.ATTR_SOURCE_ADDRESS).get();
}

private String parsePath(String uri) {
String path;
try {
URI uriObject = new URI(uri);
uriObject = uriObject.normalize();
path = uriObject.getRawPath();
if (path == null) {
// If we have an opaque URI, match existing behavior of using the URI as the path.
return uri;
}
while (path.startsWith("/..")) {
path = path.substring(3);
}
return path;
} catch (URISyntaxException ex) {
LOG.debug("URI syntax error", ex);
}
// manual path parsing
// relative uri
if (uri.startsWith("/")) {
path = uri;
} else {
Matcher m = URL_REGEX.matcher(uri);

// absolute uri
if (m.matches()) {
String match = m.group(3);
if (match == null) {
// in case of no match, default to existing behavior
path = uri;
} else {
path = match;
}
}
// unknown value
else {
// in case of unknown value, default to existing behavior
path = uri;
}
}
protected String preProcessPath(String uri) {
return uri;
}

int queryIndex = path.indexOf('?');
private String parsePath(String uri) throws URISyntaxException {
int queryIndex = uri.indexOf('?');
if (queryIndex > -1) {
path = path.substring(0, queryIndex);
uri = uri.substring(0, queryIndex);
}

while (path.startsWith("/..")) {
path = path.substring(3);
URI uriObject = new URI(uri);
if (uriObject.isOpaque()) {
throw new URISyntaxException(uri, "opaque URI");
}
String rawPath = uriObject.getRawPath();
// Mirrors Envoy's REJECT_REQUEST
// https://github.com/envoyproxy/envoy/blob/main/source/extensions/http/header_validators/envoy_default/path_normalizer.cc#L91
if (containsEncodedSlash(rawPath)) {
throw new URISyntaxException(uri, "encoded slash in path");
}
return path;
// Fold %2E → '.' (unreserved per RFC 3986 §2.4) so normalize() can remove dot-segments.
String prepared = rawPath.replace("%2e", ".").replace("%2E", ".");
String normalized = new URI(prepared).normalize().getRawPath();
// RFC 3986 §5.2.4 discards leading "/..", but Java preserves it for relative paths.
while (normalized.startsWith("/..")) {
normalized = normalized.substring(3);
}
return normalized;
}

private static boolean containsEncodedSlash(String rawPath) {
return rawPath.contains("%2F") || rawPath.contains("%2f");
}

private static Headers copyHeaders(HttpRequest req) {
Expand Down
103 changes: 98 additions & 5 deletions zuul-core/src/test/java/com/netflix/zuul/netty/server/ClientRequestReceiverTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@
import org.checkerframework.checker.nullness.qual.NonNull;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.junit.jupiter.MockitoExtension;

/**
Expand Down Expand Up @@ -269,6 +271,62 @@ void maxHeaderSizeExceeded_setBadRequestStatus() {
.isEqualTo("Invalid request provided: Decode failure");
}

@Test
void invalidUri_setBadRequestStatus() {
ClientRequestReceiver receiver = new ClientRequestReceiver(null);
EmbeddedChannel channel = new EmbeddedChannel(new HttpRequestEncoder());
PassportLoggingHandler loggingHandler = new PassportLoggingHandler(new DefaultRegistry());

channel.attr(SourceAddressChannelHandler.ATTR_SERVER_LOCAL_PORT).set(1234);
channel.pipeline().addLast(new HttpServerCodec());
channel.pipeline().addLast(receiver);
channel.pipeline().addLast(loggingHandler);

HttpRequest httpRequest = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/{invalid}");

channel.writeOutbound(httpRequest);
ByteBuf byteBuf = channel.readOutbound();
channel.writeInbound(byteBuf);
channel.readInbound();
channel.close();

HttpRequestMessage request = ClientRequestReceiver.getRequestFromChannel(channel);
SessionContext context = request.getContext();
assertThat(context.get(CommonContextKeys.BAD_URI_REASON)).isNotNull();
assertThat(StatusCategoryUtils.getStatusCategory(context))
.isEqualTo(ZuulStatusCategory.FAILURE_CLIENT_BAD_REQUEST);
// Raw URI preserved for access logging, not replaced with a placeholder.
assertThat(request.getPath()).isEqualTo("/{invalid}");
}

@Test
void invalidPercentEncoding_setBadRequestStatus() {
// %GG is invalid (G is not a hex digit) — old fallback forwarded the un-normalized path to origin
ClientRequestReceiver receiver = new ClientRequestReceiver(null);
EmbeddedChannel channel = new EmbeddedChannel(new HttpRequestEncoder());
PassportLoggingHandler loggingHandler = new PassportLoggingHandler(new DefaultRegistry());

channel.attr(SourceAddressChannelHandler.ATTR_SERVER_LOCAL_PORT).set(1234);
channel.pipeline().addLast(new HttpServerCodec());
channel.pipeline().addLast(receiver);
channel.pipeline().addLast(loggingHandler);

HttpRequest httpRequest =
new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/api/%GG/../../actuator/env");

channel.writeOutbound(httpRequest);
ByteBuf byteBuf = channel.readOutbound();
channel.writeInbound(byteBuf);
channel.readInbound();
channel.close();

HttpRequestMessage request = ClientRequestReceiver.getRequestFromChannel(channel);
SessionContext context = request.getContext();
assertThat(context.get(CommonContextKeys.BAD_URI_REASON)).isNotNull();
assertThat(StatusCategoryUtils.getStatusCategory(context))
.isEqualTo(ZuulStatusCategory.FAILURE_CLIENT_BAD_REQUEST);
}

@Test
void multipleHostHeaders_setBadRequestStatus() {
ClientRequestReceiver receiver = new ClientRequestReceiver(null);
Expand Down Expand Up @@ -536,20 +594,55 @@ void pathTraversal_withQueryString() {
channel.close();
}

@Test
void pathTraversal_withOpaqueURI() {
@ParameterizedTest
@ValueSource(strings = {"/public/%2e%2e/admin/", "/public/%2E%2E/admin/"})
void pathTraversal_encodedDotDot(String uri) {
EmbeddedChannel channel = new EmbeddedChannel(new ClientRequestReceiver(null));
channel.attr(SourceAddressChannelHandler.ATTR_SERVER_LOCAL_PORT).set(1234);
HttpRequestMessageImpl result;
{
channel.writeInbound(new DefaultFullHttpRequest(
HttpVersion.HTTP_1_1, HttpMethod.GET, "foo.netflix.net:443", Unpooled.buffer()));
channel.writeInbound(
new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, uri, Unpooled.buffer()));
result = channel.readInbound();
result.disposeBufferedBody();
}

assertThat(result.getPath()).isEqualTo("foo.netflix.net:443");
assertThat(result.getPath()).isEqualTo("/admin/");
channel.close();
}

@ParameterizedTest
@ValueSource(strings = {"/public/%2F../secret", "/public/%2f../secret"})
void encodedSlash_rejected(String uri) {
EmbeddedChannel channel = new EmbeddedChannel(new ClientRequestReceiver(null));
channel.attr(SourceAddressChannelHandler.ATTR_SERVER_LOCAL_PORT).set(1234);
channel.writeInbound(new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, uri, Unpooled.buffer()));
channel.readInbound();
channel.close();

SessionContext context =
ClientRequestReceiver.getRequestFromChannel(channel).getContext();
assertThat(context.get(CommonContextKeys.BAD_URI_REASON)).isNotNull();
assertThat(StatusCategoryUtils.getStatusCategory(context))
.isEqualTo(ZuulStatusCategory.FAILURE_CLIENT_BAD_REQUEST);
assertThat(StatusCategoryUtils.getStatusCategoryReason(context)).isEqualTo("encoded slash in path");
}

@Test
void opaqueUri_rejected() {
EmbeddedChannel channel = new EmbeddedChannel(new ClientRequestReceiver(null));
channel.attr(SourceAddressChannelHandler.ATTR_SERVER_LOCAL_PORT).set(1234);
channel.writeInbound(new DefaultFullHttpRequest(
HttpVersion.HTTP_1_1, HttpMethod.GET, "foo.netflix.net:443", Unpooled.buffer()));
channel.readInbound();
channel.close();

SessionContext context =
ClientRequestReceiver.getRequestFromChannel(channel).getContext();
assertThat(context.get(CommonContextKeys.BAD_URI_REASON)).isNotNull();
assertThat(StatusCategoryUtils.getStatusCategory(context))
.isEqualTo(ZuulStatusCategory.FAILURE_CLIENT_BAD_REQUEST);
assertThat(StatusCategoryUtils.getStatusCategoryReason(context)).isEqualTo("opaque URI");
}

@Test
Expand Down