diff --git a/benchmarks/jmh/src/jmh/java/com/linecorp/armeria/internal/common/PathParsingBenchmark.java b/benchmarks/jmh/src/jmh/java/com/linecorp/armeria/internal/common/PathParsingBenchmark.java index afb24c76562e..01e745192941 100644 --- a/benchmarks/jmh/src/jmh/java/com/linecorp/armeria/internal/common/PathParsingBenchmark.java +++ b/benchmarks/jmh/src/jmh/java/com/linecorp/armeria/internal/common/PathParsingBenchmark.java @@ -24,8 +24,10 @@ import org.openjdk.jmh.annotations.State; import org.openjdk.jmh.infra.Blackhole; +import com.linecorp.armeria.common.RequestTarget; + /** - * Microbenchmarks for the {@link PathAndQuery#parse(String)} method. + * Microbenchmarks for the {@link RequestTarget#forServer(String)} method. */ @State(Scope.Thread) public class PathParsingBenchmark { @@ -42,37 +44,37 @@ public void setUp() { } @Benchmark - public PathAndQuery normal() { + public RequestTarget normal() { return doNormal(); } @Benchmark @Fork(jvmArgsAppend = "-Dcom.linecorp.armeria.parsedPathCache=off") - public PathAndQuery normal_cacheDisabled() { + public RequestTarget normal_cacheDisabled() { return doNormal(); } - private PathAndQuery doNormal() { - final PathAndQuery parsed = PathAndQuery.parse(path1); - parsed.storeInCache(path1); + private RequestTarget doNormal() { + final RequestTarget parsed = RequestTarget.forServer(path1); + RequestTargetCache.putForServer(path1, parsed); return parsed; } @Benchmark - public PathAndQuery cachedAndNotCached(Blackhole bh) { + public RequestTarget cachedAndNotCached(Blackhole bh) { return doCachedAndNotCached(bh); } @Benchmark @Fork(jvmArgsAppend = "-Dcom.linecorp.armeria.parsedPathCache=off") - public PathAndQuery cachedAndNotCached_cacheDisabled(Blackhole bh) { + public RequestTarget cachedAndNotCached_cacheDisabled(Blackhole bh) { return doCachedAndNotCached(bh); } - private PathAndQuery doCachedAndNotCached(Blackhole bh) { - final PathAndQuery parsed = PathAndQuery.parse(path1); - parsed.storeInCache(path1); - final PathAndQuery parsed2 = PathAndQuery.parse(path2); + private RequestTarget doCachedAndNotCached(Blackhole bh) { + final RequestTarget parsed = RequestTarget.forServer(path1); + RequestTargetCache.putForServer(path1, parsed); + final RequestTarget parsed2 = RequestTarget.forServer(path2); bh.consume(parsed2); return parsed; } diff --git a/benchmarks/jmh/src/jmh/java/com/linecorp/armeria/server/RoutersBenchmark.java b/benchmarks/jmh/src/jmh/java/com/linecorp/armeria/server/RoutersBenchmark.java index 85a6fd2b5a8d..abfc4dd05cd5 100644 --- a/benchmarks/jmh/src/jmh/java/com/linecorp/armeria/server/RoutersBenchmark.java +++ b/benchmarks/jmh/src/jmh/java/com/linecorp/armeria/server/RoutersBenchmark.java @@ -32,6 +32,7 @@ import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.RequestHeaders; import com.linecorp.armeria.common.RequestId; +import com.linecorp.armeria.common.RequestTarget; import com.linecorp.armeria.common.SuccessFunction; import com.linecorp.armeria.server.logging.AccessLogWriter; @@ -48,6 +49,9 @@ public class RoutersBenchmark { private static final RequestHeaders METHOD1_HEADERS = RequestHeaders.of(HttpMethod.POST, "/grpc.package.Service/Method1"); + private static final RequestTarget METHOD1_REQ_TARGET = + RequestTarget.forServer(METHOD1_HEADERS.path()); + static { final String defaultLogName = null; final String defaultServiceName = null; @@ -60,32 +64,32 @@ public class RoutersBenchmark { SERVICE, defaultLogName, defaultServiceName, defaultServiceNaming, 0, 0, false, AccessLogWriter.disabled(), CommonPools.blockingTaskExecutor(), SuccessFunction.always(), multipartUploadsLocation, ImmutableList.of(), - HttpHeaders.of(), (ctx) -> RequestId.random()), + HttpHeaders.of(), ctx -> RequestId.random()), new ServiceConfig(route2, route2, SERVICE, defaultLogName, defaultServiceName, defaultServiceNaming, 0, 0, false, AccessLogWriter.disabled(), CommonPools.blockingTaskExecutor(), SuccessFunction.always(), multipartUploadsLocation, ImmutableList.of(), - HttpHeaders.of(), (ctx) -> RequestId.random()) + HttpHeaders.of(), ctx -> RequestId.random()) ); FALLBACK_SERVICE = new ServiceConfig(Route.ofCatchAll(), Route.ofCatchAll(), SERVICE, defaultLogName, defaultServiceName, defaultServiceNaming, 0, 0, false, AccessLogWriter.disabled(), CommonPools.blockingTaskExecutor(), SuccessFunction.always(), multipartUploadsLocation, - ImmutableList.of(), HttpHeaders.of(), (ctx) -> RequestId.random()); + ImmutableList.of(), HttpHeaders.of(), ctx -> RequestId.random()); HOST = new VirtualHost( "localhost", "localhost", 0, null, SERVICES, FALLBACK_SERVICE, RejectedRouteHandler.DISABLED, unused -> NOPLogger.NOP_LOGGER, defaultServiceNaming, 0, 0, false, AccessLogWriter.disabled(), CommonPools.blockingTaskExecutor(), multipartUploadsLocation, ImmutableList.of(), - (ctx) -> RequestId.random()); + ctx -> RequestId.random()); ROUTER = Routers.ofVirtualHost(HOST, SERVICES, RejectedRouteHandler.DISABLED); } @Benchmark public Routed exactMatch() { - final RoutingContext ctx = DefaultRoutingContext.of(HOST, "localhost", METHOD1_HEADERS.path(), - null, METHOD1_HEADERS, RoutingStatus.OK); + final RoutingContext ctx = DefaultRoutingContext.of(HOST, "localhost", METHOD1_REQ_TARGET, + METHOD1_HEADERS, RoutingStatus.OK); final Routed routed = ROUTER.find(ctx); if (routed.value() != SERVICES.get(0)) { throw new IllegalStateException("Routing error"); @@ -96,8 +100,8 @@ public Routed exactMatch() { @Benchmark public Routed exactMatch_wrapped() { final RoutingContext ctx = new RoutingContextWrapper( - DefaultRoutingContext.of(HOST, "localhost", METHOD1_HEADERS.path(), - null, METHOD1_HEADERS, RoutingStatus.OK)); + DefaultRoutingContext.of(HOST, "localhost", METHOD1_REQ_TARGET, + METHOD1_HEADERS, RoutingStatus.OK)); final Routed routed = ROUTER.find(ctx); if (routed.value() != SERVICES.get(0)) { throw new IllegalStateException("Routing error"); diff --git a/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java b/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java index 0ded24cac8e4..d8ffe00b81c6 100644 --- a/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java @@ -70,8 +70,6 @@ public void run(Throwable cause) { /* no-op */ } noopResponseCancellationScheduler.finishNow(); } - @Nullable - private final String fragment; @Nullable private Endpoint endpoint; private ClientOptions options = ClientOptions.of(); @@ -81,12 +79,10 @@ public void run(Throwable cause) { /* no-op */ } ClientRequestContextBuilder(HttpRequest request) { super(false, request); - fragment = null; } ClientRequestContextBuilder(RpcRequest request, URI uri) { super(false, request, uri); - fragment = uri.getRawFragment(); } @Override @@ -157,7 +153,7 @@ public ClientRequestContext build() { final DefaultClientRequestContext ctx = new DefaultClientRequestContext( eventLoop(), meterRegistry(), sessionProtocol(), - id(), method(), path(), query(), fragment, options, request(), rpcRequest(), + id(), method(), requestTarget(), options, request(), rpcRequest(), requestOptions, responseCancellationScheduler, isRequestStartTimeSet() ? requestStartTimeNanos() : System.nanoTime(), isRequestStartTimeSet() ? requestStartTimeMicros() : SystemInfo.currentTimeMicros()); diff --git a/core/src/main/java/com/linecorp/armeria/client/DefaultWebClient.java b/core/src/main/java/com/linecorp/armeria/client/DefaultWebClient.java index 767a07370421..2fc839f535f9 100644 --- a/core/src/main/java/com/linecorp/armeria/client/DefaultWebClient.java +++ b/core/src/main/java/com/linecorp/armeria/client/DefaultWebClient.java @@ -16,21 +16,16 @@ package com.linecorp.armeria.client; -import static com.linecorp.armeria.internal.client.ClientUtil.pathWithQuery; -import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.concatPaths; -import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.isAbsoluteUri; import static java.util.Objects.requireNonNull; -import java.net.URI; - -import com.linecorp.armeria.client.endpoint.EndpointGroup; import com.linecorp.armeria.common.ExchangeType; import com.linecorp.armeria.common.HttpRequest; import com.linecorp.armeria.common.HttpResponse; +import com.linecorp.armeria.common.RequestTarget; +import com.linecorp.armeria.common.RequestTargetForm; import com.linecorp.armeria.common.Scheme; import com.linecorp.armeria.common.SessionProtocol; import com.linecorp.armeria.common.annotation.Nullable; -import com.linecorp.armeria.internal.common.PathAndQuery; import io.micrometer.core.instrument.MeterRegistry; @@ -58,69 +53,73 @@ public HttpResponse execute(HttpRequest req, RequestOptions requestOptions) { requireNonNull(req, "req"); requireNonNull(requestOptions, "requestOptions"); + final String originalPath = req.path(); + final RequestTarget reqTarget = RequestTarget.forClient(originalPath, uri().getRawPath()); + if (reqTarget == null) { + return abortRequestAndReturnFailureResponse( + req, new IllegalArgumentException("Invalid path: " + originalPath)); + } + if (Clients.isUndefinedUri(uri())) { - final URI uri; - if (isAbsoluteUri(req.path())) { - try { - uri = URI.create(req.path()); - } catch (Exception ex) { + final String scheme; + final String authority; + if (reqTarget.form() == RequestTargetForm.ABSOLUTE) { + scheme = reqTarget.scheme(); + authority = reqTarget.authority(); + assert scheme != null; + assert authority != null; + } else { + scheme = req.scheme(); + authority = req.authority(); + + if (scheme == null || authority == null) { return abortRequestAndReturnFailureResponse(req, new IllegalArgumentException( - "Failed to create a URI: " + req.path(), ex)); + "Scheme and authority must be specified in \":path\" or " + + "in \":scheme\" and \":authority\". :path=" + + originalPath + ", :scheme=" + req.scheme() + ", :authority=" + req.authority())); } - } else if (req.scheme() != null && req.authority() != null) { - uri = req.uri(); - } else { - return abortRequestAndReturnFailureResponse(req, new IllegalArgumentException( - "Scheme and authority must be specified in \":path\" or " + - "in \":scheme\" and \":authority\". :path=" + - req.path() + ", :scheme=" + req.scheme() + ", :authority=" + req.authority())); } + final SessionProtocol protocol; try { - protocol = Scheme.parse(uri.getScheme()).sessionProtocol(); + protocol = Scheme.parse(scheme).sessionProtocol(); } catch (Exception e) { return abortRequestAndReturnFailureResponse(req, new IllegalArgumentException( - "Failed to parse a scheme: " + uri.getScheme(), e)); + "Failed to parse a scheme: " + reqTarget.scheme(), e)); } - final Endpoint endpoint = Endpoint.parse(uri.getAuthority()); - final String query = uri.getRawQuery(); - final String path = pathWithQuery(uri, query); - final HttpRequest newReq = req.withHeaders(req.headers().toBuilder().path(path)); - return execute(endpoint, newReq, protocol, requestOptions); + final Endpoint endpoint = Endpoint.parse(authority); + final HttpRequest newReq = req.withHeaders(req.headers() + .toBuilder() + .path(reqTarget.pathAndQuery())); + return execute(protocol, + endpoint, + newReq.method(), + reqTarget, + newReq, + requestOptions); } - if (isAbsoluteUri(req.path())) { + if (reqTarget.form() == RequestTargetForm.ABSOLUTE) { return abortRequestAndReturnFailureResponse(req, new IllegalArgumentException( "Cannot send a request with a \":path\" header that contains a URI with the authority, " + - "because the client was created with a base URI. path: " + req.path())); + "because the client was created with a base URI. path: " + originalPath)); } - final String originalPath = req.path(); - final String newPath = concatPaths(uri().getRawPath(), originalPath); + final String newPath = reqTarget.pathAndQuery(); final HttpRequest newReq; - // newPath and originalPath should be the same reference if uri().getRawPath() can be ignorable - if (newPath != originalPath) { - newReq = req.withHeaders(req.headers().toBuilder().path(newPath)); - } else { + if (newPath.equals(originalPath)) { newReq = req; + } else { + newReq = req.withHeaders(req.headers().toBuilder().path(newPath)); } - return execute(endpointGroup(), newReq, scheme().sessionProtocol(), requestOptions); - } - private HttpResponse execute(EndpointGroup endpointGroup, HttpRequest req, SessionProtocol protocol, - RequestOptions requestOptions) { - final PathAndQuery pathAndQuery = PathAndQuery.parse(req.path()); - if (pathAndQuery == null) { - final IllegalArgumentException cause = new IllegalArgumentException("invalid path: " + req.path()); - return abortRequestAndReturnFailureResponse(req, cause); - } - final String newPath = pathAndQuery.toString(); - if (!newPath.equals(req.path())) { - req = req.withHeaders(req.headers().toBuilder().path(newPath)); - } - return execute(protocol, endpointGroup, req.method(), - pathAndQuery.path(), pathAndQuery.query(), null, req, requestOptions); + return execute(scheme().sessionProtocol(), + endpointGroup(), + newReq.method(), + reqTarget, + newReq, + requestOptions); } private static HttpResponse abortRequestAndReturnFailureResponse( diff --git a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java index 48b51d65cc84..969856c45c99 100644 --- a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java +++ b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java @@ -28,6 +28,7 @@ import com.linecorp.armeria.client.proxy.ProxyType; import com.linecorp.armeria.common.HttpRequest; import com.linecorp.armeria.common.HttpResponse; +import com.linecorp.armeria.common.RequestTarget; import com.linecorp.armeria.common.SessionProtocol; import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.common.logging.ClientConnectionTimings; @@ -38,7 +39,6 @@ import com.linecorp.armeria.internal.client.DecodedHttpResponse; import com.linecorp.armeria.internal.client.HttpSession; import com.linecorp.armeria.internal.client.PooledChannel; -import com.linecorp.armeria.internal.common.PathAndQuery; import com.linecorp.armeria.internal.common.RequestContextUtil; import com.linecorp.armeria.server.ProxiedAddresses; import com.linecorp.armeria.server.ServiceRequestContext; @@ -222,7 +222,7 @@ private static void logSession(ClientRequestContext ctx, @Nullable PooledChannel } private static boolean isValidPath(HttpRequest req) { - return PathAndQuery.parse(req.path()) != null; + return RequestTarget.forClient(req.path()) != null; } private static HttpResponse earlyFailedResponse(Throwable t, ClientRequestContext ctx, HttpRequest req) { diff --git a/core/src/main/java/com/linecorp/armeria/client/HttpClientFactory.java b/core/src/main/java/com/linecorp/armeria/client/HttpClientFactory.java index f124b0d0e931..64e7ed4c16da 100644 --- a/core/src/main/java/com/linecorp/armeria/client/HttpClientFactory.java +++ b/core/src/main/java/com/linecorp/armeria/client/HttpClientFactory.java @@ -50,6 +50,7 @@ import com.linecorp.armeria.common.util.ReleasableHolder; import com.linecorp.armeria.common.util.ShutdownHooks; import com.linecorp.armeria.common.util.TransportType; +import com.linecorp.armeria.internal.common.RequestTargetCache; import com.linecorp.armeria.internal.common.util.SslContextUtil; import io.micrometer.core.instrument.MeterRegistry; @@ -166,6 +167,7 @@ final class HttpClientFactory implements ClientFactory { this.options = options; clientDelegate = new HttpClientDelegate(this, addressResolverGroup); + RequestTargetCache.registerClientMetrics(meterRegistry); } /** diff --git a/core/src/main/java/com/linecorp/armeria/client/UserClient.java b/core/src/main/java/com/linecorp/armeria/client/UserClient.java index dac77d9921c1..dc87b2868496 100644 --- a/core/src/main/java/com/linecorp/armeria/client/UserClient.java +++ b/core/src/main/java/com/linecorp/armeria/client/UserClient.java @@ -32,12 +32,12 @@ import com.linecorp.armeria.common.HttpResponse; import com.linecorp.armeria.common.Request; import com.linecorp.armeria.common.RequestId; +import com.linecorp.armeria.common.RequestTarget; import com.linecorp.armeria.common.Response; import com.linecorp.armeria.common.RpcRequest; import com.linecorp.armeria.common.RpcResponse; import com.linecorp.armeria.common.Scheme; import com.linecorp.armeria.common.SessionProtocol; -import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.common.util.AbstractUnwrappable; import com.linecorp.armeria.common.util.SystemInfo; import com.linecorp.armeria.internal.client.DefaultClientRequestContext; @@ -123,14 +123,11 @@ public final ClientOptions options() { * * @param protocol the {@link SessionProtocol} to use * @param method the method of the {@link Request} - * @param path the path part of the {@link Request} URI - * @param query the query part of the {@link Request} URI - * @param fragment the fragment part of the {@link Request} URI + * @param reqTarget the {@link RequestTarget} of the {@link Request} * @param req the {@link Request} */ - protected final O execute(SessionProtocol protocol, HttpMethod method, String path, - @Nullable String query, @Nullable String fragment, I req) { - return execute(protocol, method, path, query, fragment, req, RequestOptions.of()); + protected final O execute(SessionProtocol protocol, HttpMethod method, RequestTarget reqTarget, I req) { + return execute(protocol, method, reqTarget, req, RequestOptions.of()); } /** @@ -138,16 +135,13 @@ protected final O execute(SessionProtocol protocol, HttpMethod method, String pa * * @param protocol the {@link SessionProtocol} to use * @param method the method of the {@link Request} - * @param path the path part of the {@link Request} URI - * @param query the query part of the {@link Request} URI - * @param fragment the fragment part of the {@link Request} URI + * @param reqTarget the {@link RequestTarget} of the {@link Request} * @param req the {@link Request} * @param requestOptions the {@link RequestOptions} of the {@link Request} */ - protected final O execute(SessionProtocol protocol, HttpMethod method, String path, - @Nullable String query, @Nullable String fragment, I req, - RequestOptions requestOptions) { - return execute(protocol, endpointGroup(), method, path, query, fragment, req, requestOptions); + protected final O execute(SessionProtocol protocol, HttpMethod method, RequestTarget reqTarget, + I req, RequestOptions requestOptions) { + return execute(protocol, endpointGroup(), method, reqTarget, req, requestOptions); } /** @@ -156,14 +150,12 @@ protected final O execute(SessionProtocol protocol, HttpMethod method, String pa * @param protocol the {@link SessionProtocol} to use * @param endpointGroup the {@link EndpointGroup} of the {@link Request} * @param method the method of the {@link Request} - * @param path the path part of the {@link Request} URI - * @param query the query part of the {@link Request} URI - * @param fragment the fragment part of the {@link Request} URI + * @param reqTarget the {@link RequestTarget} of the {@link Request} * @param req the {@link Request} */ protected final O execute(SessionProtocol protocol, EndpointGroup endpointGroup, HttpMethod method, - String path, @Nullable String query, @Nullable String fragment, I req) { - return execute(protocol, endpointGroup, method, path, query, fragment, req, RequestOptions.of()); + RequestTarget reqTarget, I req) { + return execute(protocol, endpointGroup, method, reqTarget, req, RequestOptions.of()); } /** @@ -172,15 +164,12 @@ protected final O execute(SessionProtocol protocol, EndpointGroup endpointGroup, * @param protocol the {@link SessionProtocol} to use * @param endpointGroup the {@link EndpointGroup} of the {@link Request} * @param method the method of the {@link Request} - * @param path the path part of the {@link Request} URI - * @param query the query part of the {@link Request} URI - * @param fragment the fragment part of the {@link Request} URI + * @param reqTarget the {@link RequestTarget} of the {@link Request} * @param req the {@link Request} * @param requestOptions the {@link RequestOptions} of the {@link Request} */ protected final O execute(SessionProtocol protocol, EndpointGroup endpointGroup, HttpMethod method, - String path, @Nullable String query, @Nullable String fragment, I req, - RequestOptions requestOptions) { + RequestTarget reqTarget, I req, RequestOptions requestOptions) { final HttpRequest httpReq; final RpcRequest rpcReq; @@ -195,7 +184,7 @@ protected final O execute(SessionProtocol protocol, EndpointGroup endpointGroup, } final DefaultClientRequestContext ctx = new DefaultClientRequestContext( - meterRegistry, protocol, id, method, path, query, fragment, options(), httpReq, rpcReq, + meterRegistry, protocol, id, method, reqTarget, options(), httpReq, rpcReq, requestOptions, System.nanoTime(), SystemInfo.currentTimeMicros()); return initContextAndExecuteWithFallback(unwrap(), ctx, endpointGroup, diff --git a/core/src/main/java/com/linecorp/armeria/common/AbstractRequestContextBuilder.java b/core/src/main/java/com/linecorp/armeria/common/AbstractRequestContextBuilder.java index 1286ed606055..550ea84d6184 100644 --- a/core/src/main/java/com/linecorp/armeria/common/AbstractRequestContextBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/AbstractRequestContextBuilder.java @@ -31,7 +31,6 @@ import com.linecorp.armeria.client.ClientRequestContextBuilder; import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.common.metric.NoopMeterRegistry; -import com.linecorp.armeria.internal.common.PathAndQuery; import com.linecorp.armeria.server.Service; import com.linecorp.armeria.server.ServiceRequestContextBuilder; @@ -71,9 +70,7 @@ public abstract class AbstractRequestContextBuilder { private RequestId id; private HttpMethod method; private final String authority; - private final String path; - @Nullable - private final String query; + private final RequestTarget reqTarget; private MeterRegistry meterRegistry = NoopMeterRegistry.get(); @Nullable @@ -99,19 +96,30 @@ public abstract class AbstractRequestContextBuilder { * @param req the {@link HttpRequest}. */ protected AbstractRequestContextBuilder(boolean server, HttpRequest req) { + requireNonNull(req, "req"); this.server = server; - this.req = requireNonNull(req, "req"); rpcReq = null; sessionProtocol = SessionProtocol.H2C; method = req.headers().method(); authority = firstNonNull(req.headers().authority(), FALLBACK_AUTHORITY); - final String pathAndQueryStr = req.headers().path(); - final PathAndQuery pathAndQuery = PathAndQuery.parse(pathAndQueryStr); - checkArgument(pathAndQuery != null, "request.path is not valid: %s", req); - path = pathAndQuery.path(); - query = pathAndQuery.query(); + final String rawPath = req.headers().path(); + final RequestTarget reqTarget = parseRawPath(server, rawPath); + checkArgument(reqTarget != null, "request.path is not valid: %s", req); + checkArgument(reqTarget.form() != RequestTargetForm.ABSOLUTE, + "request.path must not contain scheme or authority: %s", req); + + final String newRawPath = reqTarget.pathAndQuery(); + if (newRawPath.equals(rawPath)) { + this.req = req; + } else { + this.req = req.withHeaders(req.headers() + .toBuilder() + .path(newRawPath)); + } + + this.reqTarget = reqTarget; } /** @@ -131,15 +139,44 @@ protected AbstractRequestContextBuilder(boolean server, RpcRequest rpcReq, URI u authority = firstNonNull(uri.getRawAuthority(), FALLBACK_AUTHORITY); sessionProtocol = getSessionProtocol(uri); - final PathAndQuery pathAndQuery; - if (uri.getRawQuery() != null) { - pathAndQuery = PathAndQuery.parse(uri.getRawPath() + '?' + uri.getRawQuery()); + // TODO(trustin): Add a way to construct a RequestTarget without normalization + // so we don't have to reconstruct and normalize again. + final RequestTarget reqTarget; + if (server) { + if (uri.getRawQuery() != null) { + reqTarget = RequestTarget.forServer(uri.getRawPath() + + '?' + uri.getRawQuery()); + } else { + reqTarget = RequestTarget.forServer(uri.getRawPath()); + } } else { - pathAndQuery = PathAndQuery.parse(uri.getRawPath()); + if (uri.getRawQuery() != null) { + if (uri.getRawFragment() != null) { + reqTarget = RequestTarget.forClient(uri.getRawPath() + + '?' + uri.getRawQuery() + + '#' + uri.getRawFragment()); + } else { + reqTarget = RequestTarget.forClient(uri.getRawPath() + + '?' + uri.getRawQuery()); + } + } else { + if (uri.getRawFragment() != null) { + reqTarget = RequestTarget.forClient(uri.getRawPath() + + '#' + uri.getRawFragment()); + } else { + reqTarget = RequestTarget.forClient(uri.getRawPath()); + } + } } - checkArgument(pathAndQuery != null, "uri.path or uri.query is not valid: %s", uri); - path = pathAndQuery.path(); - query = pathAndQuery.query(); + + checkArgument(reqTarget != null, "uri.path, uri.query or uri.fragment is not valid: %s", uri); + this.reqTarget = reqTarget; + } + + @Nullable + private static RequestTarget parseRawPath(boolean server, String rawPath) { + return server ? RequestTarget.forServer(rawPath) + : RequestTarget.forClient(rawPath); } private static SessionProtocol getSessionProtocol(URI uri) { @@ -443,10 +480,10 @@ protected final String authority() { } /** - * Returns the path of the request, excluding the query part. + * Returns the {@link RequestTarget}. */ - protected final String path() { - return path; + protected final RequestTarget requestTarget() { + return reqTarget; } /** @@ -468,16 +505,6 @@ protected final RequestId id() { return id; } - /** - * Returns the query part of the request, excluding the leading question mark ({@code '?'}). - * - * @return the query string, or {@code null} if there is no query. - */ - @Nullable - protected final String query() { - return query; - } - /** * Returns a fake {@link Channel} which is required internally when creating a context. */ diff --git a/core/src/main/java/com/linecorp/armeria/common/RequestTarget.java b/core/src/main/java/com/linecorp/armeria/common/RequestTarget.java new file mode 100644 index 000000000000..cbdcce693b7c --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/common/RequestTarget.java @@ -0,0 +1,119 @@ +/* + * Copyright 2023 LINE Corporation + * + * LINE Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package com.linecorp.armeria.common; + +import static java.util.Objects.requireNonNull; + +import com.linecorp.armeria.common.annotation.Nullable; +import com.linecorp.armeria.common.annotation.UnstableApi; +import com.linecorp.armeria.internal.common.DefaultRequestTarget; +import com.linecorp.armeria.internal.common.util.TemporaryThreadLocals; + +/** + * A parser of the raw path and query components of an HTTP path. Performs validation and allows caching of + * results. + */ +@UnstableApi +public interface RequestTarget { + + /** + * Validates the {@link String} that contains a server-side absolute path and a query, and splits them into + * the path part and the query part. + * + * @return a {@link RequestTarget} with the absolute path and query, or {@code null} if the specified + * {@link String} is not an absolute path or invalid. + */ + @Nullable + static RequestTarget forServer(String reqTarget) { + requireNonNull(reqTarget, "reqTarget"); + return DefaultRequestTarget.forServer(reqTarget, Flags.allowDoubleDotsInQueryString()); + } + + /** + * Validates the {@link String} that contains a client-side absolute path, a query and a fragment, and + * splits them into the path, query and fragment part. + * + * @return a {@link RequestTarget} with the absolute path, query and fragment, or {@code null} if the + * specified {@link String} is not an absolute path or invalid. + */ + @Nullable + static RequestTarget forClient(String reqTarget) { + return forClient(reqTarget, null); + } + + /** + * Validates the {@link String} that contains a client-side absolute path, a query and a fragment, and + * splits them into the path, query and fragment part. + * + * @return a {@link RequestTarget} with the absolute path, query and fragment, or {@code null} if the + * specified {@link String} is not an absolute path or invalid. + */ + @Nullable + static RequestTarget forClient(String reqTarget, @Nullable String prefix) { + return DefaultRequestTarget.forClient(reqTarget, prefix); + } + + /** + * TBW. + */ + RequestTargetForm form(); + + /** + * TBW. + */ + @Nullable + String scheme(); + + /** + * TBW. + */ + @Nullable + String authority(); + + /** + * TBW. + */ + String path(); + + /** + * TBW. + */ + @Nullable + String query(); + + /** + * TBW. + */ + default String pathAndQuery() { + if (query() == null) { + return path(); + } + + try (TemporaryThreadLocals tmp = TemporaryThreadLocals.acquire()) { + return tmp.stringBuilder() + .append(path()) + .append('?') + .append(query()) + .toString(); + } + } + + /** + * TBW. + */ + @Nullable + String fragment(); +} diff --git a/core/src/main/java/com/linecorp/armeria/common/RequestTargetForm.java b/core/src/main/java/com/linecorp/armeria/common/RequestTargetForm.java new file mode 100644 index 000000000000..827f0f65b577 --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/common/RequestTargetForm.java @@ -0,0 +1,37 @@ +/* + * Copyright 2023 LINE Corporation + * + * LINE Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package com.linecorp.armeria.common; + +import com.linecorp.armeria.common.annotation.UnstableApi; + +/** + * TBW. + */ +@UnstableApi +public enum RequestTargetForm { + /** + * TBW. + */ + ORIGIN, + /** + * TBW. + */ + ABSOLUTE, + /** + * TBW. + */ + ASTERISK +} diff --git a/core/src/main/java/com/linecorp/armeria/internal/client/ClientUtil.java b/core/src/main/java/com/linecorp/armeria/internal/client/ClientUtil.java index f9dd9156e0c8..661fd471326d 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/client/ClientUtil.java +++ b/core/src/main/java/com/linecorp/armeria/internal/client/ClientUtil.java @@ -18,13 +18,10 @@ import static com.google.common.base.MoreObjects.firstNonNull; import static java.util.Objects.requireNonNull; -import java.net.URI; import java.util.concurrent.CompletableFuture; import java.util.function.BiFunction; import java.util.function.Function; -import com.google.common.base.Strings; - import com.linecorp.armeria.client.Client; import com.linecorp.armeria.client.ClientRequestContext; import com.linecorp.armeria.client.Endpoint; @@ -231,18 +228,5 @@ public static ClientRequestContext newDerivedContext(ClientRequestContext ctx, return derived; } - public static String pathWithQuery(URI uri, @Nullable String query) { - return pathWithQuery(uri.getRawPath(), query); - } - - public static String pathWithQuery(String path, @Nullable String query) { - if (Strings.isNullOrEmpty(path)) { - path = query == null ? "/" : "/?" + query; - } else if (query != null) { - path = path + '?' + query; - } - return path; - } - private ClientUtil() {} } diff --git a/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java b/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java index e7c6e5b9ef70..f461ae6bc3b7 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java +++ b/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java @@ -18,8 +18,6 @@ import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; -import static com.linecorp.armeria.internal.client.ClientUtil.pathWithQuery; -import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.isAbsoluteUri; import static com.linecorp.armeria.internal.common.HttpHeadersUtil.getScheme; import static java.util.Objects.requireNonNull; @@ -57,6 +55,8 @@ import com.linecorp.armeria.common.RequestContext; import com.linecorp.armeria.common.RequestHeaders; import com.linecorp.armeria.common.RequestId; +import com.linecorp.armeria.common.RequestTarget; +import com.linecorp.armeria.common.RequestTargetForm; import com.linecorp.armeria.common.Response; import com.linecorp.armeria.common.RpcRequest; import com.linecorp.armeria.common.Scheme; @@ -72,7 +72,6 @@ import com.linecorp.armeria.common.util.UnmodifiableFuture; import com.linecorp.armeria.internal.common.CancellationScheduler; import com.linecorp.armeria.internal.common.NonWrappingRequestContext; -import com.linecorp.armeria.internal.common.PathAndQuery; import com.linecorp.armeria.internal.common.RequestContextExtension; import com.linecorp.armeria.internal.common.util.TemporaryThreadLocals; import com.linecorp.armeria.server.ServiceRequestContext; @@ -115,8 +114,6 @@ public final class DefaultClientRequestContext @Nullable private ContextAwareEventLoop contextAwareEventLoop; @Nullable - private final String fragment; - @Nullable private final ServiceRequestContext root; private final ClientOptions options; @@ -161,12 +158,12 @@ public final class DefaultClientRequestContext */ public DefaultClientRequestContext( EventLoop eventLoop, MeterRegistry meterRegistry, SessionProtocol sessionProtocol, - RequestId id, HttpMethod method, String path, @Nullable String query, @Nullable String fragment, + RequestId id, HttpMethod method, RequestTarget reqTarget, ClientOptions options, @Nullable HttpRequest req, @Nullable RpcRequest rpcReq, RequestOptions requestOptions, CancellationScheduler responseCancellationScheduler, long requestStartTimeNanos, long requestStartTimeMicros) { this(eventLoop, meterRegistry, sessionProtocol, - id, method, path, query, fragment, options, req, rpcReq, requestOptions, serviceRequestContext(), + id, method, reqTarget, options, req, rpcReq, requestOptions, serviceRequestContext(), responseCancellationScheduler, requestStartTimeNanos, requestStartTimeMicros); } @@ -185,30 +182,29 @@ id, method, path, query, fragment, options, req, rpcReq, requestOptions, service */ public DefaultClientRequestContext( MeterRegistry meterRegistry, SessionProtocol sessionProtocol, - RequestId id, HttpMethod method, String path, @Nullable String query, @Nullable String fragment, + RequestId id, HttpMethod method, RequestTarget reqTarget, ClientOptions options, @Nullable HttpRequest req, @Nullable RpcRequest rpcReq, RequestOptions requestOptions, long requestStartTimeNanos, long requestStartTimeMicros) { this(null, meterRegistry, sessionProtocol, - id, method, path, query, fragment, options, req, rpcReq, requestOptions, + id, method, reqTarget, options, req, rpcReq, requestOptions, serviceRequestContext(), /* responseCancellationScheduler */ null, requestStartTimeNanos, requestStartTimeMicros); } private DefaultClientRequestContext( @Nullable EventLoop eventLoop, MeterRegistry meterRegistry, - SessionProtocol sessionProtocol, RequestId id, HttpMethod method, String path, - @Nullable String query, @Nullable String fragment, ClientOptions options, + SessionProtocol sessionProtocol, RequestId id, HttpMethod method, + RequestTarget reqTarget, ClientOptions options, @Nullable HttpRequest req, @Nullable RpcRequest rpcReq, RequestOptions requestOptions, @Nullable ServiceRequestContext root, @Nullable CancellationScheduler responseCancellationScheduler, long requestStartTimeNanos, long requestStartTimeMicros) { - super(meterRegistry, sessionProtocol, id, method, path, query, + super(meterRegistry, sessionProtocol, id, method, reqTarget, firstNonNull(requestOptions.exchangeType(), ExchangeType.BIDI_STREAMING), req, rpcReq, getAttributes(root)); this.eventLoop = eventLoop; this.options = requireNonNull(options, "options"); - this.fragment = fragment; this.root = root; log = RequestLog.builder(this); @@ -460,8 +456,8 @@ private DefaultClientRequestContext(DefaultClientRequestContext ctx, @Nullable RpcRequest rpcReq, @Nullable Endpoint endpoint, @Nullable EndpointGroup endpointGroup, SessionProtocol sessionProtocol, HttpMethod method, - String path, @Nullable String query, @Nullable String fragment) { - super(ctx.meterRegistry(), sessionProtocol, id, method, path, query, ctx.exchangeType(), + RequestTarget reqTarget) { + super(ctx.meterRegistry(), sessionProtocol, id, method, reqTarget, ctx.exchangeType(), req, rpcReq, getAttributes(ctx.root())); // The new requests cannot be null if it was previously non-null. @@ -475,7 +471,6 @@ private DefaultClientRequestContext(DefaultClientRequestContext ctx, eventLoop = ctx.eventLoop().withoutContext(); options = ctx.options(); - this.fragment = fragment; root = ctx.root(); log = RequestLog.builder(this); @@ -533,39 +528,37 @@ public ClientRequestContext newDerivedContext(RequestId id, @Nullable Endpoint endpoint) { if (req != null) { final RequestHeaders newHeaders = req.headers(); + final String oldPath = requestTarget().pathAndQuery(); final String newPath = newHeaders.path(); - if (!path().equals(newPath)) { + if (!oldPath.equals(newPath)) { // path is changed. + final RequestTarget reqTarget = RequestTarget.forClient(newPath); + checkArgument(reqTarget != null, "invalid path: %s", newPath); - if (!isAbsoluteUri(newPath)) { - return newDerivedContext(id, req, rpcReq, newHeaders, sessionProtocol(), endpoint, newPath); + if (reqTarget.form() != RequestTargetForm.ABSOLUTE) { + // Not an absolute URI. + return new DefaultClientRequestContext(this, id, req, rpcReq, endpoint, null, + sessionProtocol(), newHeaders.method(), reqTarget); } - final URI uri = URI.create(req.path()); - final Scheme scheme = Scheme.parse(uri.getScheme()); - final SessionProtocol protocol = scheme.sessionProtocol(); - final Endpoint newEndpoint = Endpoint.parse(uri.getAuthority()); - final String rawQuery = uri.getRawQuery(); - final String pathWithQuery = pathWithQuery(uri, rawQuery); - final HttpRequest newReq = req.withHeaders(req.headers().toBuilder().path(pathWithQuery)); - return newDerivedContext(id, newReq, rpcReq, newHeaders, protocol, - newEndpoint, pathWithQuery); + + // Recalculate protocol and endpoint from the absolute URI. + final String scheme = reqTarget.scheme(); + final String authority = reqTarget.authority(); + assert scheme != null; + assert authority != null; + + final SessionProtocol protocol = Scheme.parse(scheme).sessionProtocol(); + final Endpoint newEndpoint = Endpoint.parse(authority); + final HttpRequest newReq = req.withHeaders(req.headers() + .toBuilder() + .path(reqTarget.pathAndQuery())); + return new DefaultClientRequestContext(this, id, newReq, rpcReq, newEndpoint, null, + protocol, newHeaders.method(), reqTarget); } } return new DefaultClientRequestContext(this, id, req, rpcReq, endpoint, endpointGroup(), - sessionProtocol(), method(), path(), query(), fragment()); - } - - private ClientRequestContext newDerivedContext(RequestId id, HttpRequest req, @Nullable RpcRequest rpcReq, - RequestHeaders newHeaders, SessionProtocol protocol, - @Nullable Endpoint endpoint, String pathWithQuery) { - final PathAndQuery pathAndQuery = PathAndQuery.parse(pathWithQuery); - if (pathAndQuery == null) { - throw new IllegalArgumentException("invalid path: " + req.path()); - } - return new DefaultClientRequestContext(this, id, req, rpcReq, endpoint, null, - protocol, newHeaders.method(), pathAndQuery.path(), - pathAndQuery.query(), null); + sessionProtocol(), method(), requestTarget()); } @Override @@ -576,37 +569,27 @@ protected void validateHeaders(RequestHeaders headers) { @Override protected void unsafeUpdateRequest(HttpRequest req) { - final PathAndQuery pathAndQuery; - final SessionProtocol sessionProtocol; - final String authority; - if (isAbsoluteUri(req.path())) { - final URI uri = URI.create(req.path()); - checkArgument(uri.getScheme() != null, "missing scheme"); - checkArgument(uri.getAuthority() != null, "missing authority"); - checkArgument(!uri.getAuthority().isEmpty(), "empty authority"); - final String rawQuery = uri.getRawQuery(); - final String pathWithQuery = pathWithQuery(uri, rawQuery); - pathAndQuery = PathAndQuery.parse(pathWithQuery); - sessionProtocol = Scheme.parse(uri.getScheme()).sessionProtocol(); - authority = uri.getAuthority(); - } else { - pathAndQuery = PathAndQuery.parse(req.path()); - sessionProtocol = null; - authority = null; - } - if (pathAndQuery == null) { - throw new IllegalArgumentException("invalid path: " + req.path()); - } - - // all validation is complete at this point - super.unsafeUpdateRequest(req); - path(pathAndQuery.path()); - query(pathAndQuery.query()); - if (sessionProtocol != null) { + final String rawPath = req.path(); + final RequestTarget reqTarget = RequestTarget.forClient(rawPath); + checkArgument(reqTarget != null, "invalid path: %s", rawPath); + + if (reqTarget.form() == RequestTargetForm.ABSOLUTE) { + final String authority = reqTarget.authority(); + final String scheme = reqTarget.scheme(); + assert authority != null; + assert scheme != null; + + final SessionProtocol sessionProtocol = Scheme.parse(scheme).sessionProtocol(); + final Endpoint endpoint = Endpoint.parse(authority); + + // all validation is complete at this point + super.unsafeUpdateRequest(req); + requestTarget(reqTarget); sessionProtocol(sessionProtocol); - } - if (authority != null) { - updateEndpoint(Endpoint.parse(authority)); + updateEndpoint(endpoint); + } else { + super.unsafeUpdateRequest(req); + requestTarget(reqTarget); } } @@ -663,7 +646,7 @@ public Endpoint endpoint() { @Override @Nullable public String fragment() { - return fragment; + return requestTarget().fragment(); } @Override diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtil.java b/core/src/main/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtil.java index 0ac89afe50aa..ae4a9a17bdcc 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtil.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtil.java @@ -30,6 +30,8 @@ */ package com.linecorp.armeria.internal.common; +import static com.google.common.base.MoreObjects.firstNonNull; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static io.netty.util.AsciiString.EMPTY_STRING; import static io.netty.util.ByteProcessor.FIND_COMMA; @@ -47,9 +49,6 @@ import java.util.StringJoiner; import java.util.function.BiConsumer; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import com.github.benmanes.caffeine.cache.Caffeine; import com.github.benmanes.caffeine.cache.LoadingCache; import com.google.common.annotations.VisibleForTesting; @@ -69,6 +68,7 @@ import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.RequestHeaders; import com.linecorp.armeria.common.RequestHeadersBuilder; +import com.linecorp.armeria.common.RequestTarget; import com.linecorp.armeria.common.ResponseHeaders; import com.linecorp.armeria.common.ResponseHeadersBuilder; import com.linecorp.armeria.common.annotation.Nullable; @@ -102,8 +102,6 @@ public final class ArmeriaHttpUtil { // Forked from Netty 4.1.34 at 4921f62c8ab8205fd222439dcd1811760b05daf1 - private static final Logger logger = LoggerFactory.getLogger(ArmeriaHttpUtil.class); - /** * The default case-insensitive {@link AsciiString} hasher and comparator for HTTP/2 headers. */ @@ -236,12 +234,6 @@ public boolean equals(AsciiString a, AsciiString b) { HttpHeaderNames.HOST); } - /** - * rfc7540, 8.1.2.3 - * states the path must not be empty, and instead should be {@code /}. - */ - private static final String EMPTY_REQUEST_PATH = "/"; - private static final Splitter COOKIE_SPLITTER = Splitter.on(';').trimResults().omitEmptyStrings(); private static final String COOKIE_SEPARATOR = "; "; private static final Joiner COOKIE_JOINER = Joiner.on(COOKIE_SEPARATOR); @@ -257,51 +249,72 @@ private static LoadingCache buildCache(String spec) { } /** - * Concatenates two path strings. + * Concatenates the specified {@code prefix} and {@code path} into an absolute path. + * + * @throws IllegalArgumentException if {@code prefix} is not an absolute path prefix */ - public static String concatPaths(@Nullable String path1, @Nullable String path2) { - path2 = path2 == null ? "" : path2; - - if (path1 == null || path1.isEmpty() || EMPTY_REQUEST_PATH.equals(path1)) { - if (path2.isEmpty()) { - return EMPTY_REQUEST_PATH; - } + public static String concatPaths(String prefix, @Nullable String path) { + requireNonNull(prefix, "prefix"); + checkArgument(!prefix.isEmpty() && prefix.charAt(0) == '/', + "prefix: %s (expected: an absolute path starting with '/')", prefix); + + path = firstNonNull(path, ""); + if (path.isEmpty()) { + return prefix; + } - if (path2.charAt(0) == '/') { - return path2; // Most requests will land here. + if (prefix.length() == 1) { // means "/".equals(prefix) + if (path.charAt(0) == '/') { + return path; // Most requests will land here. } - - return '/' + path2; + return simpleConcat("/", path); } - // At this point, we are sure path1 is neither empty nor null. - if (path2.isEmpty()) { - // Only path1 is non-empty. No need to concatenate. - return path1; - } + return slowConcatPaths(prefix, path); + } - if (path1.charAt(path1.length() - 1) == '/') { - if (path2.charAt(0) == '/') { - // path1 ends with '/' and path2 starts with '/'. - // Avoid double-slash by stripping the first slash of path2. - return new StringBuilder(path1.length() + path2.length() - 1) - .append(path1).append(path2, 1, path2.length()).toString(); + private static String slowConcatPaths(String prefix, String path) { + if (prefix.charAt(prefix.length() - 1) == '/') { + if (path.charAt(0) == '/') { + // `prefix` ends with '/' and `path` starts with '/'. + // Avoid double-slash by stripping the first slash of `path`. + try (TemporaryThreadLocals tmp = TemporaryThreadLocals.acquire()) { + return tmp.stringBuilder() + .append(prefix) + .append(path, 1, path.length()) + .toString(); + } } - // path1 ends with '/' and path2 does not start with '/'. + // `prefix` ends with '/' and `path` does not start with '/'. // Simple concatenation would suffice. - return path1 + path2; + return simpleConcat(prefix, path); } - if (path2.charAt(0) == '/' || path2.charAt(0) == '?') { - // path1 does not end with '/' and path2 starts with '/' or '?' + if (path.charAt(0) == '/' || path.charAt(0) == '?') { + // `prefix` does not end with '/' and `path` starts with '/' or '?' // Simple concatenation would suffice. - return path1 + path2; + return prefix + path; } - // path1 does not end with '/' and path2 does not start with '/' or '?'. - // Need to insert '/' between path1 and path2. - return path1 + '/' + path2; + // `prefix` does not end with '/' and `path` does not start with '/' or '?'. + // Need to insert '/' in-between. + try (TemporaryThreadLocals tmp = TemporaryThreadLocals.acquire()) { + return tmp.stringBuilder() + .append(prefix) + .append('/') + .append(path) + .toString(); + } + } + + private static String simpleConcat(String prefix, String path) { + try (TemporaryThreadLocals tmp = TemporaryThreadLocals.acquire()) { + return tmp.stringBuilder() + .append(prefix) + .append(path) + .toString(); + } } /** @@ -400,6 +413,23 @@ public static boolean isAbsoluteUri(@Nullable String maybeUri) { return maybeUri.charAt(firstColonIdx + 1) == '/' && maybeUri.charAt(firstColonIdx + 2) == '/'; } + /** + * Returns {@code true} if the specified {@code path} is a valid HTTP/2 {@code :path} header value. + */ + public static boolean isValidHttp2Path(String path) { + // We support only origin form and asterisk form. + if (path.isEmpty()) { + return false; + } + + final char firstChar = path.charAt(0); + if (firstChar == '/') { + return true; + } + + return firstChar == '*' && path.length() == 1; + } + /** * Returns {@code true} if the specified HTTP status string represents an informational status. */ @@ -557,7 +587,7 @@ public static long parseDirectiveValueAsSeconds(@Nullable String value) { public static RequestHeaders toArmeriaRequestHeaders(ChannelHandlerContext ctx, Http2Headers headers, boolean endOfStream, String scheme, ServerConfig cfg, - @Nullable PathAndQuery pathAndQuery) { + RequestTarget reqTarget) { assert headers instanceof ArmeriaHttp2Headers; final HttpHeadersBuilder builder = ((ArmeriaHttp2Headers) headers).delegate(); builder.endOfStream(endOfStream); @@ -565,15 +595,12 @@ public static RequestHeaders toArmeriaRequestHeaders(ChannelHandlerContext ctx, if (!builder.contains(HttpHeaderNames.SCHEME)) { builder.add(HttpHeaderNames.SCHEME, scheme); } - // if pathAndQuery == null, then either the path is invalid or *, and will be handled later. - if (pathAndQuery != null) { - builder.set(HttpHeaderNames.PATH, pathAndQuery.toString()); - } if (builder.get(HttpHeaderNames.AUTHORITY) == null && builder.get(HttpHeaderNames.HOST) == null) { final String defaultHostname = cfg.defaultVirtualHost().defaultHostname(); final int port = ((InetSocketAddress) ctx.channel().localAddress()).getPort(); builder.add(HttpHeaderNames.AUTHORITY, defaultHostname + ':' + port); } + builder.set(HttpHeaderNames.PATH, reqTarget.toString()); final List cookies = builder.getAll(HttpHeaderNames.COOKIE); if (cookies.size() > 1) { // Cookies must be concatenated into a single octet string. @@ -616,19 +643,14 @@ public static HttpHeaders toArmeria(Http2Headers http2Headers, boolean request, */ public static RequestHeaders toArmeria( ChannelHandlerContext ctx, HttpRequest in, - ServerConfig cfg, String scheme, @Nullable PathAndQuery pathAndQuery) throws URISyntaxException { + ServerConfig cfg, String scheme, RequestTarget reqTarget) throws URISyntaxException { final io.netty.handler.codec.http.HttpHeaders inHeaders = in.headers(); final RequestHeadersBuilder out = RequestHeaders.builder(); out.sizeHint(inHeaders.size()); - out.method(HttpMethod.valueOf(in.method().name())) - .scheme(scheme); - // if pathAndQuery == null, then either the path is invalid or *, and will be handled later. - if (pathAndQuery == null) { - out.path(in.uri()); - } else { - out.path(pathAndQuery.toString()); - } + out.method(firstNonNull(HttpMethod.tryParse(in.method().name()), HttpMethod.UNKNOWN)) + .scheme(scheme) + .path(reqTarget.toString()); // Add the HTTP headers which have not been consumed above toArmeria(inHeaders, out); diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/PathAndQuery.java b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultRequestTarget.java similarity index 54% rename from core/src/main/java/com/linecorp/armeria/internal/common/PathAndQuery.java rename to core/src/main/java/com/linecorp/armeria/internal/common/DefaultRequestTarget.java index 7444657e5e0d..2d3e8fb56242 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/PathAndQuery.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultRequestTarget.java @@ -13,36 +13,26 @@ * License for the specific language governing permissions and limitations * under the License. */ - package com.linecorp.armeria.internal.common; import static io.netty.util.internal.StringUtil.decodeHexNibble; import static java.util.Objects.requireNonNull; +import java.net.URI; import java.util.BitSet; import java.util.Objects; -import java.util.Set; -import com.github.benmanes.caffeine.cache.Cache; -import com.github.benmanes.caffeine.cache.Caffeine; import com.google.common.annotations.VisibleForTesting; -import com.linecorp.armeria.common.Flags; +import com.linecorp.armeria.common.RequestTarget; +import com.linecorp.armeria.common.RequestTargetForm; import com.linecorp.armeria.common.annotation.Nullable; -import com.linecorp.armeria.common.metric.MeterIdPrefix; -import com.linecorp.armeria.internal.common.metric.CaffeineMetricSupport; +import com.linecorp.armeria.internal.common.util.TemporaryThreadLocals; -import io.micrometer.core.instrument.MeterRegistry; import it.unimi.dsi.fastutil.Arrays; import it.unimi.dsi.fastutil.bytes.ByteArrays; -/** - * A parser of the raw path and query components of an HTTP path. Performs validation and allows caching of - * results. - */ -public final class PathAndQuery { - - private static final PathAndQuery ROOT_PATH_QUERY = new PathAndQuery("/", null); +public final class DefaultRequestTarget implements RequestTarget { /** * The lookup table for the characters allowed in a path. @@ -90,158 +80,229 @@ public final class PathAndQuery { } } - private static final Bytes EMPTY_QUERY = new Bytes(0); - private static final Bytes ROOT_PATH = new Bytes(new byte[] { '/' }); - - @Nullable - private static final Cache CACHE = - Flags.parsedPathCacheSpec() != null ? buildCache(Flags.parsedPathCacheSpec()) : null; - - private static Cache buildCache(String spec) { - return Caffeine.from(spec).build(); - } - - public static void registerMetrics(MeterRegistry registry, MeterIdPrefix idPrefix) { - if (CACHE != null) { - CaffeineMetricSupport.setup(registry, idPrefix, CACHE); - } - } + private static final Bytes EMPTY_BYTES = new Bytes(0); + private static final Bytes SLASH_BYTES = new Bytes(new byte[] { '/' }); /** - * Clears the currently cached parsed paths. Only for use in tests. + * The main implementation of {@link RequestTarget#forServer(String)}. */ - @VisibleForTesting - public static void clearCachedPaths() { - requireNonNull(CACHE, "CACHE"); - CACHE.asMap().clear(); - } + @Nullable + public static RequestTarget forServer(String reqTarget, boolean allowDoubleDotsInQueryString) { + final RequestTarget cached = RequestTargetCache.getForServer(reqTarget); + if (cached != null) { + return cached; + } - /** - * Returns paths that have had their parse result cached. Only for use in tests. - */ - @VisibleForTesting - public static Set cachedPaths() { - requireNonNull(CACHE, "CACHE"); - return CACHE.asMap().keySet(); + return slowForServer(reqTarget, allowDoubleDotsInQueryString); } /** - * Validates the {@link String} that contains an absolute path and a query, and splits them into - * the path part and the query part. If the path is usable (e.g., can be served a successful response from - * the server and doesn't have variable path parameters), {@link PathAndQuery#storeInCache(String)} should - * be called to cache the parsing result for faster future invocations. - * - * @return a {@link PathAndQuery} with the absolute path and query, or {@code null} if the specified - * {@link String} is not an absolute path or invalid. + * The main implementation of {@link RequestTarget#forClient(String, String)}. */ @Nullable - public static PathAndQuery parse(@Nullable String rawPath) { - return parse(rawPath, Flags.allowDoubleDotsInQueryString()); - } - - @VisibleForTesting - @Nullable - static PathAndQuery parse(@Nullable String rawPath, boolean allowDoubleDotsInQueryString) { - if (CACHE != null && rawPath != null) { - final PathAndQuery parsed = CACHE.getIfPresent(rawPath); - if (parsed != null) { - return parsed; + public static RequestTarget forClient(String reqTarget, @Nullable String prefix) { + requireNonNull(reqTarget, "reqTarget"); + + final int authorityPos = findAuthority(reqTarget); + if (authorityPos >= 0) { + final RequestTarget cached = RequestTargetCache.getForClient(reqTarget); + if (cached != null) { + return cached; } + + // reqTarget is an absolute URI with scheme and authority. + return slowAbsoluteFormForClient(reqTarget, authorityPos); + } + + // Concatenate `prefix` and `reqTarget` if necessary. + final String actualReqTarget; + if (prefix == null || prefix.isEmpty() || "*".equals(reqTarget)) { + // No prefix was given or request target is `*`. + actualReqTarget = reqTarget; + } else { + actualReqTarget = ArmeriaHttpUtil.concatPaths(prefix, reqTarget); + } + + final RequestTarget cached = RequestTargetCache.getForClient(actualReqTarget); + if (cached != null) { + return cached; } - return splitPathAndQuery(rawPath, allowDoubleDotsInQueryString); + + // reqTarget is not an absolute URI; split by the first '?'. + return slowForClient(actualReqTarget, null, 0); } /** - * Stores this {@link PathAndQuery} into cache for the given raw path. This should be used by callers when - * the parsed result was valid (e.g., when a server is able to successfully handle the parsed path). + * (Advanced users only) Returns a newly created {@link RequestTarget} filled with the specified + * properties without any validation. */ - public void storeInCache(@Nullable String rawPath) { - if (CACHE != null && !cached && rawPath != null) { - cached = true; - CACHE.put(rawPath, this); - } + public static RequestTarget createWithoutValidation( + RequestTargetForm form, @Nullable String scheme, @Nullable String authority, + String path, @Nullable String query, @Nullable String fragment) { + return new DefaultRequestTarget(form, scheme, authority, path, query, fragment); } + private final RequestTargetForm form; + @Nullable + private final String scheme; + @Nullable + private final String authority; private final String path; @Nullable private final String query; - + @Nullable + private final String fragment; private boolean cached; - private PathAndQuery(String path, @Nullable String query) { + private DefaultRequestTarget(RequestTargetForm form, @Nullable String scheme, @Nullable String authority, + String path, @Nullable String query, @Nullable String fragment) { + + assert (scheme != null && authority != null) || + (scheme == null && authority == null) : "scheme: " + scheme + ", authority: " + authority; + + this.form = form; + this.scheme = scheme; + this.authority = authority; this.path = path; this.query = query; + this.fragment = fragment; + } + + @Override + public RequestTargetForm form() { + return form; + } + + @Override + public String scheme() { + return scheme; } + @Override + public String authority() { + return authority; + } + + @Override public String path() { return path; } - @Nullable + @Override public String query() { return query; } + @Override + public String fragment() { + return fragment; + } + + /** + * Returns {@code true} if this {@link RequestTarget} is already stored in {@link RequestTargetCache}. + */ + public boolean isCached() { + return cached; + } + + /** + * Marks this {@link RequestTarget} as stored in {@link RequestTargetCache} so that it doesn't + * try to store again. + */ + public void setCached() { + cached = true; + } + + /** + * Returns a copy of this {@link RequestTarget} with its {@link #path()} overridden with + * the specified {@code path}. + */ + public RequestTarget withPath(String path) { + if (this.path == path) { + return this; + } + + return new DefaultRequestTarget(form, scheme, authority, path, query, fragment); + } + @Override public boolean equals(@Nullable Object o) { if (this == o) { return true; } - if (!(o instanceof PathAndQuery)) { + if (!(o instanceof DefaultRequestTarget)) { return false; } - final PathAndQuery that = (PathAndQuery) o; - return Objects.equals(path, that.path) && - Objects.equals(query, that.query); + final DefaultRequestTarget that = (DefaultRequestTarget) o; + return path.equals(that.path) && + Objects.equals(query, that.query) && + Objects.equals(fragment, that.fragment) && + Objects.equals(authority, that.authority) && + Objects.equals(scheme, that.scheme); } @Override public int hashCode() { - return Objects.hash(path, query); + return Objects.hash(scheme, authority, path, query, fragment); } @Override public String toString() { - if (query == null) { - return path; + try (TemporaryThreadLocals tmp = TemporaryThreadLocals.acquire()) { + final StringBuilder buf = tmp.stringBuilder(); + if (scheme != null) { + buf.append(scheme).append("://").append(authority); + } + buf.append(path); + if (query != null) { + buf.append('?').append(query); + } + if (fragment != null) { + buf.append('#').append(fragment); + } + return buf.toString(); } - return path + '?' + query; } @Nullable - private static PathAndQuery splitPathAndQuery(@Nullable String pathAndQuery, - boolean allowDoubleDotsInQueryString) { + private static RequestTarget slowForServer(String reqTarget, boolean allowDoubleDotsInQueryString) { final Bytes path; final Bytes query; - if (pathAndQuery == null) { - return ROOT_PATH_QUERY; - } - // Split by the first '?'. - final int queryPos = pathAndQuery.indexOf('?'); + final int queryPos = reqTarget.indexOf('?'); if (queryPos >= 0) { if ((path = decodePercentsAndEncodeToUtf8( - pathAndQuery, 0, queryPos, true)) == null) { + reqTarget, 0, queryPos, true, SLASH_BYTES)) == null) { return null; } if ((query = decodePercentsAndEncodeToUtf8( - pathAndQuery, queryPos + 1, pathAndQuery.length(), false)) == null) { + reqTarget, queryPos + 1, reqTarget.length(), false, EMPTY_BYTES)) == null) { return null; } } else { if ((path = decodePercentsAndEncodeToUtf8( - pathAndQuery, 0, pathAndQuery.length(), true)) == null) { + reqTarget, 0, reqTarget.length(), true, SLASH_BYTES)) == null) { return null; } query = null; } - if (path.data[0] != '/' || path.isEncoded(0)) { - // Do not accept a relative path. - return null; + // Reject a relative path and accept an asterisk (e.g. OPTIONS * HTTP/1.1). + if (isRelativePath(path)) { + if (query == null && path.length == 1 && path.data[0] == '*') { + return new DefaultRequestTarget(RequestTargetForm.ASTERISK, + null, + null, + "*", + null, + null); + } else { + // Do not accept a relative path. + return null; + } } // Reject the prohibited patterns. @@ -252,24 +313,222 @@ private static PathAndQuery splitPathAndQuery(@Nullable String pathAndQuery, return null; } - return new PathAndQuery(encodePathToPercents(path), encodeQueryToPercents(query)); + return new DefaultRequestTarget(RequestTargetForm.ORIGIN, + null, + null, + encodePathToPercents(path), + encodeQueryToPercents(query), + null); + } + + @Nullable + private static RequestTarget slowAbsoluteFormForClient(String reqTarget, int authorityPos) { + // Extract scheme and authority while looking for path. + final URI schemeAndAuthority; + final String scheme = reqTarget.substring(0, authorityPos - 3); + final int pathPos = reqTarget.indexOf('/', authorityPos); + final String authority; + if (pathPos < 0) { + // Path doesn't exist. + authority = reqTarget.substring(authorityPos); + } else { + // Path (and maybe query) exists. + authority = reqTarget.substring(authorityPos, pathPos); + } + + // Reject a URI with an empty authority. + if (authority.isEmpty()) { + return null; + } + + // Normalize scheme and authority. + schemeAndAuthority = normalizeSchemeAndAuthority(scheme, authority); + if (schemeAndAuthority == null) { + // Invalid scheme or authority. + return null; + } + + if (pathPos < 0) { + return new DefaultRequestTarget(RequestTargetForm.ABSOLUTE, + schemeAndAuthority.getScheme(), + schemeAndAuthority.getRawAuthority(), + "/", + null, + null); + } + + return slowForClient(reqTarget, schemeAndAuthority, pathPos); + } + + @Nullable + private static RequestTarget slowForClient(String reqTarget, + @Nullable URI schemeAndAuthority, + int pathPos) { + final Bytes fragment; + final Bytes path; + final Bytes query; + // Find where a query string and a fragment starts. + final int queryPos; + final int fragmentPos; + final int maybeQueryPos = reqTarget.indexOf('?', pathPos + 1); + final int maybeFragmentPos = reqTarget.indexOf('#', pathPos + 1); + if (maybeQueryPos >= 0) { + // Found '?'. + if (maybeFragmentPos >= 0) { + // Found '#', too. + fragmentPos = maybeFragmentPos; + if (maybeQueryPos < maybeFragmentPos) { + // '#' appeared after '?', e.g. ?foo#bar + queryPos = maybeQueryPos; + } else { + // '#' appeared before '?', e.g. #foo?bar. + // It means the '?' we found is not a part of query string. + queryPos = -1; + } + } else { + // No '#' in reqTarget. + queryPos = maybeQueryPos; + fragmentPos = -1; + } + } else { + // No '?'. + queryPos = -1; + fragmentPos = maybeFragmentPos; + } + + // Split into path, query and fragment. + if (queryPos >= 0) { + if ((path = decodePercentsAndEncodeToUtf8( + reqTarget, pathPos, queryPos, true, EMPTY_BYTES)) == null) { + return null; + } + + if (fragmentPos >= 0) { + // path?query#fragment + if ((query = decodePercentsAndEncodeToUtf8( + reqTarget, queryPos + 1, fragmentPos, false, EMPTY_BYTES)) == null) { + return null; + } + if ((fragment = decodePercentsAndEncodeToUtf8( + reqTarget, fragmentPos + 1, reqTarget.length(), true, EMPTY_BYTES)) == null) { + return null; + } + } else { + // path?query + if ((query = decodePercentsAndEncodeToUtf8( + reqTarget, queryPos + 1, reqTarget.length(), false, EMPTY_BYTES)) == null) { + return null; + } + fragment = null; + } + } else { + if (fragmentPos >= 0) { + // path#fragment + if ((path = decodePercentsAndEncodeToUtf8( + reqTarget, pathPos, fragmentPos, true, EMPTY_BYTES)) == null) { + return null; + } + query = null; + if ((fragment = decodePercentsAndEncodeToUtf8( + reqTarget, fragmentPos + 1, reqTarget.length(), true, EMPTY_BYTES)) == null) { + return null; + } + } else { + // path + if ((path = decodePercentsAndEncodeToUtf8( + reqTarget, pathPos, reqTarget.length(), true, EMPTY_BYTES)) == null) { + return null; + } + query = null; + fragment = null; + } + } + + // Reject a relative path and accept an asterisk (e.g. OPTIONS * HTTP/1.1). + if (isRelativePath(path)) { + if (query == null && path.length == 1 && path.data[0] == '*') { + return new DefaultRequestTarget(RequestTargetForm.ASTERISK, + null, + null, + "*", + null, + null); + } else { + // Do not accept a relative path. + return null; + } + } + + final String encodedPath = encodePathToPercents(path); + final String encodedQuery = encodeQueryToPercents(query); + final String encodedFragment = encodeFragmentToPercents(fragment); + if (schemeAndAuthority != null) { + return new DefaultRequestTarget(RequestTargetForm.ABSOLUTE, + schemeAndAuthority.getScheme(), + schemeAndAuthority.getRawAuthority(), + encodedPath, + encodedQuery, + encodedFragment); + } else { + return new DefaultRequestTarget(RequestTargetForm.ORIGIN, + null, + null, + encodedPath, + encodedQuery, + encodedFragment); + } + } + + /** + * Returns the index of the authority part if the specified {@code reqTarget} is an absolute URI. + * Returns {@code -1} otherwise. + */ + private static int findAuthority(String reqTarget) { + final int firstColonIdx = reqTarget.indexOf(':'); + if (firstColonIdx <= 0 || reqTarget.length() <= firstColonIdx + 3) { + return -1; + } + final int firstSlashIdx = reqTarget.indexOf('/'); + if (firstSlashIdx <= 0 || firstSlashIdx < firstColonIdx) { + return -1; + } + + if (reqTarget.charAt(firstColonIdx + 1) == '/' && reqTarget.charAt(firstColonIdx + 2) == '/') { + return firstColonIdx + 3; + } + + return -1; + } + + @Nullable + private static URI normalizeSchemeAndAuthority(String scheme, String authority) { + try { + return new URI(scheme, authority, null, null, null); + } catch (Exception unused) { + return null; + } + } + + private static boolean isRelativePath(Bytes path) { + return path.length == 0 || path.data[0] != '/' || path.isEncoded(0); } /** - * Decodes a percent-encoded query string. This method is only used for {@code PathAndQueryTest}. + * Decodes a percent-encoded query string. This method is only used for {@code RequestTargetTest}. */ @Nullable @VisibleForTesting static String decodePercentEncodedQuery(String query) { - final Bytes bytes = decodePercentsAndEncodeToUtf8(query, 0, query.length(), false); + final Bytes bytes = decodePercentsAndEncodeToUtf8(query, 0, query.length(), false, EMPTY_BYTES); return encodeQueryToPercents(bytes); } @Nullable - private static Bytes decodePercentsAndEncodeToUtf8(String value, int start, int end, boolean isPath) { + private static Bytes decodePercentsAndEncodeToUtf8(String value, int start, int end, + boolean isPath, Bytes whenEmpty) { final int length = end - start; if (length == 0) { - return isPath ? ROOT_PATH : EMPTY_QUERY; + return whenEmpty; } final Bytes buf = new Bytes(Math.max(length * 3 / 2, 4)); @@ -520,6 +779,22 @@ private static String encodeQueryToPercents(@Nullable Bytes value) { return slowEncodeQueryToPercents(value); } + @Nullable + private static String encodeFragmentToPercents(@Nullable Bytes value) { + if (value == null) { + return null; + } + + if (!value.hasEncodedBytes()) { + // Deprecated, but it fits perfect for our use case. + // noinspection deprecation + return new String(value.data, 0, 0, value.length); + } + + // Slow path: some percent-encoded chars. + return slowEncodePathToPercents(value); + } + private static String slowEncodePathToPercents(Bytes value) { final int length = value.length; final StringBuilder buf = new StringBuilder(length + value.numEncodedBytes() * 2); diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/NonWrappingRequestContext.java b/core/src/main/java/com/linecorp/armeria/internal/common/NonWrappingRequestContext.java index 183389144f0c..e6e1abcef3a5 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/NonWrappingRequestContext.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/NonWrappingRequestContext.java @@ -34,6 +34,7 @@ import com.linecorp.armeria.common.RequestContextStorage; import com.linecorp.armeria.common.RequestHeaders; import com.linecorp.armeria.common.RequestId; +import com.linecorp.armeria.common.RequestTarget; import com.linecorp.armeria.common.RpcRequest; import com.linecorp.armeria.common.SessionProtocol; import com.linecorp.armeria.common.annotation.Nullable; @@ -59,14 +60,12 @@ public abstract class NonWrappingRequestContext implements RequestContextExtensi private SessionProtocol sessionProtocol; private final RequestId id; private final HttpMethod method; - private String path; + private RequestTarget reqTarget; private final ExchangeType exchangeType; @Nullable private String decodedPath; @Nullable - private String query; - @Nullable private volatile HttpRequest req; @Nullable private volatile RpcRequest rpcReq; @@ -83,7 +82,7 @@ public abstract class NonWrappingRequestContext implements RequestContextExtensi */ protected NonWrappingRequestContext( MeterRegistry meterRegistry, SessionProtocol sessionProtocol, - RequestId id, HttpMethod method, String path, @Nullable String query, ExchangeType exchangeType, + RequestId id, HttpMethod method, RequestTarget reqTarget, ExchangeType exchangeType, @Nullable HttpRequest req, @Nullable RpcRequest rpcReq, @Nullable AttributesGetters rootAttributeMap) { @@ -97,8 +96,7 @@ protected NonWrappingRequestContext( this.sessionProtocol = requireNonNull(sessionProtocol, "sessionProtocol"); this.id = requireNonNull(id, "id"); this.method = requireNonNull(method, "method"); - this.path = requireNonNull(path, "path"); - this.query = query; + this.reqTarget = requireNonNull(reqTarget, "reqTarget"); this.exchangeType = requireNonNull(exchangeType, "exchangeType"); this.req = req; this.rpcReq = rpcReq; @@ -189,11 +187,16 @@ public final HttpMethod method() { @Override public final String path() { - return path; + return reqTarget.path(); + } + + protected final RequestTarget requestTarget() { + return reqTarget; } - protected void path(String path) { - this.path = requireNonNull(path, "path"); + protected final void requestTarget(RequestTarget reqTarget) { + this.reqTarget = requireNonNull(reqTarget, "reqTarget"); + decodedPath = null; } @Override @@ -203,16 +206,12 @@ public final String decodedPath() { return decodedPath; } - return this.decodedPath = ArmeriaHttpUtil.decodePath(path); + return this.decodedPath = ArmeriaHttpUtil.decodePath(path()); } @Override public final String query() { - return query; - } - - protected void query(@Nullable String query) { - this.query = query; + return reqTarget.query(); } @Override diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/RequestTargetCache.java b/core/src/main/java/com/linecorp/armeria/internal/common/RequestTargetCache.java new file mode 100644 index 000000000000..3d2c92018de7 --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/internal/common/RequestTargetCache.java @@ -0,0 +1,129 @@ +/* + * Copyright 2023 LINE Corporation + * + * LINE Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package com.linecorp.armeria.internal.common; + +import java.util.Set; + +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; +import com.google.common.annotations.VisibleForTesting; + +import com.linecorp.armeria.common.Flags; +import com.linecorp.armeria.common.RequestTarget; +import com.linecorp.armeria.common.annotation.Nullable; +import com.linecorp.armeria.common.metric.MeterIdPrefix; +import com.linecorp.armeria.internal.common.metric.CaffeineMetricSupport; + +import io.micrometer.core.instrument.MeterRegistry; + +public final class RequestTargetCache { + + private static final MeterIdPrefix METER_ID_PREFIX = new MeterIdPrefix("armeria.path.cache"); + + @Nullable + private static final Cache SERVER_CACHE = + Flags.parsedPathCacheSpec() != null ? buildCache(Flags.parsedPathCacheSpec()) : null; + + @Nullable + private static final Cache CLIENT_CACHE = + Flags.parsedPathCacheSpec() != null ? buildCache(Flags.parsedPathCacheSpec()) : null; + + private static Cache buildCache(String spec) { + return Caffeine.from(spec).build(); + } + + public static void registerServerMetrics(MeterRegistry registry) { + if (SERVER_CACHE != null) { + CaffeineMetricSupport.setup(registry, METER_ID_PREFIX.withTags("type", "server"), SERVER_CACHE); + } + } + + public static void registerClientMetrics(MeterRegistry registry) { + if (CLIENT_CACHE != null) { + CaffeineMetricSupport.setup(registry, METER_ID_PREFIX.withTags("type", "client"), CLIENT_CACHE); + } + } + + @Nullable + public static RequestTarget getForServer(String reqTarget) { + return get(reqTarget, SERVER_CACHE); + } + + @Nullable + public static RequestTarget getForClient(String reqTarget) { + return get(reqTarget, CLIENT_CACHE); + } + + @Nullable + private static RequestTarget get(String reqTarget, @Nullable Cache cache) { + if (cache != null) { + return cache.getIfPresent(reqTarget); + } else { + return null; + } + } + + public static void putForServer(String reqTarget, RequestTarget normalized) { + put(reqTarget, normalized, SERVER_CACHE); + } + + public static void putForClient(String reqTarget, RequestTarget normalized) { + put(reqTarget, normalized, CLIENT_CACHE); + } + + private static void put(String reqTarget, RequestTarget normalized, + @Nullable Cache cache) { + + if (cache != null && normalized instanceof DefaultRequestTarget) { + final DefaultRequestTarget value = (DefaultRequestTarget) normalized; + if (!value.isCached()) { + value.setCached(); + cache.put(reqTarget, normalized); + } + } + } + + /** + * Clears the currently cached parsed paths. Only for use in tests. + */ + @VisibleForTesting + public static void clearCachedPaths() { + assert CLIENT_CACHE != null : "CLIENT_CACHE"; + assert SERVER_CACHE != null : "SERVER_CACHE"; + CLIENT_CACHE.asMap().clear(); + SERVER_CACHE.asMap().clear(); + } + + /** + * Returns server-side paths that have had their parse result cached. Only for use in tests. + */ + @VisibleForTesting + public static Set cachedServerPaths() { + assert SERVER_CACHE != null : "SERVER_CACHE"; + return SERVER_CACHE.asMap().keySet(); + } + + /** + * Returns client-side paths that have had their parse result cached. Only for use in tests. + */ + @VisibleForTesting + public static Set cachedClientPaths() { + assert CLIENT_CACHE != null : "CLIENT_CACHE"; + return CLIENT_CACHE.asMap().keySet(); + } + + private RequestTargetCache() {} +} diff --git a/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java b/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java index 9cb92aff722a..727f1cb6183d 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java +++ b/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java @@ -160,8 +160,8 @@ public DefaultServiceRequestContext( HttpHeaders additionalResponseHeaders, HttpHeaders additionalResponseTrailers) { super(meterRegistry, sessionProtocol, id, - requireNonNull(routingContext, "routingContext").method(), routingContext.path(), - requireNonNull(routingResult, "routingResult").query(), exchangeType, + requireNonNull(routingContext, "routingContext").method(), + routingContext.requestTarget(), exchangeType, requireNonNull(req, "req"), null, null); this.ch = requireNonNull(ch, "ch"); diff --git a/core/src/main/java/com/linecorp/armeria/server/DefaultRoutingContext.java b/core/src/main/java/com/linecorp/armeria/server/DefaultRoutingContext.java index d7415a038a8d..f59503ad8910 100644 --- a/core/src/main/java/com/linecorp/armeria/server/DefaultRoutingContext.java +++ b/core/src/main/java/com/linecorp/armeria/server/DefaultRoutingContext.java @@ -29,8 +29,8 @@ import com.linecorp.armeria.common.MediaType; import com.linecorp.armeria.common.QueryParams; import com.linecorp.armeria.common.RequestHeaders; +import com.linecorp.armeria.common.RequestTarget; import com.linecorp.armeria.common.annotation.Nullable; -import com.linecorp.armeria.internal.common.PathAndQuery; /** * Holds the parameters which are required to find a service available to handle the request. @@ -40,33 +40,16 @@ final class DefaultRoutingContext implements RoutingContext { /** * Returns a new {@link RoutingContext} instance. */ - static RoutingContext of(VirtualHost virtualHost, String hostname, - String path, @Nullable String query, + static RoutingContext of(VirtualHost virtualHost, String hostname, RequestTarget reqTarget, RequestHeaders headers, RoutingStatus routingStatus) { - return new DefaultRoutingContext(virtualHost, hostname, headers, path, query, null, - routingStatus); - } - - /** - * Returns a new {@link RoutingContext} instance. - */ - static RoutingContext of(VirtualHost virtualHost, String hostname, - PathAndQuery pathAndQuery, - RequestHeaders headers, RoutingStatus routingStatus) { - requireNonNull(pathAndQuery, "pathAndQuery"); - return new DefaultRoutingContext(virtualHost, hostname, headers, pathAndQuery.path(), - pathAndQuery.query(), pathAndQuery, routingStatus); + return new DefaultRoutingContext(virtualHost, hostname, headers, reqTarget, routingStatus); } private final VirtualHost virtualHost; private final String hostname; private final HttpMethod method; private final RequestHeaders headers; - private final String path; - @Nullable - private final String query; - @Nullable - private final PathAndQuery pathAndQuery; + private final RequestTarget reqTarget; @Nullable private final MediaType contentType; private final List acceptTypes; @@ -81,14 +64,11 @@ static RoutingContext of(VirtualHost virtualHost, String hostname, private final int hashCode; DefaultRoutingContext(VirtualHost virtualHost, String hostname, RequestHeaders headers, - String path, @Nullable String query, @Nullable PathAndQuery pathAndQuery, - RoutingStatus routingStatus) { + RequestTarget reqTarget, RoutingStatus routingStatus) { this.virtualHost = requireNonNull(virtualHost, "virtualHost"); this.hostname = requireNonNull(hostname, "hostname"); this.headers = requireNonNull(headers, "headers"); - this.path = requireNonNull(path, "path"); - this.query = query; - this.pathAndQuery = pathAndQuery; + this.reqTarget = requireNonNull(reqTarget, "reqTarget"); this.routingStatus = routingStatus; method = headers.method(); contentType = headers.contentType(); @@ -112,25 +92,26 @@ public HttpMethod method() { } @Override - public String path() { - return path; + public RequestTarget requestTarget() { + return reqTarget; } - @Nullable @Override - public String query() { - return query; + public String path() { + return reqTarget.path(); } @Nullable - PathAndQuery pathAndQuery() { - return pathAndQuery; + @Override + public String query() { + return reqTarget.query(); } @Override public QueryParams params() { QueryParams queryParams = this.queryParams; if (queryParams == null) { + final String query = reqTarget.query(); if (query == null) { queryParams = QueryParams.of(); } else { @@ -269,8 +250,7 @@ static String toString(RoutingContext routingCtx) { if (!routingCtx.acceptTypes().isEmpty()) { helper.add("acceptTypes", routingCtx.acceptTypes()); } - helper.add("isCorsPreflight", routingCtx.isCorsPreflight()) - .add("requiresMatchingParamsPredicates", routingCtx.requiresMatchingParamsPredicates()) + helper.add("requiresMatchingParamsPredicates", routingCtx.requiresMatchingParamsPredicates()) .add("requiresMatchingHeadersPredicates", routingCtx.requiresMatchingHeadersPredicates()); return helper.toString(); } diff --git a/core/src/main/java/com/linecorp/armeria/server/Http1RequestDecoder.java b/core/src/main/java/com/linecorp/armeria/server/Http1RequestDecoder.java index 64b212ac3e1b..86ccd6bb59e6 100644 --- a/core/src/main/java/com/linecorp/armeria/server/Http1RequestDecoder.java +++ b/core/src/main/java/com/linecorp/armeria/server/Http1RequestDecoder.java @@ -33,6 +33,7 @@ import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.ProtocolViolationException; import com.linecorp.armeria.common.RequestHeaders; +import com.linecorp.armeria.common.RequestTarget; import com.linecorp.armeria.common.ResponseHeaders; import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.internal.common.ArmeriaHttpUtil; @@ -40,7 +41,6 @@ import com.linecorp.armeria.internal.common.InitiateConnectionShutdown; import com.linecorp.armeria.internal.common.KeepAliveHandler; import com.linecorp.armeria.internal.common.NoopKeepAliveHandler; -import com.linecorp.armeria.internal.common.PathAndQuery; import com.linecorp.armeria.server.HttpServerUpgradeHandler.UpgradeEvent; import io.netty.buffer.ByteBuf; @@ -148,13 +148,6 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception return; } - // Do not accept unsupported methods. - final io.netty.handler.codec.http.HttpMethod nettyMethod = nettyReq.method(); - if (!HttpMethod.isSupported(nettyMethod.name())) { - fail(id, null, HttpStatus.METHOD_NOT_ALLOWED, "Unsupported method", null); - return; - } - // Handle `expect: 100-continue` first to give `handle100Continue()` a chance to remove // the `expect` header before converting the Netty HttpHeaders into Armeria RequestHeaders. // This is because removing a header from RequestHeaders is more expensive due to its @@ -163,15 +156,30 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception final String path = HttpHeaderUtil .maybeTransformAbsoluteUri(nettyReq.uri(), cfg.absoluteUriTransformer()); - final PathAndQuery pathAndQuery = PathAndQuery.parse(path); + + // Parse and normalize the request path. + final RequestTarget reqTarget = RequestTarget.forServer(path); + if (reqTarget == null) { + failWithInvalidRequestPath(id, null); + return; + } // Convert the Netty HttpHeaders into Armeria RequestHeaders. final RequestHeaders headers = - ArmeriaHttpUtil.toArmeria(ctx, nettyReq, cfg, scheme.toString(), pathAndQuery); + ArmeriaHttpUtil.toArmeria(ctx, nettyReq, cfg, scheme.toString(), reqTarget); - // Do not accept a CONNECT request. - if (headers.method() == HttpMethod.CONNECT) { - fail(id, headers, HttpStatus.METHOD_NOT_ALLOWED, "Unsupported method", null); + // Do not accept unsupported methods. + final HttpMethod method = headers.method(); + switch (method) { + case CONNECT: + case UNKNOWN: + fail(id, headers, HttpStatus.METHOD_NOT_ALLOWED, "Unsupported method", null); + return; + } + + // Do not accept the request path '*' for a non-OPTIONS request. + if (method != HttpMethod.OPTIONS && "*".equals(path)) { + failWithInvalidRequestPath(id, headers); return; } @@ -204,7 +212,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception // Close the request early when it is certain there will be neither content nor trailers. final RoutingContext routingCtx = newRoutingContext(cfg, ctx.channel(), - headers, pathAndQuery); + headers, reqTarget); if (routingCtx.status().routeMustExist()) { try { // Find the service that matches the path. @@ -341,6 +349,10 @@ private boolean handle100Continue(int id, HttpRequest nettyReq) { return true; } + private void failWithInvalidRequestPath(int id, @Nullable RequestHeaders headers) { + fail(id, headers, HttpStatus.BAD_REQUEST, "Invalid request path", null); + } + private void fail(int id, @Nullable RequestHeaders headers, HttpStatus status, Http2Error error, @Nullable String message, @Nullable Throwable cause) { if (encoder.isResponseHeadersSent(id, 1)) { diff --git a/core/src/main/java/com/linecorp/armeria/server/Http2RequestDecoder.java b/core/src/main/java/com/linecorp/armeria/server/Http2RequestDecoder.java index b2af187496a5..c66ab7225393 100644 --- a/core/src/main/java/com/linecorp/armeria/server/Http2RequestDecoder.java +++ b/core/src/main/java/com/linecorp/armeria/server/Http2RequestDecoder.java @@ -31,6 +31,7 @@ import com.linecorp.armeria.common.HttpMethod; import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.RequestHeaders; +import com.linecorp.armeria.common.RequestTarget; import com.linecorp.armeria.common.ResponseHeaders; import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.common.stream.ClosedStreamException; @@ -38,7 +39,6 @@ import com.linecorp.armeria.internal.common.Http2GoAwayHandler; import com.linecorp.armeria.internal.common.InboundTrafficController; import com.linecorp.armeria.internal.common.KeepAliveHandler; -import com.linecorp.armeria.internal.common.PathAndQuery; import io.netty.buffer.ByteBuf; import io.netty.channel.Channel; @@ -119,25 +119,38 @@ public void onHeadersRead(ChannelHandlerContext ctx, int streamId, Http2Headers return; } - // Reject a request with an unsupported method. - final HttpMethod method = HttpMethod.tryParse(methodText.toString()); - if (method == null) { - writeErrorResponse(streamId, null, HttpStatus.METHOD_NOT_ALLOWED, "Unsupported method", null); + // Parse and normalize the request path. + final String path = nettyHeaders.path().toString(); + final RequestTarget reqTarget = RequestTarget.forServer(path); + if (reqTarget == null) { + writeInvalidRequestPathResponse(streamId, null); return; } - final PathAndQuery pathAndQuery = PathAndQuery.parse(nettyHeaders.path().toString()); - // Convert the Netty Http2Headers into Armeria RequestHeaders. final RequestHeaders headers = ArmeriaHttpUtil.toArmeriaRequestHeaders(ctx, nettyHeaders, endOfStream, - scheme, cfg, pathAndQuery); + scheme, cfg, reqTarget); + + // Reject a request with an unsupported method. + final HttpMethod method = headers.method(); + switch (method) { + case CONNECT: + // Accept a CONNECT request only when it has a :protocol header, as defined in: + // https://datatracker.ietf.org/doc/html/rfc8441#section-4 + if (!nettyHeaders.contains(HttpHeaderNames.PROTOCOL)) { + writeUnsupportedMethodResponse(streamId, headers); + return; + } + break; + case UNKNOWN: + writeUnsupportedMethodResponse(streamId, headers); + return; + } - // Accept a CONNECT request only when it has a :protocol header, as defined in: - // https://datatracker.ietf.org/doc/html/rfc8441#section-4 - if (method == HttpMethod.CONNECT && !nettyHeaders.contains(HttpHeaderNames.PROTOCOL)) { - writeErrorResponse(streamId, headers, HttpStatus.METHOD_NOT_ALLOWED, - "Unsupported method", null); + // Do not accept the request path '*' for a non-OPTIONS request. + if (method != HttpMethod.OPTIONS && "*".equals(path)) { + writeInvalidRequestPathResponse(streamId, headers); return; } @@ -162,7 +175,7 @@ public void onHeadersRead(ChannelHandlerContext ctx, int streamId, Http2Headers return; } - final RoutingContext routingCtx = newRoutingContext(cfg, ctx.channel(), headers, pathAndQuery); + final RoutingContext routingCtx = newRoutingContext(cfg, ctx.channel(), headers, reqTarget); if (routingCtx.status().routeMustExist()) { try { // Find the service that matches the path. @@ -337,6 +350,16 @@ private static boolean isWritable(@Nullable Http2Stream stream) { } } + private void writeInvalidRequestPathResponse(int streamId, @Nullable RequestHeaders headers) { + writeErrorResponse(streamId, headers, HttpStatus.BAD_REQUEST, + "Invalid request path", null); + } + + private void writeUnsupportedMethodResponse(int streamId, RequestHeaders headers) { + writeErrorResponse(streamId, headers, HttpStatus.METHOD_NOT_ALLOWED, + "Unsupported method", null); + } + private void writeErrorResponse(int streamId, @Nullable RequestHeaders headers, HttpStatus status, @Nullable String message, @Nullable Throwable cause) { diff --git a/core/src/main/java/com/linecorp/armeria/server/HttpHeaderUtil.java b/core/src/main/java/com/linecorp/armeria/server/HttpHeaderUtil.java index 415540a47d25..6179a124f0d1 100644 --- a/core/src/main/java/com/linecorp/armeria/server/HttpHeaderUtil.java +++ b/core/src/main/java/com/linecorp/armeria/server/HttpHeaderUtil.java @@ -179,7 +179,7 @@ static void ensureUniqueMediaTypes(Iterable types, String typeName) { static String maybeTransformAbsoluteUri( String path, Function absoluteUriTransformer) throws URISyntaxException { - if (isValidHttp2Path(path)) { + if (ArmeriaHttpUtil.isValidHttp2Path(path)) { return path; } @@ -200,7 +200,7 @@ static String maybeTransformAbsoluteUri( throw newInvalidPathException(path); } - if (!isValidHttp2Path(newPath)) { + if (!ArmeriaHttpUtil.isValidHttp2Path(newPath)) { throw newInvalidPathException(path); } @@ -225,10 +225,5 @@ private static URISyntaxException newInvalidPathException(String path) { return new URISyntaxException(path, "neither origin form nor asterisk form"); } - private static boolean isValidHttp2Path(String path) { - // We support only origin form and asterisk form. - return path.charAt(0) == '/' || "*".equals(path); - } - private HttpHeaderUtil() {} } diff --git a/core/src/main/java/com/linecorp/armeria/server/HttpServerHandler.java b/core/src/main/java/com/linecorp/armeria/server/HttpServerHandler.java index 6ee027e5ec22..f7a86258c62a 100644 --- a/core/src/main/java/com/linecorp/armeria/server/HttpServerHandler.java +++ b/core/src/main/java/com/linecorp/armeria/server/HttpServerHandler.java @@ -63,8 +63,8 @@ import com.linecorp.armeria.common.util.SystemInfo; import com.linecorp.armeria.internal.common.AbstractHttp2ConnectionHandler; import com.linecorp.armeria.internal.common.Http1ObjectEncoder; -import com.linecorp.armeria.internal.common.PathAndQuery; import com.linecorp.armeria.internal.common.RequestContextUtil; +import com.linecorp.armeria.internal.common.RequestTargetCache; import com.linecorp.armeria.internal.server.DefaultServiceRequestContext; import io.netty.buffer.Unpooled; @@ -328,17 +328,14 @@ private void handleRequest(ChannelHandlerContext ctx, DecodedHttpRequest req) th if (!routingStatus.routeMustExist()) { final ServiceRequestContext reqCtx = newEarlyRespondingRequestContext(channel, req, proxiedAddresses, clientAddress, routingCtx); - switch (routingStatus) { - case OPTIONS: - // Handle 'OPTIONS * HTTP/1.1'. - handleOptions(ctx, reqCtx); - return; - case INVALID_PATH: - rejectInvalidPath(ctx, reqCtx); - return; - default: - throw new Error(); // Should never reach here. + + // Handle 'OPTIONS * HTTP/1.1'. + if (routingStatus == RoutingStatus.OPTIONS) { + handleOptions(ctx, reqCtx); + return; } + + throw new Error(); // Should never reach here. } // Find the service that matches the path. @@ -395,9 +392,7 @@ private void handleRequest(ChannelHandlerContext ctx, DecodedHttpRequest req) th reqCtx.log().whenComplete().thenAccept(log -> { final int statusCode = log.responseHeaders().status().code(); if (statusCode >= 200 && statusCode < 400 && routingCtx instanceof DefaultRoutingContext) { - final PathAndQuery pathAndQuery = ((DefaultRoutingContext) routingCtx).pathAndQuery(); - assert pathAndQuery != null; - pathAndQuery.storeInCache(req.path()); + RequestTargetCache.putForServer(req.path(), routingCtx.requestTarget()); } }); } diff --git a/core/src/main/java/com/linecorp/armeria/server/RoutingContext.java b/core/src/main/java/com/linecorp/armeria/server/RoutingContext.java index 084192e7e30c..eb08575a1975 100644 --- a/core/src/main/java/com/linecorp/armeria/server/RoutingContext.java +++ b/core/src/main/java/com/linecorp/armeria/server/RoutingContext.java @@ -25,8 +25,11 @@ import com.linecorp.armeria.common.MediaType; import com.linecorp.armeria.common.QueryParams; import com.linecorp.armeria.common.RequestHeaders; +import com.linecorp.armeria.common.RequestTarget; +import com.linecorp.armeria.common.RequestTargetForm; import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.common.annotation.UnstableApi; +import com.linecorp.armeria.internal.common.DefaultRequestTarget; /** * Holds the parameters which are required to find a service available to handle the request. @@ -65,18 +68,29 @@ public HttpMethod method() { }; } + /** + * Returns the {@link RequestTarget} of the request. The form of the returned {@link RequestTarget} is + * never {@link RequestTargetForm#ABSOLUTE}, which means it is always {@link RequestTargetForm#ORIGIN} or + * {@link RequestTargetForm#ASTERISK}. + */ + RequestTarget requestTarget(); + /** * Returns the absolute path retrieved from the request, * as defined in RFC3986. */ - String path(); + default String path() { + return requestTarget().path(); + } /** * Returns the query retrieved from the request, * as defined in RFC3986. */ @Nullable - String query(); + default String query() { + return requestTarget().query(); + } /** * Returns the query parameters retrieved from the request path. @@ -120,18 +134,26 @@ public HttpMethod method() { HttpStatusException deferredStatusException(); /** - * Returns a wrapped {@link RoutingContext} which holds the specified {@code path}. + * (Advanced users only) Returns a wrapped {@link RoutingContext} which holds the specified {@code path}. * It is usually used to find an {@link HttpService} with a prefix-stripped path. + * Note that specifying a malformed or relative path will lead to unspecified behavior. */ default RoutingContext withPath(String path) { requireNonNull(path, "path"); - if (path.equals(path())) { - return this; - } + final RequestTarget oldReqTarget = requestTarget(); + final RequestTarget newReqTarget = + DefaultRequestTarget.createWithoutValidation( + oldReqTarget.form(), + oldReqTarget.scheme(), + oldReqTarget.authority(), + path, + oldReqTarget.query(), + oldReqTarget.fragment()); + return new RoutingContextWrapper(this) { @Override - public String path() { - return path; + public RequestTarget requestTarget() { + return newReqTarget; } }; } @@ -140,7 +162,7 @@ public String path() { * Returns a wrapped {@link RoutingContext} which holds the specified {@code path}. * It is usually used to find an {@link HttpService} with a prefix-stripped path. * - * @deprecated Use {@link #withPath}. + * @deprecated Use {@link #withPath(String)}. */ @Deprecated default RoutingContext overridePath(String path) { diff --git a/core/src/main/java/com/linecorp/armeria/server/RoutingContextWrapper.java b/core/src/main/java/com/linecorp/armeria/server/RoutingContextWrapper.java index 10a01d68feef..f8a32fc06e66 100644 --- a/core/src/main/java/com/linecorp/armeria/server/RoutingContextWrapper.java +++ b/core/src/main/java/com/linecorp/armeria/server/RoutingContextWrapper.java @@ -22,6 +22,7 @@ import com.linecorp.armeria.common.MediaType; import com.linecorp.armeria.common.QueryParams; import com.linecorp.armeria.common.RequestHeaders; +import com.linecorp.armeria.common.RequestTarget; import com.linecorp.armeria.common.annotation.Nullable; class RoutingContextWrapper implements RoutingContext { @@ -51,14 +52,19 @@ public HttpMethod method() { } @Override - public String path() { - return delegate.path(); + public RequestTarget requestTarget() { + return delegate.requestTarget(); + } + + @Override + public final String path() { + return RoutingContext.super.path(); } @Nullable @Override - public String query() { - return delegate.query(); + public final String query() { + return RoutingContext.super.query(); } @Override @@ -103,6 +109,7 @@ public RoutingContext withPath(String path) { } @Override + @Deprecated public boolean isCorsPreflight() { return delegate.isCorsPreflight(); } diff --git a/core/src/main/java/com/linecorp/armeria/server/RoutingStatus.java b/core/src/main/java/com/linecorp/armeria/server/RoutingStatus.java index 41b2c7768848..08e49d07c7cd 100644 --- a/core/src/main/java/com/linecorp/armeria/server/RoutingStatus.java +++ b/core/src/main/java/com/linecorp/armeria/server/RoutingStatus.java @@ -36,12 +36,7 @@ public enum RoutingStatus { /** * An {@code "OPTIONS * HTTP/1.1"} request. */ - OPTIONS(false), - - /** - * The request specified an invalid path. - */ - INVALID_PATH(false); + OPTIONS(false); private final boolean routeMustExist; diff --git a/core/src/main/java/com/linecorp/armeria/server/Server.java b/core/src/main/java/com/linecorp/armeria/server/Server.java index 2698b030eb97..0940239a0674 100644 --- a/core/src/main/java/com/linecorp/armeria/server/Server.java +++ b/core/src/main/java/com/linecorp/armeria/server/Server.java @@ -69,14 +69,13 @@ import com.linecorp.armeria.common.Flags; import com.linecorp.armeria.common.SessionProtocol; import com.linecorp.armeria.common.annotation.Nullable; -import com.linecorp.armeria.common.metric.MeterIdPrefix; import com.linecorp.armeria.common.util.EventLoopGroups; import com.linecorp.armeria.common.util.Exceptions; import com.linecorp.armeria.common.util.ListenableAsyncCloseable; import com.linecorp.armeria.common.util.ShutdownHooks; import com.linecorp.armeria.common.util.StartStopSupport; import com.linecorp.armeria.common.util.Version; -import com.linecorp.armeria.internal.common.PathAndQuery; +import com.linecorp.armeria.internal.common.RequestTargetCache; import com.linecorp.armeria.internal.common.util.ChannelUtil; import io.micrometer.core.instrument.Gauge; @@ -130,10 +129,8 @@ public static ServerBuilder builder() { startStop = new ServerStartStopSupport(config.startStopExecutor()); connectionLimitingHandler = new ConnectionLimitingHandler(config.maxNumConnections()); - // Server-wide cache metrics. - final MeterIdPrefix idPrefix = new MeterIdPrefix("armeria.server.parsed.path.cache"); - PathAndQuery.registerMetrics(config.meterRegistry(), idPrefix); - + // Server-wide metrics. + RequestTargetCache.registerServerMetrics(config.meterRegistry()); setupVersionMetrics(); for (VirtualHost virtualHost : config().virtualHosts()) { diff --git a/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java b/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java index 14683bbfbc7b..0e76560bc972 100644 --- a/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java @@ -210,7 +210,7 @@ public ServiceRequestContext build() { if (route != null) { serviceBindingBuilder = serverBuilder.route().addRoute(route); } else { - serviceBindingBuilder = serverBuilder.route().path(path()); + serviceBindingBuilder = serverBuilder.route().path(requestTarget().path()); } if (defaultServiceNaming != null) { @@ -236,15 +236,17 @@ public ServiceRequestContext build() { final RoutingContext routingCtx = DefaultRoutingContext.of( server.config().defaultVirtualHost(), ((InetSocketAddress) localAddress()).getHostString(), - path(), - query(), + requestTarget(), req.headers(), RoutingStatus.OK); final RoutingResult routingResult = this.routingResult != null ? this.routingResult - : RoutingResult.builder().path(path()).query(query()).build(); - final Route route = Route.builder().path(path()).build(); + : RoutingResult.builder() + .path(requestTarget().path()) + .query(requestTarget().query()) + .build(); + final Route route = Route.builder().path(requestTarget().path()).build(); final Routed routed = Routed.of(route, routingResult, serviceCfg); routingCtx.setResult(routed); final ExchangeType exchangeType = service.exchangeType(routingCtx); diff --git a/core/src/main/java/com/linecorp/armeria/server/ServiceRouteUtil.java b/core/src/main/java/com/linecorp/armeria/server/ServiceRouteUtil.java index c1e037331554..1b5f5b6ab96b 100644 --- a/core/src/main/java/com/linecorp/armeria/server/ServiceRouteUtil.java +++ b/core/src/main/java/com/linecorp/armeria/server/ServiceRouteUtil.java @@ -22,43 +22,34 @@ import com.linecorp.armeria.common.HttpMethod; import com.linecorp.armeria.common.RequestHeaders; -import com.linecorp.armeria.common.annotation.Nullable; -import com.linecorp.armeria.internal.common.PathAndQuery; +import com.linecorp.armeria.common.RequestTarget; import io.netty.channel.Channel; final class ServiceRouteUtil { static RoutingContext newRoutingContext(ServerConfig serverConfig, Channel channel, - RequestHeaders headers, - @Nullable PathAndQuery pathAndQuery) { + RequestHeaders headers, RequestTarget reqTarget) { final String hostname = hostname(headers); final int port = ((InetSocketAddress) channel.localAddress()).getPort(); final String originalPath = headers.path(); final RoutingStatus routingStatus; - if (pathAndQuery != null) { + if (headers.method() == HttpMethod.OPTIONS) { if (isCorsPreflightRequest(headers)) { routingStatus = RoutingStatus.CORS_PREFLIGHT; + } else if ("*".equals(originalPath)) { + routingStatus = RoutingStatus.OPTIONS; } else { routingStatus = RoutingStatus.OK; } } else { - if (headers.method() == HttpMethod.OPTIONS && "*".equals(originalPath)) { - routingStatus = RoutingStatus.OPTIONS; - } else { - routingStatus = RoutingStatus.INVALID_PATH; - } + routingStatus = RoutingStatus.OK; } final VirtualHost virtualHost = serverConfig.findVirtualHost(hostname, port); - if (pathAndQuery == null) { - return DefaultRoutingContext.of(virtualHost, hostname, headers.path(), /* query */ null, headers, - routingStatus); - } else { - return DefaultRoutingContext.of(virtualHost, hostname, pathAndQuery, headers, routingStatus); - } + return DefaultRoutingContext.of(virtualHost, hostname, reqTarget, headers, routingStatus); } private static String hostname(RequestHeaders headers) { diff --git a/core/src/main/java/com/linecorp/armeria/server/docs/MethodInfo.java b/core/src/main/java/com/linecorp/armeria/server/docs/MethodInfo.java index 3bcc7be30f42..b25cdf09c091 100644 --- a/core/src/main/java/com/linecorp/armeria/server/docs/MethodInfo.java +++ b/core/src/main/java/com/linecorp/armeria/server/docs/MethodInfo.java @@ -33,9 +33,9 @@ import com.linecorp.armeria.common.HttpHeaders; import com.linecorp.armeria.common.HttpMethod; +import com.linecorp.armeria.common.RequestTarget; import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.common.annotation.UnstableApi; -import com.linecorp.armeria.internal.common.PathAndQuery; import com.linecorp.armeria.server.Service; /** @@ -147,9 +147,9 @@ public MethodInfo(String serviceName, String name, final ImmutableList.Builder examplePathsBuilder = ImmutableList.builderWithExpectedSize(Iterables.size(examplePaths)); for (String path : examplePaths) { - final PathAndQuery pathAndQuery = PathAndQuery.parse(path); - checkArgument(pathAndQuery != null, "examplePaths contains an invalid path: %s", path); - examplePathsBuilder.add(pathAndQuery.path()); + final RequestTarget reqTarget = RequestTarget.forServer(path); + checkArgument(reqTarget != null, "examplePaths contains an invalid path: %s", path); + examplePathsBuilder.add(reqTarget.path()); } this.examplePaths = examplePathsBuilder.build(); @@ -157,9 +157,9 @@ public MethodInfo(String serviceName, String name, final ImmutableList.Builder exampleQueriesBuilder = ImmutableList.builderWithExpectedSize(Iterables.size(exampleQueries)); for (String query : exampleQueries) { - final PathAndQuery pathAndQuery = PathAndQuery.parse('?' + query); - checkArgument(pathAndQuery != null, "exampleQueries contains an invalid query string: %s", query); - exampleQueriesBuilder.add(pathAndQuery.query()); + final RequestTarget reqTarget = RequestTarget.forServer('?' + query); + checkArgument(reqTarget != null, "exampleQueries contains an invalid query string: %s", query); + exampleQueriesBuilder.add(reqTarget.query()); } this.exampleQueries = exampleQueriesBuilder.build(); diff --git a/core/src/test/java/com/linecorp/armeria/client/HttpClientContextCaptorTest.java b/core/src/test/java/com/linecorp/armeria/client/HttpClientContextCaptorTest.java index 4103b2c0e437..24dab2a9fa9b 100644 --- a/core/src/test/java/com/linecorp/armeria/client/HttpClientContextCaptorTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/HttpClientContextCaptorTest.java @@ -62,7 +62,7 @@ void connectionRefused() { void badPath() { try (ClientRequestContextCaptor ctxCaptor = Clients.newContextCaptor()) { // Send a request with a bad path. - final HttpResponse res = WebClient.of().get("http://127.0.0.1:1/|"); + final HttpResponse res = WebClient.of().get("http://127.0.0.1:1/%%"); assertThatThrownBy(ctxCaptor::get).isInstanceOf(NoSuchElementException.class) .hasMessageContaining("no request was made"); res.aggregate(); diff --git a/core/src/test/java/com/linecorp/armeria/internal/client/DefaultClientRequestContextTest.java b/core/src/test/java/com/linecorp/armeria/internal/client/DefaultClientRequestContextTest.java index b98989c2fe24..d576941f7936 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/client/DefaultClientRequestContextTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/client/DefaultClientRequestContextTest.java @@ -43,8 +43,8 @@ import com.linecorp.armeria.common.HttpRequest; import com.linecorp.armeria.common.RequestHeaders; import com.linecorp.armeria.common.RequestId; +import com.linecorp.armeria.common.RequestTarget; import com.linecorp.armeria.common.SessionProtocol; -import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.common.metric.NoopMeterRegistry; import com.linecorp.armeria.common.util.SafeCloseable; import com.linecorp.armeria.common.util.SystemInfo; @@ -276,10 +276,10 @@ void requestUpdateAllComponents() { @Test void uriIncludesAllComponents() { final HttpRequest request = HttpRequest.of(RequestHeaders.of( - HttpMethod.POST, "https://path.com/a/b/c?q1=p1&q2=p2", + HttpMethod.POST, "https://path.com/a/b/c?q1=p1&q2=p2#fragment1", HttpHeaderNames.SCHEME, "http", HttpHeaderNames.AUTHORITY, "request.com")); - final DefaultClientRequestContext ctx = newContext(ClientOptions.of(), request, "fragment1"); + final DefaultClientRequestContext ctx = newContext(ClientOptions.of(), request); ctx.updateRequest(request); assertThat(ctx.uri().toString()).isEqualTo("https://request.com/a/b/c?q1=p1&q2=p2#fragment1"); assertThat(ctx.endpoint().authority()).isEqualTo("path.com"); @@ -307,16 +307,12 @@ private static DefaultClientRequestContext newContext() { private static DefaultClientRequestContext newContext(ClientOptions clientOptions, HttpRequest httpRequest) { - return newContext(clientOptions, httpRequest, null); - } + final RequestTarget reqTarget = RequestTarget.forClient(httpRequest.path()); + assertThat(reqTarget).isNotNull(); - private static DefaultClientRequestContext newContext(ClientOptions clientOptions, - HttpRequest httpRequest, - @Nullable String fragment) { return new DefaultClientRequestContext( mock(EventLoop.class), NoopMeterRegistry.get(), SessionProtocol.H2C, - RequestId.random(), HttpMethod.POST, "/foo", null, fragment, - clientOptions, httpRequest, + RequestId.random(), HttpMethod.POST, reqTarget, clientOptions, httpRequest, null, RequestOptions.of(), new CancellationScheduler(0), System.nanoTime(), SystemInfo.currentTimeMicros()); } diff --git a/core/src/test/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtilTest.java b/core/src/test/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtilTest.java index cd7f2736d8f6..64d523f21460 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtilTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtilTest.java @@ -23,6 +23,7 @@ import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.toNettyHttp1ServerHeaders; import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.toNettyHttp2ClientHeaders; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -49,6 +50,7 @@ import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.MediaType; import com.linecorp.armeria.common.RequestHeaders; +import com.linecorp.armeria.common.RequestTarget; import com.linecorp.armeria.common.ResponseHeaders; import com.linecorp.armeria.common.ResponseHeadersBuilder; import com.linecorp.armeria.server.Server; @@ -70,12 +72,6 @@ class ArmeriaHttpUtilTest { @Test void testConcatPaths() throws Exception { - assertThat(concatPaths(null, "a")).isEqualTo("/a"); - assertThat(concatPaths(null, "/a")).isEqualTo("/a"); - - assertThat(concatPaths("", "a")).isEqualTo("/a"); - assertThat(concatPaths("", "/a")).isEqualTo("/a"); - assertThat(concatPaths("/", "a")).isEqualTo("/a"); assertThat(concatPaths("/", "/a")).isEqualTo("/a"); assertThat(concatPaths("/", "/")).isEqualTo("/"); @@ -88,6 +84,11 @@ void testConcatPaths() throws Exception { assertThat(concatPaths("/a/", "")).isEqualTo("/a/"); assertThat(concatPaths("/a", "?foo=bar")).isEqualTo("/a?foo=bar"); assertThat(concatPaths("/a/", "?foo=bar")).isEqualTo("/a/?foo=bar"); + + // Bad prefixes + assertThatThrownBy(() -> concatPaths(null, "a")).isInstanceOf(NullPointerException.class); + assertThatThrownBy(() -> concatPaths("", "b")).isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> concatPaths("relative", "c")).isInstanceOf(IllegalArgumentException.class); } @ParameterizedTest @@ -260,9 +261,9 @@ void inboundCookiesMustBeMergedForHttp2() { in.add(HttpHeaderNames.COOKIE, "i=j"); in.add(HttpHeaderNames.COOKIE, "k=l;"); - final PathAndQuery pathAndQuery = PathAndQuery.parse(in.path().toString()); + final RequestTarget reqTarget = RequestTarget.forServer(in.path().toString()); final RequestHeaders out = ArmeriaHttpUtil.toArmeriaRequestHeaders( - null, in, false, "http", null, pathAndQuery); + null, in, false, "http", null, reqTarget); assertThat(out.getAll(HttpHeaderNames.COOKIE)) .containsExactly("a=b; c=d; e=f;g=h; i=j; k=l;"); @@ -279,7 +280,7 @@ void addHostHeaderIfMissing() throws URISyntaxException { final ChannelHandlerContext ctx = mockChannelHandlerContext(); RequestHeaders armeriaHeaders = toArmeria(ctx, originReq, serverConfig(), "http", - PathAndQuery.parse(originReq.uri())); + RequestTarget.forServer(originReq.uri())); assertThat(armeriaHeaders.get(HttpHeaderNames.HOST)).isEqualTo("bar"); assertThat(armeriaHeaders.authority()).isEqualTo("bar"); assertThat(armeriaHeaders.scheme()).isEqualTo("http"); @@ -288,7 +289,7 @@ void addHostHeaderIfMissing() throws URISyntaxException { // Remove Host header. headers.remove(HttpHeaderNames.HOST); armeriaHeaders = toArmeria(ctx, originReq, serverConfig(), "https", - PathAndQuery.parse(originReq.uri())); + RequestTarget.forServer(originReq.uri())); assertThat(armeriaHeaders.get(HttpHeaderNames.HOST)).isEqualTo("foo:36462"); // The default hostname. assertThat(armeriaHeaders.authority()).isEqualTo("foo:36462"); assertThat(armeriaHeaders.scheme()).isEqualTo("https"); @@ -523,10 +524,10 @@ void toArmeriaRequestHeaders() { in.set(HttpHeaderNames.METHOD, "GET") .set(HttpHeaderNames.PATH, "/"); // Request headers without pseudo headers. - final PathAndQuery pathAndQuery = PathAndQuery.parse(in.path().toString()); + final RequestTarget reqTarget = RequestTarget.forServer(in.path().toString()); final RequestHeaders headers = ArmeriaHttpUtil.toArmeriaRequestHeaders(ctx, in, false, "https", - serverConfig(), pathAndQuery); + serverConfig(), reqTarget); assertThat(headers.scheme()).isEqualTo("https"); assertThat(headers.authority()).isEqualTo("foo:36462"); } diff --git a/core/src/test/java/com/linecorp/armeria/internal/common/PathAndQueryTest.java b/core/src/test/java/com/linecorp/armeria/internal/common/DefaultRequestTargetTest.java similarity index 86% rename from core/src/test/java/com/linecorp/armeria/internal/common/PathAndQueryTest.java rename to core/src/test/java/com/linecorp/armeria/internal/common/DefaultRequestTargetTest.java index f0f8b1647ce8..4e55f9a4d603 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/common/PathAndQueryTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/common/DefaultRequestTargetTest.java @@ -15,11 +15,13 @@ */ package com.linecorp.armeria.internal.common; -import static com.linecorp.armeria.internal.common.PathAndQuery.decodePercentEncodedQuery; +import static com.linecorp.armeria.internal.common.DefaultRequestTarget.decodePercentEncodedQuery; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.util.Set; +import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -29,11 +31,12 @@ import com.google.common.collect.Maps; import com.linecorp.armeria.common.QueryParams; +import com.linecorp.armeria.common.RequestTarget; import com.linecorp.armeria.common.annotation.Nullable; -class PathAndQueryTest { +class DefaultRequestTargetTest { - private static final Logger logger = LoggerFactory.getLogger(PathAndQueryTest.class); + private static final Logger logger = LoggerFactory.getLogger(DefaultRequestTargetTest.class); private static final Set QUERY_SEPARATORS = ImmutableSet.of("&", ";"); @@ -67,20 +70,18 @@ class PathAndQueryTest { @Test void empty() { - final PathAndQuery res = parse(null); + assertThatThrownBy(() -> RequestTarget.forServer(null)) + .isInstanceOf(NullPointerException.class); + + final RequestTarget res = parse(""); assertThat(res).isNotNull(); assertThat(res.path()).isEqualTo("/"); assertThat(res.query()).isNull(); - final PathAndQuery res2 = parse(""); + final RequestTarget res2 = parse("?"); assertThat(res2).isNotNull(); assertThat(res2.path()).isEqualTo("/"); - assertThat(res2.query()).isNull(); - - final PathAndQuery res3 = parse("?"); - assertThat(res3).isNotNull(); - assertThat(res3.path()).isEqualTo("/"); - assertThat(res3.query()).isEqualTo(""); + assertThat(res2.query()).isEqualTo(""); } @Test @@ -93,7 +94,7 @@ void doubleDotsInPath() { BAD_DOUBLE_DOT_PATTERNS.forEach(pattern -> assertProhibited(pattern)); GOOD_DOUBLE_DOT_PATTERNS.forEach(pattern -> { final String path = pattern.startsWith("/") ? pattern : '/' + pattern; - final PathAndQuery res = parse(path); + final RequestTarget res = parse(path); assertThat(res).as("Ensure %s is allowed.", path).isNotNull(); assertThat(res.path()).as("Ensure %s is parsed as-is.", path).isEqualTo(path); }); @@ -231,21 +232,21 @@ void allowDoubleDotsInNameValueQuery() { } /** - * {@link PathAndQuery} treats the first `=` in a query parameter as `/` internally to simplify + * {@link RequestTarget} treats the first `=` in a query parameter as `/` internally to simplify * the detection the logic. This test makes sure the `=` appeared later is not treated as `/`. */ @Test void dotsAndEqualsInNameValueQuery() { QUERY_SEPARATORS.forEach(qs -> { - final PathAndQuery res = parse("/?a=..=" + qs + "b=..="); + final RequestTarget res = parse("/?a=..=" + qs + "b=..="); assertThat(res).isNotNull(); assertThat(res.query()).isEqualTo("a=..=" + qs + "b=..="); - assertThat(QueryParams.fromQueryString(res.query(), true)).containsExactly( + Assertions.assertThat(QueryParams.fromQueryString(res.query(), true)).containsExactly( Maps.immutableEntry("a", "..="), Maps.immutableEntry("b", "..=") ); - final PathAndQuery res2 = parse("/?a==.." + qs + "b==.."); + final RequestTarget res2 = parse("/?a==.." + qs + "b==.."); assertThat(res2).isNotNull(); assertThat(res2.query()).isEqualTo("a==.." + qs + "b==.."); assertThat(QueryParams.fromQueryString(res2.query(), true)).containsExactly( @@ -253,7 +254,7 @@ void dotsAndEqualsInNameValueQuery() { Maps.immutableEntry("b", "=..") ); - final PathAndQuery res3 = parse("/?a==..=" + qs + "b==..="); + final RequestTarget res3 = parse("/?a==..=" + qs + "b==..="); assertThat(res3).isNotNull(); assertThat(res3.query()).isEqualTo("a==..=" + qs + "b==..="); assertThat(QueryParams.fromQueryString(res3.query(), true)).containsExactly( @@ -307,7 +308,7 @@ void controlChars() { @Test void percent() { - final PathAndQuery res = parse("/%25"); + final RequestTarget res = parse("/%25"); assertThat(res).isNotNull(); assertThat(res.path()).isEqualTo("/%25"); assertThat(res.query()).isNull(); @@ -315,20 +316,20 @@ void percent() { @Test void shouldNotDecodeSlash() { - final PathAndQuery res = parse("%2F?%2F"); + final RequestTarget res = parse("%2F?%2F"); // Do not accept a relative path. assertThat(res).isNull(); - final PathAndQuery res1 = parse("/%2F?%2F"); + final RequestTarget res1 = parse("/%2F?%2F"); assertThat(res1).isNotNull(); assertThat(res1.path()).isEqualTo("/%2F"); assertThat(res1.query()).isEqualTo("%2F"); - final PathAndQuery pathOnly = parse("/foo%2F"); + final RequestTarget pathOnly = parse("/foo%2F"); assertThat(pathOnly).isNotNull(); assertThat(pathOnly.path()).isEqualTo("/foo%2F"); assertThat(pathOnly.query()).isNull(); - final PathAndQuery queryOnly = parse("/?%2f=%2F"); + final RequestTarget queryOnly = parse("/?%2f=%2F"); assertThat(queryOnly).isNotNull(); assertThat(queryOnly.path()).isEqualTo("/"); assertThat(queryOnly.query()).isEqualTo("%2F=%2F"); @@ -336,14 +337,14 @@ void shouldNotDecodeSlash() { @Test void consecutiveSlashes() { - final PathAndQuery res = parse( + final RequestTarget res = parse( "/path//with///consecutive////slashes?/query//with///consecutive////slashes"); assertThat(res).isNotNull(); assertThat(res.path()).isEqualTo("/path/with/consecutive/slashes"); assertThat(res.query()).isEqualTo("/query//with///consecutive////slashes"); // Encoded slashes - final PathAndQuery res2 = parse( + final RequestTarget res2 = parse( "/path%2F/with/%2F/consecutive//%2F%2Fslashes?/query%2F/with/%2F/consecutive//%2F%2Fslashes"); assertThat(res2).isNotNull(); assertThat(res2.path()).isEqualTo("/path%2F/with/%2F/consecutive/%2F%2Fslashes"); @@ -361,13 +362,13 @@ void colon() { @Test void rawUnicode() { // 2- and 3-byte UTF-8 - final PathAndQuery res1 = parse("/\u00A2?\u20AC"); // ¢ and € + final RequestTarget res1 = parse("/\u00A2?\u20AC"); // ¢ and € assertThat(res1).isNotNull(); assertThat(res1.path()).isEqualTo("/%C2%A2"); assertThat(res1.query()).isEqualTo("%E2%82%AC"); // 4-byte UTF-8 - final PathAndQuery res2 = parse("/\uD800\uDF48"); // 𐍈 + final RequestTarget res2 = parse("/\uD800\uDF48"); // 𐍈 assertThat(res2).isNotNull(); assertThat(res2.path()).isEqualTo("/%F0%90%8D%88"); assertThat(res2.query()).isNull(); @@ -379,7 +380,7 @@ void rawUnicode() { void encodedUnicode() { final String encodedPath = "/%ec%95%88"; final String encodedQuery = "%eb%85%95"; - final PathAndQuery res = parse(encodedPath + '?' + encodedQuery); + final RequestTarget res = parse(encodedPath + '?' + encodedQuery); assertThat(res).isNotNull(); assertThat(res.path()).isEqualTo(Ascii.toUpperCase(encodedPath)); assertThat(res.query()).isEqualTo(Ascii.toUpperCase(encodedQuery)); @@ -387,7 +388,7 @@ void encodedUnicode() { @Test void noEncoding() { - final PathAndQuery res = parse("/a?b=c"); + final RequestTarget res = parse("/a?b=c"); assertThat(res).isNotNull(); assertThat(res.path()).isEqualTo("/a"); assertThat(res.query()).isEqualTo("b=c"); @@ -395,12 +396,12 @@ void noEncoding() { @Test void space() { - final PathAndQuery res = parse("/ ? "); + final RequestTarget res = parse("/ ? "); assertThat(res).isNotNull(); assertThat(res.path()).isEqualTo("/%20"); assertThat(res.query()).isEqualTo("+"); - final PathAndQuery res2 = parse("/%20?%20"); + final RequestTarget res2 = parse("/%20?%20"); assertThat(res2).isNotNull(); assertThat(res2.path()).isEqualTo("/%20"); assertThat(res2.query()).isEqualTo("+"); @@ -408,12 +409,12 @@ void space() { @Test void plus() { - final PathAndQuery res = parse("/+?a+b=c+d"); + final RequestTarget res = parse("/+?a+b=c+d"); assertThat(res).isNotNull(); assertThat(res.path()).isEqualTo("/+"); assertThat(res.query()).isEqualTo("a+b=c+d"); - final PathAndQuery res2 = parse("/%2b?a%2bb=c%2bd"); + final RequestTarget res2 = parse("/%2b?a%2bb=c%2bd"); assertThat(res2).isNotNull(); assertThat(res2.path()).isEqualTo("/+"); assertThat(res2.query()).isEqualTo("a%2Bb=c%2Bd"); @@ -421,13 +422,13 @@ void plus() { @Test void ampersand() { - final PathAndQuery res = parse("/&?a=1&a=2&b=3"); + final RequestTarget res = parse("/&?a=1&a=2&b=3"); assertThat(res).isNotNull(); assertThat(res.path()).isEqualTo("/&"); assertThat(res.query()).isEqualTo("a=1&a=2&b=3"); // '%26' in a query string should never be decoded into '&'. - final PathAndQuery res2 = parse("/%26?a=1%26a=2&b=3"); + final RequestTarget res2 = parse("/%26?a=1%26a=2&b=3"); assertThat(res2).isNotNull(); assertThat(res2.path()).isEqualTo("/&"); assertThat(res2.query()).isEqualTo("a=1%26a=2&b=3"); @@ -435,13 +436,13 @@ void ampersand() { @Test void semicolon() { - final PathAndQuery res = parse("/;?a=b;c=d"); + final RequestTarget res = parse("/;?a=b;c=d"); assertThat(res).isNotNull(); assertThat(res.path()).isEqualTo("/;"); assertThat(res.query()).isEqualTo("a=b;c=d"); // '%3B' in a query string should never be decoded into ';'. - final PathAndQuery res2 = parse("/%3b?a=b%3Bc=d"); + final RequestTarget res2 = parse("/%3b?a=b%3Bc=d"); assertThat(res2).isNotNull(); assertThat(res2.path()).isEqualTo("/;"); assertThat(res2.query()).isEqualTo("a=b%3Bc=d"); @@ -449,13 +450,13 @@ void semicolon() { @Test void equal() { - final PathAndQuery res = parse("/=?a=b=1"); + final RequestTarget res = parse("/=?a=b=1"); assertThat(res).isNotNull(); assertThat(res.path()).isEqualTo("/="); assertThat(res.query()).isEqualTo("a=b=1"); // '%3D' in a query string should never be decoded into '='. - final PathAndQuery res2 = parse("/%3D?a%3db=1"); + final RequestTarget res2 = parse("/%3D?a%3db=1"); assertThat(res2).isNotNull(); assertThat(res2.path()).isEqualTo("/="); assertThat(res2.query()).isEqualTo("a%3Db=1"); @@ -464,13 +465,13 @@ void equal() { @Test void sharp() { // '#' must be encoded into '%23'. - final PathAndQuery res = parse("/#?a=b#1"); + final RequestTarget res = parse("/#?a=b#1"); assertThat(res).isNotNull(); assertThat(res.path()).isEqualTo("/%23"); assertThat(res.query()).isEqualTo("a=b%231"); // '%23' should never be decoded into '#'. - final PathAndQuery res2 = parse("/%23?a=b%231"); + final RequestTarget res2 = parse("/%23?a=b%231"); assertThat(res2).isNotNull(); assertThat(res2.path()).isEqualTo("/%23"); assertThat(res2.query()).isEqualTo("a=b%231"); @@ -478,12 +479,12 @@ void sharp() { @Test void allReservedCharacters() { - final PathAndQuery res = parse("/#/:@!$&'()*+,;=?a=/#/:[]@!$&'()*+,;="); + final RequestTarget res = parse("/#/:@!$&'()*+,;=?a=/#/:[]@!$&'()*+,;="); assertThat(res).isNotNull(); assertThat(res.path()).isEqualTo("/%23/:@!$&'()*+,;="); assertThat(res.query()).isEqualTo("a=/%23/:[]@!$&'()*+,;="); - final PathAndQuery res2 = + final RequestTarget res2 = parse("/%23%2F%3A%40%21%24%26%27%28%29%2A%2B%2C%3B%3D%3F" + "?a=%23%2F%3A%5B%5D%40%21%24%26%27%28%29%2A%2B%2C%3B%3D%3F"); assertThat(res2).isNotNull(); @@ -493,7 +494,7 @@ void allReservedCharacters() { @Test void doubleQuote() { - final PathAndQuery res = parse("/\"?\""); + final RequestTarget res = parse("/\"?\""); assertThat(res).isNotNull(); assertThat(res.path()).isEqualTo("/%22"); assertThat(res.query()).isEqualTo("%22"); @@ -519,9 +520,9 @@ private static void assertQueryStringAllowed(String rawPath, String expectedQuer assertQueryStringAllowed(rawPath, expectedQuery, false); } - private static void assertQueryStringAllowed(String rawPath, String expectedQuery, + private static void assertQueryStringAllowed(String rawPath, @Nullable String expectedQuery, boolean allowDoubleDotsInQueryString) { - final PathAndQuery res = parse(rawPath, allowDoubleDotsInQueryString); + final RequestTarget res = parse(rawPath, allowDoubleDotsInQueryString); assertThat(res) .as("parse(\"%s\") must return non-null.", rawPath) .isNotNull(); @@ -531,13 +532,13 @@ private static void assertQueryStringAllowed(String rawPath, String expectedQuer } @Nullable - private static PathAndQuery parse(@Nullable String rawPath) { + private static RequestTarget parse(String rawPath) { return parse(rawPath, false); } @Nullable - private static PathAndQuery parse(@Nullable String rawPath, boolean allowDoubleDotsInQueryString) { - final PathAndQuery res = PathAndQuery.parse(rawPath, allowDoubleDotsInQueryString); + private static RequestTarget parse(String rawPath, boolean allowDoubleDotsInQueryString) { + final RequestTarget res = DefaultRequestTarget.forServer(rawPath, allowDoubleDotsInQueryString); if (res != null) { logger.info("parse({}) => path: {}, query: {}", rawPath, res.path(), res.query()); } else { @@ -548,11 +549,11 @@ private static PathAndQuery parse(@Nullable String rawPath, boolean allowDoubleD @Test void assertSquareBracketsInPath() { - final PathAndQuery res = parse("/@/:[]!$&'()*+,;="); + final RequestTarget res = parse("/@/:[]!$&'()*+,;="); assertThat(res).isNotNull(); assertThat(res.path()).isEqualTo("/@/:%5B%5D!$&'()*+,;="); - final PathAndQuery res2 = + final RequestTarget res2 = parse("/%40%2F%3A%5B%5D%21%24%26%27%28%29%2A%2B%2C%3B%3D%3F"); assertThat(res2).isNotNull(); assertThat(res2.path()).isEqualTo("/@%2F:%5B%5D!$&'()*+,;=?"); @@ -560,7 +561,7 @@ void assertSquareBracketsInPath() { @Test void toStringAppendsQueryCorrectly() { - PathAndQuery res = parse("/"); + RequestTarget res = parse("/"); assertThat(res.toString()).isEqualTo("/"); res = parse("/?"); diff --git a/core/src/test/java/com/linecorp/armeria/server/CachingRoutingContextTest.java b/core/src/test/java/com/linecorp/armeria/server/CachingRoutingContextTest.java index f59b84106178..02c23c1d08e1 100644 --- a/core/src/test/java/com/linecorp/armeria/server/CachingRoutingContextTest.java +++ b/core/src/test/java/com/linecorp/armeria/server/CachingRoutingContextTest.java @@ -18,14 +18,13 @@ import static com.linecorp.armeria.server.RoutingContextTest.virtualHost; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; import org.junit.jupiter.api.Test; import com.linecorp.armeria.common.HttpMethod; import com.linecorp.armeria.common.QueryParams; import com.linecorp.armeria.common.RequestHeaders; +import com.linecorp.armeria.common.RequestTarget; import com.linecorp.armeria.server.RouteCache.CachingRoutingContext; class CachingRoutingContextTest { @@ -39,13 +38,22 @@ void disableMatchingQueryParamsByCachingRoutingContext() { .matchesParams("foo=bar") .build(); - final RoutingContext context = mock(RoutingContext.class); - when(context.path()).thenReturn("/test"); - when(context.method()).thenReturn(HttpMethod.GET); - when(context.params()).thenReturn(QueryParams.of("foo", "qux")); - when(context.requiresMatchingParamsPredicates()).thenReturn(true); - when(context.virtualHost()).thenReturn(virtualHost); + final RequestTarget reqTarget = RequestTarget.forServer("/test?foo=qux"); + assertThat(reqTarget).isNotNull(); + final RoutingContext context = + new RoutingContextWrapper(DefaultRoutingContext.of( + virtualHost, virtualHost.defaultHostname(), + reqTarget, + RequestHeaders.of(HttpMethod.GET, reqTarget.pathAndQuery()), + RoutingStatus.OK)) { + @Override + public boolean requiresMatchingParamsPredicates() { + return true; + } + }; + + assertThat(context.params()).isEqualTo(QueryParams.of("foo", "qux")); assertThat(route.apply(context, false).isPresent()).isFalse(); // Because of the query parameters. final CachingRoutingContext cachingContext = new CachingRoutingContext(context); @@ -61,13 +69,23 @@ void disableMatchingHeadersByCachingRoutingContext() { .matchesHeaders("foo=bar") .build(); - final RoutingContext context = mock(RoutingContext.class); - when(context.path()).thenReturn("/test"); - when(context.method()).thenReturn(HttpMethod.GET); - when(context.headers()).thenReturn(RequestHeaders.of(HttpMethod.GET, "/test", "foo", "qux")); - when(context.requiresMatchingHeadersPredicates()).thenReturn(true); - when(context.virtualHost()).thenReturn(virtualHost); + final RequestTarget reqTarget = RequestTarget.forServer("/test"); + assertThat(reqTarget).isNotNull(); + + final RoutingContext context = + new RoutingContextWrapper(DefaultRoutingContext.of( + virtualHost, virtualHost.defaultHostname(), + reqTarget, + RequestHeaders.of(HttpMethod.GET, reqTarget.pathAndQuery(), + "foo", "qux"), + RoutingStatus.OK)) { + @Override + public boolean requiresMatchingHeadersPredicates() { + return true; + } + }; + assertThat(context.headers().contains("foo", "qux")).isTrue(); assertThat(route.apply(context, false).isPresent()).isFalse(); // Because of HTTP headers. final CachingRoutingContext cachingContext = new CachingRoutingContext(context); diff --git a/core/src/test/java/com/linecorp/armeria/server/GlobPathMappingTest.java b/core/src/test/java/com/linecorp/armeria/server/GlobPathMappingTest.java index bca7ddc7366a..ce36f50678ea 100644 --- a/core/src/test/java/com/linecorp/armeria/server/GlobPathMappingTest.java +++ b/core/src/test/java/com/linecorp/armeria/server/GlobPathMappingTest.java @@ -18,7 +18,6 @@ import static com.linecorp.armeria.server.RoutingContextTest.create; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.fail; import org.junit.jupiter.api.Test; @@ -75,13 +74,6 @@ void testRelativePattern() { mustFail("bar/baz", "/bar/baz/", "/foo/bar/baz/", "/foo/bar/baz/quo"); } - @Test - void testPathValidation() { - final Route route = glob("**"); - assertThatThrownBy(() -> route.apply(create("not/an/absolute/path"), false)) - .isInstanceOf(IllegalArgumentException.class); - } - @Test void params() throws Exception { Route route = glob("baz"); diff --git a/core/src/test/java/com/linecorp/armeria/server/HttpServerTest.java b/core/src/test/java/com/linecorp/armeria/server/HttpServerTest.java index 9426eab678b3..c57e4cd3242e 100644 --- a/core/src/test/java/com/linecorp/armeria/server/HttpServerTest.java +++ b/core/src/test/java/com/linecorp/armeria/server/HttpServerTest.java @@ -86,7 +86,7 @@ import com.linecorp.armeria.common.stream.ClosedStreamException; import com.linecorp.armeria.common.util.EventLoopGroups; import com.linecorp.armeria.common.util.TimeoutMode; -import com.linecorp.armeria.internal.common.PathAndQuery; +import com.linecorp.armeria.internal.common.RequestTargetCache; import com.linecorp.armeria.server.encoding.EncodingService; import com.linecorp.armeria.testing.junit5.server.ServerExtension; @@ -457,7 +457,7 @@ void resetOptions() { serverMaxRequestLength = MAX_CONTENT_LENGTH; clientMaxResponseLength = MAX_CONTENT_LENGTH; - PathAndQuery.clearCachedPaths(); + RequestTargetCache.clearCachedPaths(); } @AfterEach @@ -899,7 +899,7 @@ void testTrailers(WebClient client) throws Exception { void testExactPathCached(WebClient client) throws Exception { assertThat(client.get("/cached-exact-path") .aggregate().get().status()).isEqualTo(HttpStatus.OK); - assertThat(PathAndQuery.cachedPaths()).contains("/cached-exact-path"); + assertThat(RequestTargetCache.cachedServerPaths()).contains("/cached-exact-path"); } @ParameterizedTest @@ -907,7 +907,7 @@ void testExactPathCached(WebClient client) throws Exception { void testPrefixPathNotCached(WebClient client) throws Exception { assertThat(client.get("/not-cached-paths/hoge") .aggregate().get().status()).isEqualTo(HttpStatus.OK); - assertThat(PathAndQuery.cachedPaths()).doesNotContain("/not-cached-paths/hoge"); + assertThat(RequestTargetCache.cachedServerPaths()).doesNotContain("/not-cached-paths/hoge"); } @ParameterizedTest @@ -915,7 +915,7 @@ void testPrefixPathNotCached(WebClient client) throws Exception { void testPrefixPath_cacheForced(WebClient client) throws Exception { assertThat(client.get("/cached-paths/hoge") .aggregate().get().status()).isEqualTo(HttpStatus.OK); - assertThat(PathAndQuery.cachedPaths()).contains("/cached-paths/hoge"); + assertThat(RequestTargetCache.cachedServerPaths()).contains("/cached-paths/hoge"); } @ParameterizedTest diff --git a/core/src/test/java/com/linecorp/armeria/server/RouteTest.java b/core/src/test/java/com/linecorp/armeria/server/RouteTest.java index 3b08326acc4d..1798e03cd0e9 100644 --- a/core/src/test/java/com/linecorp/armeria/server/RouteTest.java +++ b/core/src/test/java/com/linecorp/armeria/server/RouteTest.java @@ -30,11 +30,14 @@ import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.MediaType; import com.linecorp.armeria.common.RequestHeaders; +import com.linecorp.armeria.common.RequestTarget; class RouteTest { private static final String PATH = "/test"; + private static final RequestTarget REQ_TARGET = RequestTarget.forServer(PATH); + @Test void route() { Route route; @@ -407,30 +410,36 @@ void testRouteExclusionIsEvaluatedAtLast() { private static RoutingContext withMethod(HttpMethod method) { return DefaultRoutingContext.of(virtualHost(), "example.com", - PATH, null, RequestHeaders.of(method, PATH), RoutingStatus.OK); + REQ_TARGET, RequestHeaders.of(method, PATH), RoutingStatus.OK); } private static RoutingContext withConsumeType(HttpMethod method, MediaType contentType) { final RequestHeaders headers = RequestHeaders.of(method, PATH, HttpHeaderNames.CONTENT_TYPE, contentType); - return DefaultRoutingContext.of(virtualHost(), "example.com", PATH, null, headers, RoutingStatus.OK); + return DefaultRoutingContext.of(virtualHost(), "example.com", + REQ_TARGET, headers, RoutingStatus.OK); } private static RoutingContext withAcceptHeader(HttpMethod method, String acceptHeader) { final RequestHeaders headers = RequestHeaders.of(method, PATH, HttpHeaderNames.ACCEPT, acceptHeader); - return DefaultRoutingContext.of(virtualHost(), "example.com", PATH, null, headers, RoutingStatus.OK); + return DefaultRoutingContext.of(virtualHost(), "example.com", + REQ_TARGET, headers, RoutingStatus.OK); } private static RoutingContext withPath(String path) { + final RequestTarget reqTarget = RequestTarget.forServer(path); + assertThat(reqTarget).isNotNull(); + return DefaultRoutingContext.of(virtualHost(), "example.com", - path, null, RequestHeaders.of(HttpMethod.GET, path), RoutingStatus.OK); + reqTarget, RequestHeaders.of(HttpMethod.GET, path), + RoutingStatus.OK); } private static RoutingContext withRequestHeaders(RequestHeaders headers) { - final String[] pathAndQuery = headers.path().split("\\?", 2); + final RequestTarget reqTarget = RequestTarget.forServer(headers.path()); + assertThat(reqTarget).isNotNull(); return DefaultRoutingContext.of(virtualHost(), "example.com", - pathAndQuery[0], pathAndQuery.length == 2 ? pathAndQuery[1] : null, - headers, RoutingStatus.OK); + reqTarget, headers, RoutingStatus.OK); } } diff --git a/core/src/test/java/com/linecorp/armeria/server/RouterTest.java b/core/src/test/java/com/linecorp/armeria/server/RouterTest.java index 01472989b8ad..772104878f49 100644 --- a/core/src/test/java/com/linecorp/armeria/server/RouterTest.java +++ b/core/src/test/java/com/linecorp/armeria/server/RouterTest.java @@ -44,6 +44,7 @@ import com.linecorp.armeria.common.HttpMethod; import com.linecorp.armeria.common.MediaType; import com.linecorp.armeria.common.RequestHeaders; +import com.linecorp.armeria.common.RequestTarget; class RouterTest { private static final Logger logger = LoggerFactory.getLogger(RouterTest.class); @@ -118,7 +119,7 @@ void testFindAllMatchedRouters(String path, int expectForFind, List exp private static DefaultRoutingContext routingCtx(String path) { return new DefaultRoutingContext(virtualHost(), "example.com", RequestHeaders.of(HttpMethod.GET, path), - path, null, null, RoutingStatus.OK); + RequestTarget.forServer(path), RoutingStatus.OK); } static Stream generateRouteMatchData() { diff --git a/core/src/test/java/com/linecorp/armeria/server/RoutingContextTest.java b/core/src/test/java/com/linecorp/armeria/server/RoutingContextTest.java index 2994a3905228..a8b81583fdf1 100644 --- a/core/src/test/java/com/linecorp/armeria/server/RoutingContextTest.java +++ b/core/src/test/java/com/linecorp/armeria/server/RoutingContextTest.java @@ -27,6 +27,7 @@ import com.linecorp.armeria.common.HttpMethod; import com.linecorp.armeria.common.MediaType; import com.linecorp.armeria.common.RequestHeaders; +import com.linecorp.armeria.common.RequestTarget; import com.linecorp.armeria.common.annotation.Nullable; class RoutingContextTest { @@ -50,6 +51,8 @@ void testAcceptTypes() { @Test void testEquals() { final VirtualHost virtualHost = virtualHost(); + final RequestTarget reqTarget = RequestTarget.forServer("/hello"); + assertThat(reqTarget).isNotNull(); final RoutingContext ctx1 = new DefaultRoutingContext(virtualHost, "example.com", @@ -58,7 +61,7 @@ void testEquals() { HttpHeaderNames.ACCEPT, MediaType.JSON_UTF_8 + ", " + MediaType.XML_UTF_8 + "; q=0.8"), - "/hello", null, null, RoutingStatus.OK); + reqTarget, RoutingStatus.OK); final RoutingContext ctx2 = new DefaultRoutingContext(virtualHost, "example.com", RequestHeaders.of(HttpMethod.GET, "/hello", @@ -66,7 +69,7 @@ void testEquals() { HttpHeaderNames.ACCEPT, MediaType.JSON_UTF_8 + ", " + MediaType.XML_UTF_8 + "; q=0.8"), - "/hello", null, null, RoutingStatus.OK); + reqTarget, RoutingStatus.OK); final RoutingContext ctx3 = new DefaultRoutingContext(virtualHost, "example.com", RequestHeaders.of(HttpMethod.GET, "/hello", @@ -74,7 +77,7 @@ void testEquals() { HttpHeaderNames.ACCEPT, MediaType.XML_UTF_8 + ", " + MediaType.JSON_UTF_8 + "; q=0.8"), - "/hello", null, null, RoutingStatus.OK); + reqTarget, RoutingStatus.OK); assertThat(ctx1.hashCode()).isEqualTo(ctx2.hashCode()); assertThat(ctx1).isEqualTo(ctx2); @@ -93,6 +96,9 @@ void queryDoesNotMatterWhenComparing() { @Test void hashcodeRecalculateWhenMethodChange() { final VirtualHost virtualHost = virtualHost(); + final RequestTarget reqTarget = RequestTarget.forServer("/hello"); + assertThat(reqTarget).isNotNull(); + final RoutingContext ctx1 = new DefaultRoutingContext(virtualHost, "example.com", RequestHeaders.of(HttpMethod.GET, "/hello", @@ -100,7 +106,7 @@ void hashcodeRecalculateWhenMethodChange() { HttpHeaderNames.ACCEPT, MediaType.JSON_UTF_8 + ", " + MediaType.XML_UTF_8 + "; q=0.8"), - "/hello", null, null, RoutingStatus.OK); + reqTarget, RoutingStatus.OK); final RoutingContext ctx2 = new DefaultRoutingContext(virtualHost, "example.com", RequestHeaders.of(HttpMethod.POST, "/hello", @@ -108,7 +114,7 @@ void hashcodeRecalculateWhenMethodChange() { HttpHeaderNames.ACCEPT, MediaType.JSON_UTF_8 + ", " + MediaType.XML_UTF_8 + "; q=0.8"), - "/hello", null, null, RoutingStatus.OK); + reqTarget, RoutingStatus.OK); final RoutingContext ctx3 = ctx1.withMethod(HttpMethod.POST); assertThat(ctx1.hashCode()).isNotEqualTo(ctx3.hashCode()); assertThat(ctx2.hashCode()).isEqualTo(ctx3.hashCode()); @@ -124,9 +130,12 @@ static RoutingContext create(String path, @Nullable String query) { static RoutingContext create(VirtualHost virtualHost, String path, @Nullable String query) { final String requestPath = query != null ? path + '?' + query : path; + final RequestTarget reqTarget = RequestTarget.forServer(requestPath); final RequestHeaders headers = RequestHeaders.of(HttpMethod.GET, requestPath); + assertThat(reqTarget).isNotNull(); + return DefaultRoutingContext.of(virtualHost, "example.com", - path, query, headers, RoutingStatus.OK); + reqTarget, headers, RoutingStatus.OK); } static VirtualHost virtualHost() { diff --git a/core/src/test/java/com/linecorp/armeria/server/ServiceRouteUtilTest.java b/core/src/test/java/com/linecorp/armeria/server/ServiceRouteUtilTest.java index 1145e7c3f9c3..16e6e9268c31 100644 --- a/core/src/test/java/com/linecorp/armeria/server/ServiceRouteUtilTest.java +++ b/core/src/test/java/com/linecorp/armeria/server/ServiceRouteUtilTest.java @@ -24,7 +24,7 @@ import com.linecorp.armeria.common.HttpMethod; import com.linecorp.armeria.common.HttpRequest; import com.linecorp.armeria.common.RequestHeaders; -import com.linecorp.armeria.internal.common.PathAndQuery; +import com.linecorp.armeria.common.RequestTarget; import io.netty.channel.Channel; @@ -39,7 +39,7 @@ void optionRequest() { .authority("foo.com") .build(); final RoutingContext routingContext = ServiceRouteUtil.newRoutingContext( - config, channel, headers, PathAndQuery.parse(headers.path())); + config, channel, headers, RequestTarget.forServer(headers.path())); assertThat(routingContext.status()).isEqualTo(RoutingStatus.OPTIONS); } @@ -49,20 +49,10 @@ void normalRequest() { .authority("foo.com") .build(); final RoutingContext routingContext = ServiceRouteUtil.newRoutingContext( - config, channel, headers, PathAndQuery.parse(headers.path())); + config, channel, headers, RequestTarget.forServer(headers.path())); assertThat(routingContext.status()).isEqualTo(RoutingStatus.OK); } - @Test - void invalidPath() { - final RequestHeaders headers = RequestHeaders.builder(HttpMethod.GET, "abc/def") - .authority("foo.com") - .build(); - final RoutingContext routingContext = ServiceRouteUtil.newRoutingContext( - config, channel, headers, PathAndQuery.parse(headers.path())); - assertThat(routingContext.status()).isEqualTo(RoutingStatus.INVALID_PATH); - } - @Test void cors() { final RequestHeaders headers = @@ -74,7 +64,7 @@ void cors() { "X-PINGOTHER, Content-Type") .build(); final RoutingContext routingContext = ServiceRouteUtil.newRoutingContext( - config, channel, headers, PathAndQuery.parse(headers.path())); + config, channel, headers, RequestTarget.forServer(headers.path())); assertThat(routingContext.status()).isEqualTo(RoutingStatus.CORS_PREFLIGHT); } } diff --git a/core/src/test/java/com/linecorp/armeria/server/VirtualHostBuilderTest.java b/core/src/test/java/com/linecorp/armeria/server/VirtualHostBuilderTest.java index 8cdf1dc29930..2ac1a4fcfc85 100644 --- a/core/src/test/java/com/linecorp/armeria/server/VirtualHostBuilderTest.java +++ b/core/src/test/java/com/linecorp/armeria/server/VirtualHostBuilderTest.java @@ -37,6 +37,7 @@ import com.linecorp.armeria.common.HttpMethod; import com.linecorp.armeria.common.HttpResponse; import com.linecorp.armeria.common.RequestHeaders; +import com.linecorp.armeria.common.RequestTarget; import io.netty.handler.ssl.SslContextBuilder; @@ -316,6 +317,9 @@ void virtualHostWithMismatch2() { void precedenceOfDuplicateRoute() { final Route routeA = Route.builder().path("/").build(); final Route routeB = Route.builder().path("/").build(); + final RequestTarget reqTarget = RequestTarget.forServer("/"); + assertThat(reqTarget).isNotNull(); + final VirtualHost virtualHost = new VirtualHostBuilder(Server.builder(), true) .service(routeA, (ctx, req) -> HttpResponse.of(OK)) .service(routeB, (ctx, req) -> HttpResponse.of(OK)) @@ -323,8 +327,7 @@ void precedenceOfDuplicateRoute() { assertThat(virtualHost.serviceConfigs().size()).isEqualTo(2); final RoutingContext routingContext = new DefaultRoutingContext(virtualHost(), "example.com", RequestHeaders.of(HttpMethod.GET, "/"), - "/", null, null, - RoutingStatus.OK); + reqTarget, RoutingStatus.OK); final Routed serviceConfig = virtualHost.findServiceConfig(routingContext); final Route route = serviceConfig.route(); assertThat(route).isSameAs(routeA); diff --git a/core/src/test/java/com/linecorp/armeria/server/file/FileServiceTest.java b/core/src/test/java/com/linecorp/armeria/server/file/FileServiceTest.java index 89401e5d5542..b94681ea4890 100644 --- a/core/src/test/java/com/linecorp/armeria/server/file/FileServiceTest.java +++ b/core/src/test/java/com/linecorp/armeria/server/file/FileServiceTest.java @@ -60,7 +60,7 @@ import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.common.util.OsType; import com.linecorp.armeria.common.util.SystemInfo; -import com.linecorp.armeria.internal.common.PathAndQuery; +import com.linecorp.armeria.internal.common.RequestTargetCache; import com.linecorp.armeria.server.ServerBuilder; import com.linecorp.armeria.server.logging.LoggingService; import com.linecorp.armeria.testing.junit5.server.ServerExtension; @@ -188,7 +188,7 @@ static void stopSynchronously() { @BeforeEach void setUp() { - PathAndQuery.clearCachedPaths(); + RequestTargetCache.clearCachedPaths(); } @ParameterizedTest @@ -207,7 +207,7 @@ void testClassPathGet(String baseUri) throws Exception { // Confirm file service paths are cached when cache is enabled. if (baseUri.contains("/cached")) { - assertThat(PathAndQuery.cachedPaths()).contains("/cached/foo.txt"); + assertThat(RequestTargetCache.cachedServerPaths()).contains("/cached/foo.txt"); } } } @@ -391,8 +391,7 @@ void testGetPreCompressedSupportsNone(String baseUri) throws Exception { assertThat(new String(content, StandardCharsets.UTF_8)).isEqualTo("foo"); // Confirm path not cached when cache disabled. - assertThat(PathAndQuery.cachedPaths()) - .doesNotContain("/compressed/foo.txt"); + assertThat(RequestTargetCache.cachedServerPaths()).doesNotContain("/compressed/foo.txt"); } } } @@ -411,8 +410,7 @@ void testGetWithoutPreCompression(String baseUri) throws Exception { assertThat(new String(content, StandardCharsets.UTF_8)).isEqualTo("foo_alone"); // Confirm path not cached when cache disabled. - assertThat(PathAndQuery.cachedPaths()) - .doesNotContain("/compressed/foo_alone.txt"); + assertThat(RequestTargetCache.cachedServerPaths()).doesNotContain("/compressed/foo_alone.txt"); } } } diff --git a/core/src/test/java12/com/linecorp/armeria/server/InvalidPathWithDataTest.java b/core/src/test/java12/com/linecorp/armeria/server/InvalidPathWithDataTest.java index 5360b1f4fe68..eba6abdbd0a2 100644 --- a/core/src/test/java12/com/linecorp/armeria/server/InvalidPathWithDataTest.java +++ b/core/src/test/java12/com/linecorp/armeria/server/InvalidPathWithDataTest.java @@ -28,7 +28,7 @@ import org.slf4j.LoggerFactory; import com.linecorp.armeria.common.HttpResponse; -import com.linecorp.armeria.internal.common.PathAndQuery; +import com.linecorp.armeria.common.RequestTarget; import com.linecorp.armeria.server.logging.LoggingService; import com.linecorp.armeria.testing.junit5.server.ServerExtension; @@ -54,8 +54,8 @@ protected void configure(ServerBuilder sb) { @Test void invalidPath() throws Exception { final String invalidPath = "/foo?download=../../secret.txt"; - final PathAndQuery pathAndQuery = PathAndQuery.parse(invalidPath); - assertThat(pathAndQuery).isNull(); + final RequestTarget reqTarget = RequestTarget.forServer(invalidPath); + assertThat(reqTarget).isNull(); final HttpClient client = HttpClient.newHttpClient(); diff --git a/grpc/src/main/java/com/linecorp/armeria/internal/client/grpc/ArmeriaChannel.java b/grpc/src/main/java/com/linecorp/armeria/internal/client/grpc/ArmeriaChannel.java index 7adb741c1c0a..fe4ca2e86760 100644 --- a/grpc/src/main/java/com/linecorp/armeria/internal/client/grpc/ArmeriaChannel.java +++ b/grpc/src/main/java/com/linecorp/armeria/internal/client/grpc/ArmeriaChannel.java @@ -38,6 +38,7 @@ import com.linecorp.armeria.common.HttpRequestWriter; import com.linecorp.armeria.common.RequestHeaders; import com.linecorp.armeria.common.RequestHeadersBuilder; +import com.linecorp.armeria.common.RequestTarget; import com.linecorp.armeria.common.Scheme; import com.linecorp.armeria.common.SerializationFormat; import com.linecorp.armeria.common.SessionProtocol; @@ -47,6 +48,7 @@ import com.linecorp.armeria.common.util.SystemInfo; import com.linecorp.armeria.common.util.Unwrappable; import com.linecorp.armeria.internal.client.DefaultClientRequestContext; +import com.linecorp.armeria.internal.common.RequestTargetCache; import io.grpc.CallCredentials; import io.grpc.CallOptions; @@ -221,14 +223,17 @@ public T as(Class type) { private DefaultClientRequestContext newContext(HttpMethod method, HttpRequest req, MethodDescriptor methodDescriptor) { + final String path = req.path(); + final RequestTarget reqTarget = RequestTarget.forClient(path); + assert reqTarget != null : path; + RequestTargetCache.putForClient(path, reqTarget); + return new DefaultClientRequestContext( meterRegistry, sessionProtocol, options().requestIdGenerator().get(), method, - req.path(), - null, - null, + reqTarget, options(), req, null, diff --git a/grpc/src/test/java/com/linecorp/armeria/client/grpc/GrpcClientTest.java b/grpc/src/test/java/com/linecorp/armeria/client/grpc/GrpcClientTest.java index 5ccb0a82b786..3c01d6a1bb7c 100644 --- a/grpc/src/test/java/com/linecorp/armeria/client/grpc/GrpcClientTest.java +++ b/grpc/src/test/java/com/linecorp/armeria/client/grpc/GrpcClientTest.java @@ -94,6 +94,7 @@ import com.linecorp.armeria.grpc.testing.UnitTestServiceGrpc.UnitTestServiceBlockingStub; import com.linecorp.armeria.grpc.testing.UnitTestServiceGrpc.UnitTestServiceImplBase; import com.linecorp.armeria.grpc.testing.UnitTestServiceGrpc.UnitTestServiceStub; +import com.linecorp.armeria.internal.common.RequestTargetCache; import com.linecorp.armeria.internal.common.grpc.GrpcLogUtil; import com.linecorp.armeria.internal.common.grpc.GrpcStatus; import com.linecorp.armeria.internal.common.grpc.MetadataUtil; @@ -236,6 +237,7 @@ public void close(Status status, Metadata trailers) { @BeforeEach void setUp() { + RequestTargetCache.clearCachedPaths(); requestLogQueue.clear(); final DecoratingHttpClientFunction requestLogRecorder = (delegate, ctx, req) -> { ctx.log().whenComplete().thenAccept(requestLogQueue::add); @@ -272,6 +274,8 @@ void emptyUnary() throws Exception { assertThat(rpcReq.params()).containsExactly(EMPTY); assertThat(rpcRes.get()).isEqualTo(EMPTY); }); + await().untilAsserted(() -> assertThat(RequestTargetCache.cachedClientPaths()) + .contains("/armeria.grpc.testing.TestService/EmptyCall")); } @Test @@ -322,6 +326,8 @@ void largeUnary() throws Exception { assertThat(rpcReq.params()).containsExactly(request); assertThat(rpcRes.get()).isEqualTo(goldenResponse); }); + await().untilAsserted(() -> assertThat(RequestTargetCache.cachedClientPaths()) + .contains("/armeria.grpc.testing.TestService/UnaryCall")); } @Test @@ -473,6 +479,8 @@ void serverStreaming() throws Exception { assertThat(rpcReq.params()).containsExactly(request); assertThat(rpcRes.get()).isEqualTo(goldenResponses.get(0)); }); + await().untilAsserted(() -> assertThat(RequestTargetCache.cachedClientPaths()) + .contains("/armeria.grpc.testing.TestService/StreamingOutputCall")); } @Test @@ -552,6 +560,8 @@ void clientStreaming() throws Exception { assertThat(rpcReq.params()).containsExactly(requests.get(0)); assertThat(rpcRes.get()).isEqualTo(goldenResponse); }); + await().untilAsserted(() -> assertThat(RequestTargetCache.cachedClientPaths()) + .contains("/armeria.grpc.testing.TestService/StreamingInputCall")); } @Test @@ -634,6 +644,8 @@ public void onCompleted() { assertThat(rpcReq.params()).containsExactly(requests.get(0)); assertThat(rpcRes.get()).isEqualTo(goldenResponses.get(0)); }); + await().untilAsserted(() -> assertThat(RequestTargetCache.cachedClientPaths()) + .contains("/armeria.grpc.testing.TestService/FullDuplexCall")); } @Test diff --git a/grpc/src/test/java/com/linecorp/armeria/server/grpc/GrpcServiceServerTest.java b/grpc/src/test/java/com/linecorp/armeria/server/grpc/GrpcServiceServerTest.java index 511298b8ef87..cb8e99b1131f 100644 --- a/grpc/src/test/java/com/linecorp/armeria/server/grpc/GrpcServiceServerTest.java +++ b/grpc/src/test/java/com/linecorp/armeria/server/grpc/GrpcServiceServerTest.java @@ -89,7 +89,7 @@ import com.linecorp.armeria.grpc.testing.UnitTestServiceGrpc.UnitTestServiceBlockingStub; import com.linecorp.armeria.grpc.testing.UnitTestServiceGrpc.UnitTestServiceImplBase; import com.linecorp.armeria.grpc.testing.UnitTestServiceGrpc.UnitTestServiceStub; -import com.linecorp.armeria.internal.common.PathAndQuery; +import com.linecorp.armeria.internal.common.RequestTargetCache; import com.linecorp.armeria.internal.common.grpc.GrpcLogUtil; import com.linecorp.armeria.internal.common.grpc.GrpcTestUtil; import com.linecorp.armeria.internal.common.grpc.StreamRecorder; @@ -545,7 +545,7 @@ void setUp() { COMPLETED.set(false); CLIENT_CLOSED.set(false); - PathAndQuery.clearCachedPaths(); + RequestTargetCache.clearCachedPaths(); } @AfterEach @@ -564,7 +564,7 @@ void unary_normal(UnitTestServiceBlockingStub blockingClient) throws Exception { assertThat(blockingClient.staticUnaryCall(REQUEST_MESSAGE)).isEqualTo(RESPONSE_MESSAGE); // Confirm gRPC paths are cached despite using serviceUnder - await().untilAsserted(() -> assertThat(PathAndQuery.cachedPaths()) + await().untilAsserted(() -> assertThat(RequestTargetCache.cachedServerPaths()) .contains("/armeria.grpc.testing.UnitTestService/StaticUnaryCall")); checkRequestLog((rpcReq, rpcRes, grpcStatus) -> { diff --git a/thrift/thrift0.13/src/main/java/com/linecorp/armeria/internal/client/thrift/DefaultTHttpClient.java b/thrift/thrift0.13/src/main/java/com/linecorp/armeria/internal/client/thrift/DefaultTHttpClient.java index 4bf823db3765..7149ce3c327b 100644 --- a/thrift/thrift0.13/src/main/java/com/linecorp/armeria/internal/client/thrift/DefaultTHttpClient.java +++ b/thrift/thrift0.13/src/main/java/com/linecorp/armeria/internal/client/thrift/DefaultTHttpClient.java @@ -17,7 +17,6 @@ package com.linecorp.armeria.internal.client.thrift; import static com.linecorp.armeria.internal.client.thrift.THttpClientDelegate.decodeException; -import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.concatPaths; import static java.util.Objects.requireNonNull; import org.apache.thrift.transport.TTransportException; @@ -29,10 +28,11 @@ import com.linecorp.armeria.client.thrift.THttpClient; import com.linecorp.armeria.common.ExchangeType; import com.linecorp.armeria.common.HttpMethod; +import com.linecorp.armeria.common.RequestTarget; import com.linecorp.armeria.common.RpcRequest; import com.linecorp.armeria.common.RpcResponse; import com.linecorp.armeria.common.annotation.Nullable; -import com.linecorp.armeria.internal.common.PathAndQuery; +import com.linecorp.armeria.internal.common.RequestTargetCache; import io.micrometer.core.instrument.MeterRegistry; @@ -63,19 +63,22 @@ public RpcResponse executeMultiplexed( private RpcResponse execute0( String path, Class serviceType, @Nullable String serviceName, String method, Object[] args) { - path = concatPaths(uri().getRawPath(), path); - final PathAndQuery pathAndQuery = PathAndQuery.parse(path); - if (pathAndQuery == null) { + if (serviceName != null) { + path = path + '#' + serviceName; + } + + final RequestTarget reqTarget = RequestTarget.forClient(path, uri().getRawPath()); + if (reqTarget == null) { return RpcResponse.ofFailure(new TTransportException( new IllegalArgumentException("invalid path: " + path))); } // A thrift path is always good to cache as it cannot have non-fixed parameters. - pathAndQuery.storeInCache(path); + RequestTargetCache.putForClient(path, reqTarget); final RpcRequest call = RpcRequest.of(serviceType, method, args); return execute(scheme().sessionProtocol(), HttpMethod.POST, - pathAndQuery.path(), null, serviceName, call, UNARY_REQUEST_OPTIONS); + reqTarget, call, UNARY_REQUEST_OPTIONS); } @Override