From 49e04ef231ad65750739529c7fa4ce946ff7588b Mon Sep 17 00:00:00 2001 From: minux Date: Tue, 25 Jul 2023 15:37:02 +0900 Subject: [PATCH] Merge pull request from GHSA-wvp2-9ppw-337j * Remove matrix variables from the path on the server-side Motivation: Spring supports [Matrix variables](https://docs.spring.io/spring-framework/reference/web/webmvc/mvc-controller/ann-methods/matrix-variables.html). When Spring integration is used, Armeria calls Spring controllers via `TomcatService` or `JettyService` with the path that may contain matrix variables. In this situation, the Armeria decorators might not invoked because of the matrix variables. Let's see the following example: ``` // Spring controller @GetMapping("/important/resources") public String important() {...} // Armeria decorator ServerBuilder sb = ... sb.decoratorUnder("/important/", authService); ``` If an attacker sends a request with `/important;a=b/resources`, the request would bypass the authrorizer which is a significant security vulnerability. Armeria neither supports it nor has any plans to support it. So we can just remove the matrix variables from the path and restore them when we call Spring controllers. Modifications: - Remove matrix varilbles from the server-side path. - The path is restored when calling Spring service using `RequestTarget.pathWithMatrixVariables()`. - Do not decode `%3B` which is `;`. - Add `it/boot3-jetty` module by Copying `it/boot3-tomcat` Result: - The decorators work correctly when the request path contains matrix varialbes. * Update it/spring/boot3-jetty11/src/test/resources/application-testbed.yml --- .../common/AbstractRequestContextBuilder.java | 15 ++- .../armeria/common/DefaultFlagsProvider.java | 5 + .../com/linecorp/armeria/common/Flags.java | 24 ++++ .../armeria/common/FlagsProvider.java | 22 ++++ .../armeria/common/RequestTarget.java | 13 +- .../common/SystemPropertyFlagsProvider.java | 5 + .../internal/common/DefaultRequestTarget.java | 114 ++++++++++++------ .../internal/common/util/MappedPathUtil.java | 54 +++++++++ .../armeria/server/ExactPathMapping.java | 6 + .../server/ParameterizedPathMapping.java | 7 ++ .../armeria/server/PrefixPathMapping.java | 4 + .../armeria/server/RoutingContext.java | 2 + .../common/DefaultRequestTargetTest.java | 78 +++++++++--- .../armeria/server/MatrixVariablesTest.java | 49 ++++++++ .../linecorp/armeria/server/RouteTest.java | 14 +++ it/spring/boot3-jetty11/build.gradle | 12 ++ .../spring/jetty/ErrorHandlingController.java | 72 +++++++++++ .../spring/jetty/GlobalBaseException.java | 23 ++++ .../spring/jetty/GlobalExceptionHandler.java | 38 ++++++ .../armeria/spring/jetty/Greeting.java | 34 ++++++ .../spring/jetty/GreetingController.java | 40 ++++++ .../jetty/MatrixVariablesController.java | 39 ++++++ .../spring/jetty/SpringJettyApplication.java | 85 +++++++++++++ .../armeria/spring/jetty/package-info.java | 20 +++ .../src/main/resources/application.properties | 0 .../src/main/resources/config/application.yml | 7 ++ ...uatorAutoConfigurationHealthGroupTest.java | 98 +++++++++++++++ .../spring/jetty/ErrorHandlingTest.java | 63 ++++++++++ .../spring/jetty/MatrixVariablesTest.java | 60 +++++++++ .../jetty/SpringJettyApplicationItTest.java | 86 +++++++++++++ .../resources/application-healthGroupTest.yml | 23 ++++ .../test/resources/application-testbed.yml | 7 ++ .../tomcat/MatrixVariablesController.java | 39 ++++++ ...uatorAutoConfigurationHealthGroupTest.java | 3 +- .../spring/tomcat/ErrorHandlingTest.java | 3 +- .../spring/tomcat/MatrixVariablesTest.java | 60 +++++++++ .../test/resources/application-testbed.yml | 7 ++ .../armeria/server/jetty/JettyService.java | 28 ++++- .../armeria/server/jetty/JettyService.java | 8 ++ settings.gradle | 1 + .../boot3-webflux-autoconfigure/build.gradle | 1 + .../reactive/ArmeriaServerHttpRequest.java | 12 +- .../web/reactive/MatrixVariablesTest.java | 68 +++++++++++ .../armeria/server/tomcat/TomcatService.java | 14 ++- 44 files changed, 1291 insertions(+), 72 deletions(-) create mode 100644 core/src/main/java/com/linecorp/armeria/internal/common/util/MappedPathUtil.java create mode 100644 core/src/test/java/com/linecorp/armeria/server/MatrixVariablesTest.java create mode 100644 it/spring/boot3-jetty11/build.gradle create mode 100644 it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/ErrorHandlingController.java create mode 100644 it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/GlobalBaseException.java create mode 100644 it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/GlobalExceptionHandler.java create mode 100644 it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/Greeting.java create mode 100644 it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/GreetingController.java create mode 100644 it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/MatrixVariablesController.java create mode 100644 it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/SpringJettyApplication.java create mode 100644 it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/package-info.java create mode 100644 it/spring/boot3-jetty11/src/main/resources/application.properties create mode 100644 it/spring/boot3-jetty11/src/main/resources/config/application.yml create mode 100644 it/spring/boot3-jetty11/src/test/java/com/linecorp/armeria/spring/jetty/ActuatorAutoConfigurationHealthGroupTest.java create mode 100644 it/spring/boot3-jetty11/src/test/java/com/linecorp/armeria/spring/jetty/ErrorHandlingTest.java create mode 100644 it/spring/boot3-jetty11/src/test/java/com/linecorp/armeria/spring/jetty/MatrixVariablesTest.java create mode 100644 it/spring/boot3-jetty11/src/test/java/com/linecorp/armeria/spring/jetty/SpringJettyApplicationItTest.java create mode 100644 it/spring/boot3-jetty11/src/test/resources/application-healthGroupTest.yml create mode 100644 it/spring/boot3-jetty11/src/test/resources/application-testbed.yml create mode 100644 it/spring/boot3-tomcat10/src/main/java/com/linecorp/armeria/spring/tomcat/MatrixVariablesController.java create mode 100644 it/spring/boot3-tomcat10/src/test/java/com/linecorp/armeria/spring/tomcat/MatrixVariablesTest.java create mode 100644 it/spring/boot3-tomcat10/src/test/resources/application-testbed.yml create mode 100644 spring/boot3-webflux-autoconfigure/src/test/java/com/linecorp/armeria/spring/web/reactive/MatrixVariablesTest.java 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 1d01dca8367..b9e21288ef1 100644 --- a/core/src/main/java/com/linecorp/armeria/common/AbstractRequestContextBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/AbstractRequestContextBuilder.java @@ -142,13 +142,20 @@ protected AbstractRequestContextBuilder(boolean server, RpcRequest rpcReq, URI u sessionProtocol = getSessionProtocol(uri); if (server) { - reqTarget = DefaultRequestTarget.createWithoutValidation( - RequestTargetForm.ORIGIN, null, null, - uri.getRawPath(), uri.getRawQuery(), null); + String path = uri.getRawPath(); + final String query = uri.getRawQuery(); + if (query != null) { + path += '?' + query; + } + final RequestTarget reqTarget = RequestTarget.forServer(path); + if (reqTarget == null) { + throw new IllegalArgumentException("invalid uri: " + uri); + } + this.reqTarget = reqTarget; } else { reqTarget = DefaultRequestTarget.createWithoutValidation( RequestTargetForm.ORIGIN, null, null, - uri.getRawPath(), uri.getRawQuery(), uri.getRawFragment()); + uri.getRawPath(), uri.getRawPath(), uri.getRawQuery(), uri.getRawFragment()); } } diff --git a/core/src/main/java/com/linecorp/armeria/common/DefaultFlagsProvider.java b/core/src/main/java/com/linecorp/armeria/common/DefaultFlagsProvider.java index 6947f86083d..f4873c4afbe 100644 --- a/core/src/main/java/com/linecorp/armeria/common/DefaultFlagsProvider.java +++ b/core/src/main/java/com/linecorp/armeria/common/DefaultFlagsProvider.java @@ -413,6 +413,11 @@ public Boolean allowDoubleDotsInQueryString() { return false; } + @Override + public Boolean allowSemicolonInPathComponent() { + return false; + } + @Override public Path defaultMultipartUploadsLocation() { return Paths.get(System.getProperty("java.io.tmpdir") + diff --git a/core/src/main/java/com/linecorp/armeria/common/Flags.java b/core/src/main/java/com/linecorp/armeria/common/Flags.java index d0591df12d5..b0d99054d70 100644 --- a/core/src/main/java/com/linecorp/armeria/common/Flags.java +++ b/core/src/main/java/com/linecorp/armeria/common/Flags.java @@ -375,6 +375,9 @@ private static boolean validateTransportType(TransportType transportType, String private static final boolean ALLOW_DOUBLE_DOTS_IN_QUERY_STRING = getValue(FlagsProvider::allowDoubleDotsInQueryString, "allowDoubleDotsInQueryString"); + private static final boolean ALLOW_SEMICOLON_IN_PATH_COMPONENT = + getValue(FlagsProvider::allowSemicolonInPathComponent, "allowSemicolonInPathComponent"); + private static final Path DEFAULT_MULTIPART_UPLOADS_LOCATION = getValue(FlagsProvider::defaultMultipartUploadsLocation, "defaultMultipartUploadsLocation"); @@ -1340,6 +1343,27 @@ public static boolean allowDoubleDotsInQueryString() { return ALLOW_DOUBLE_DOTS_IN_QUERY_STRING; } + /** + * Returns whether to allow a semicolon ({@code ;}) in a request path component on the server-side. + * If disabled, the substring from the semicolon to before the next slash, commonly referred to as + * matrix variables, is removed. For example, {@code /foo;a=b/bar} will be converted to {@code /foo/bar}. + * Also, an exception is raised if a semicolon is used for binding a service. For example, the following + * code raises an exception: + * 
{@code
+     * Server server =
+     *    Server.builder()
+     *      .service("/foo;bar", ...)
+     *      .build();
+     * }
+ * Note that this flag has no effect on the client-side. + * + *

This flag is disabled by default. Specify the + * {@code -Dcom.linecorp.armeria.allowSemicolonInPathComponent=true} JVM option to enable it. + */ + public static boolean allowSemicolonInPathComponent() { + return ALLOW_SEMICOLON_IN_PATH_COMPONENT; + } + /** * Returns the {@link Sampler} that determines whether to trace the stack trace of request contexts leaks * and how frequently to keeps stack trace. A sampled exception will have the stack trace while the others diff --git a/core/src/main/java/com/linecorp/armeria/common/FlagsProvider.java b/core/src/main/java/com/linecorp/armeria/common/FlagsProvider.java index 5af62b43964..925d854a598 100644 --- a/core/src/main/java/com/linecorp/armeria/common/FlagsProvider.java +++ b/core/src/main/java/com/linecorp/armeria/common/FlagsProvider.java @@ -984,6 +984,28 @@ default Boolean allowDoubleDotsInQueryString() { return null; } + /** + * Returns whether to allow a semicolon ({@code ;}) in a request path component on the server-side. + * If disabled, the substring from the semicolon to before the next slash, commonly referred to as + * matrix variables, is removed. For example, {@code /foo;a=b/bar} will be converted to {@code /foo/bar}. + * Also, an exception is raised if a semicolon is used for binding a service. For example, the following + * code raises an exception: + * 

{@code
+     * Server server =
+     *    Server.builder()
+     *      .service("/foo;bar", ...)
+     *      .build();
+     * }
+ * Note that this flag has no effect on the client-side. + * + *

This flag is disabled by default. Specify the + * {@code -Dcom.linecorp.armeria.allowSemicolonInPathComponent=true} JVM option to enable it. + */ + @Nullable + default Boolean allowSemicolonInPathComponent() { + return null; + } + /** * Returns the {@link Path} that is used to store the files uploaded from {@code multipart/form-data} * requests. diff --git a/core/src/main/java/com/linecorp/armeria/common/RequestTarget.java b/core/src/main/java/com/linecorp/armeria/common/RequestTarget.java index 3dbd9d1be7b..f2d2332d7bc 100644 --- a/core/src/main/java/com/linecorp/armeria/common/RequestTarget.java +++ b/core/src/main/java/com/linecorp/armeria/common/RequestTarget.java @@ -44,7 +44,8 @@ public interface RequestTarget { @Nullable static RequestTarget forServer(String reqTarget) { requireNonNull(reqTarget, "reqTarget"); - return DefaultRequestTarget.forServer(reqTarget, Flags.allowDoubleDotsInQueryString()); + return DefaultRequestTarget.forServer(reqTarget, Flags.allowSemicolonInPathComponent(), + Flags.allowDoubleDotsInQueryString()); } /** @@ -102,6 +103,16 @@ static RequestTarget forClient(String reqTarget, @Nullable String prefix) { */ String path(); + /** + * Returns the path of this {@link RequestTarget}, which always starts with {@code '/'}. + * Unlike {@link #path()}, the returned string contains matrix variables it the original request path + * contains them. + * + * @see + * Matrix Variables + */ + String maybePathWithMatrixVariables(); + /** * Returns the query of this {@link RequestTarget}. */ diff --git a/core/src/main/java/com/linecorp/armeria/common/SystemPropertyFlagsProvider.java b/core/src/main/java/com/linecorp/armeria/common/SystemPropertyFlagsProvider.java index 69874731a14..f6fe37c13e7 100644 --- a/core/src/main/java/com/linecorp/armeria/common/SystemPropertyFlagsProvider.java +++ b/core/src/main/java/com/linecorp/armeria/common/SystemPropertyFlagsProvider.java @@ -435,6 +435,11 @@ public Boolean allowDoubleDotsInQueryString() { return getBoolean("allowDoubleDotsInQueryString"); } + @Override + public Boolean allowSemicolonInPathComponent() { + return getBoolean("allowSemicolonInPathComponent"); + } + @Override public Path defaultMultipartUploadsLocation() { return getAndParse("defaultMultipartUploadsLocation", Paths::get); diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultRequestTarget.java b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultRequestTarget.java index 3d1fb006c4d..1bbd4c61ee2 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultRequestTarget.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultRequestTarget.java @@ -123,6 +123,7 @@ boolean mustPreserveEncoding(int cp) { null, null, "*", + "*", null, null); @@ -130,13 +131,14 @@ boolean mustPreserveEncoding(int cp) { * The main implementation of {@link RequestTarget#forServer(String)}. */ @Nullable - public static RequestTarget forServer(String reqTarget, boolean allowDoubleDotsInQueryString) { + public static RequestTarget forServer(String reqTarget, boolean allowSemicolonInPathComponent, + boolean allowDoubleDotsInQueryString) { final RequestTarget cached = RequestTargetCache.getForServer(reqTarget); if (cached != null) { return cached; } - return slowForServer(reqTarget, allowDoubleDotsInQueryString); + return slowForServer(reqTarget, allowSemicolonInPathComponent, allowDoubleDotsInQueryString); } /** @@ -183,8 +185,9 @@ public static RequestTarget forClient(String reqTarget, @Nullable String prefix) */ 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); + String path, String pathWithMatrixVariables, @Nullable String query, @Nullable String fragment) { + return new DefaultRequestTarget( + form, scheme, authority, path, pathWithMatrixVariables, query, fragment); } private final RequestTargetForm form; @@ -193,6 +196,7 @@ public static RequestTarget createWithoutValidation( @Nullable private final String authority; private final String path; + private final String maybePathWithMatrixVariables; @Nullable private final String query; @Nullable @@ -200,7 +204,8 @@ public static RequestTarget createWithoutValidation( private boolean cached; private DefaultRequestTarget(RequestTargetForm form, @Nullable String scheme, @Nullable String authority, - String path, @Nullable String query, @Nullable String fragment) { + String path, String maybePathWithMatrixVariables, + @Nullable String query, @Nullable String fragment) { assert (scheme != null && authority != null) || (scheme == null && authority == null) : "scheme: " + scheme + ", authority: " + authority; @@ -209,6 +214,7 @@ private DefaultRequestTarget(RequestTargetForm form, @Nullable String scheme, @N this.scheme = scheme; this.authority = authority; this.path = path; + this.maybePathWithMatrixVariables = maybePathWithMatrixVariables; this.query = query; this.fragment = fragment; } @@ -233,6 +239,11 @@ public String path() { return path; } + @Override + public String maybePathWithMatrixVariables() { + return maybePathWithMatrixVariables; + } + @Override public String query() { return query; @@ -258,18 +269,6 @@ 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) { @@ -312,7 +311,8 @@ public String toString() { } @Nullable - private static RequestTarget slowForServer(String reqTarget, boolean allowDoubleDotsInQueryString) { + private static RequestTarget slowForServer(String reqTarget, boolean allowSemicolonInPathComponent, + boolean allowDoubleDotsInQueryString) { final Bytes path; final Bytes query; @@ -321,18 +321,18 @@ private static RequestTarget slowForServer(String reqTarget, boolean allowDouble if (queryPos >= 0) { if ((path = decodePercentsAndEncodeToUtf8( reqTarget, 0, queryPos, - ComponentType.SERVER_PATH, null)) == null) { + ComponentType.SERVER_PATH, null, allowSemicolonInPathComponent)) == null) { return null; } if ((query = decodePercentsAndEncodeToUtf8( reqTarget, queryPos + 1, reqTarget.length(), - ComponentType.QUERY, EMPTY_BYTES)) == null) { + ComponentType.QUERY, EMPTY_BYTES, true)) == null) { return null; } } else { if ((path = decodePercentsAndEncodeToUtf8( reqTarget, 0, reqTarget.length(), - ComponentType.SERVER_PATH, null)) == null) { + ComponentType.SERVER_PATH, null, allowSemicolonInPathComponent)) == null) { return null; } query = null; @@ -356,14 +356,58 @@ private static RequestTarget slowForServer(String reqTarget, boolean allowDouble return null; } + final String encodedPath = encodePathToPercents(path); + final String matrixVariablesRemovedPath; + if (allowSemicolonInPathComponent) { + matrixVariablesRemovedPath = encodedPath; + } else { + matrixVariablesRemovedPath = removeMatrixVariables(encodedPath); + if (matrixVariablesRemovedPath == null) { + return null; + } + } return new DefaultRequestTarget(RequestTargetForm.ORIGIN, null, null, - encodePathToPercents(path), + matrixVariablesRemovedPath, + encodedPath, encodeQueryToPercents(query), null); } + @Nullable + public static String removeMatrixVariables(String path) { + int semicolonIndex = path.indexOf(';'); + if (semicolonIndex < 0) { + return path; + } + if (semicolonIndex == 0 || path.charAt(semicolonIndex - 1) == '/') { + // Invalid matrix variable e.g. /;a=b/foo + return null; + } + int subStringStartIndex = 0; + try (TemporaryThreadLocals threadLocals = TemporaryThreadLocals.acquire()) { + final StringBuilder sb = threadLocals.stringBuilder(); + for (;;) { + sb.append(path, subStringStartIndex, semicolonIndex); + final int slashIndex = path.indexOf('/', semicolonIndex + 1); + if (slashIndex < 0) { + return sb.toString(); + } + subStringStartIndex = slashIndex; + semicolonIndex = path.indexOf(';', subStringStartIndex + 1); + if (semicolonIndex < 0) { + sb.append(path, subStringStartIndex, path.length()); + return sb.toString(); + } + if (path.charAt(semicolonIndex - 1) == '/') { + // Invalid matrix variable e.g. /prefix/;a=b/foo + return null; + } + } + } + } + @Nullable private static RequestTarget slowAbsoluteFormForClient(String reqTarget, int authorityPos) { // Extract scheme and authority while looking for path. @@ -396,7 +440,7 @@ private static RequestTarget slowAbsoluteFormForClient(String reqTarget, int aut schemeAndAuthority.getScheme(), schemeAndAuthority.getRawAuthority(), "/", - null, + "/", null, null); } @@ -457,7 +501,7 @@ private static RequestTarget slowForClient(String reqTarget, if (queryPos >= 0) { if ((path = decodePercentsAndEncodeToUtf8( reqTarget, pathPos, queryPos, - ComponentType.CLIENT_PATH, SLASH_BYTES)) == null) { + ComponentType.CLIENT_PATH, SLASH_BYTES, true)) == null) { return null; } @@ -465,19 +509,19 @@ private static RequestTarget slowForClient(String reqTarget, // path?query#fragment if ((query = decodePercentsAndEncodeToUtf8( reqTarget, queryPos + 1, fragmentPos, - ComponentType.QUERY, EMPTY_BYTES)) == null) { + ComponentType.QUERY, EMPTY_BYTES, true)) == null) { return null; } if ((fragment = decodePercentsAndEncodeToUtf8( reqTarget, fragmentPos + 1, reqTarget.length(), - ComponentType.FRAGMENT, EMPTY_BYTES)) == null) { + ComponentType.FRAGMENT, EMPTY_BYTES, true)) == null) { return null; } } else { // path?query if ((query = decodePercentsAndEncodeToUtf8( reqTarget, queryPos + 1, reqTarget.length(), - ComponentType.QUERY, EMPTY_BYTES)) == null) { + ComponentType.QUERY, EMPTY_BYTES, true)) == null) { return null; } fragment = null; @@ -487,20 +531,20 @@ private static RequestTarget slowForClient(String reqTarget, // path#fragment if ((path = decodePercentsAndEncodeToUtf8( reqTarget, pathPos, fragmentPos, - ComponentType.CLIENT_PATH, EMPTY_BYTES)) == null) { + ComponentType.CLIENT_PATH, EMPTY_BYTES, true)) == null) { return null; } query = null; if ((fragment = decodePercentsAndEncodeToUtf8( reqTarget, fragmentPos + 1, reqTarget.length(), - ComponentType.FRAGMENT, EMPTY_BYTES)) == null) { + ComponentType.FRAGMENT, EMPTY_BYTES, true)) == null) { return null; } } else { // path if ((path = decodePercentsAndEncodeToUtf8( reqTarget, pathPos, reqTarget.length(), - ComponentType.CLIENT_PATH, EMPTY_BYTES)) == null) { + ComponentType.CLIENT_PATH, EMPTY_BYTES, true)) == null) { return null; } query = null; @@ -529,14 +573,14 @@ private static RequestTarget slowForClient(String reqTarget, schemeAndAuthority.getScheme(), schemeAndAuthority.getRawAuthority(), encodedPath, - encodedQuery, + encodedPath, encodedQuery, encodedFragment); } else { return new DefaultRequestTarget(RequestTargetForm.ORIGIN, null, null, encodedPath, - encodedQuery, + encodedPath, encodedQuery, encodedFragment); } } @@ -577,7 +621,8 @@ private static boolean isRelativePath(Bytes path) { @Nullable private static Bytes decodePercentsAndEncodeToUtf8(String value, int start, int end, - ComponentType type, @Nullable Bytes whenEmpty) { + ComponentType type, @Nullable Bytes whenEmpty, + boolean allowSemicolonInPathComponent) { final int length = end - start; if (length == 0) { return whenEmpty; @@ -605,7 +650,8 @@ private static Bytes decodePercentsAndEncodeToUtf8(String value, int start, int } final int decoded = (digit1 << 4) | digit2; - if (type.mustPreserveEncoding(decoded)) { + if (type.mustPreserveEncoding(decoded) || + (!allowSemicolonInPathComponent && decoded == ';')) { buf.ensure(1); buf.addEncoded((byte) decoded); wasSlash = false; diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/util/MappedPathUtil.java b/core/src/main/java/com/linecorp/armeria/internal/common/util/MappedPathUtil.java new file mode 100644 index 00000000000..1f6c07d6303 --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/internal/common/util/MappedPathUtil.java @@ -0,0 +1,54 @@ +/* + * 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.util; + +import com.linecorp.armeria.common.RequestTarget; +import com.linecorp.armeria.common.annotation.Nullable; +import com.linecorp.armeria.server.ServiceRequestContext; + +public final class MappedPathUtil { + + /** + * Returns the path with its prefix removed. Unlike {@link ServiceRequestContext#mappedPath()}, this method + * returns the path with matrix variables if the mapped path contains matrix variables. + * This returns {@code null} if the path has matrix variables in the prefix. + */ + @Nullable + public static String mappedPath(ServiceRequestContext ctx) { + final RequestTarget requestTarget = ctx.routingContext().requestTarget(); + final String pathWithMatrixVariables = requestTarget.maybePathWithMatrixVariables(); + if (pathWithMatrixVariables.equals(ctx.path())) { + return ctx.mappedPath(); + } + // The request path contains matrix variables. e.g. "/foo/bar/users;name=alice" + + final String prefix = ctx.path().substring(0, ctx.path().length() - ctx.mappedPath().length()); + // The prefix is now `/foo/bar` + if (!pathWithMatrixVariables.startsWith(prefix)) { + // The request path has matrix variables in the wrong place. e.g. "/foo;name=alice/bar/users" + return null; + } + final String mappedPath = pathWithMatrixVariables.substring(prefix.length()); + if (mappedPath.charAt(0) != '/') { + // Again, the request path has matrix variables in the wrong place. e.g. "/foo/bar;/users" + return null; + } + return mappedPath; + } + + private MappedPathUtil() {} +} diff --git a/core/src/main/java/com/linecorp/armeria/server/ExactPathMapping.java b/core/src/main/java/com/linecorp/armeria/server/ExactPathMapping.java index d9dcee4267b..fdffcbb6d71 100644 --- a/core/src/main/java/com/linecorp/armeria/server/ExactPathMapping.java +++ b/core/src/main/java/com/linecorp/armeria/server/ExactPathMapping.java @@ -16,6 +16,7 @@ package com.linecorp.armeria.server; +import static com.google.common.base.Preconditions.checkArgument; import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.concatPaths; import static com.linecorp.armeria.internal.server.RouteUtil.ensureAbsolutePath; @@ -25,6 +26,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.linecorp.armeria.common.Flags; import com.linecorp.armeria.common.annotation.Nullable; final class ExactPathMapping extends AbstractPathMapping { @@ -38,6 +40,10 @@ final class ExactPathMapping extends AbstractPathMapping { } private ExactPathMapping(String prefix, String exactPath) { + if (!Flags.allowSemicolonInPathComponent()) { + checkArgument(prefix.indexOf(';') < 0, "prefix: %s (expected not to have a ';')", prefix); + checkArgument(exactPath.indexOf(';') < 0, "exactPath: %s (expected not to have a ';')", exactPath); + } this.prefix = prefix; this.exactPath = ensureAbsolutePath(exactPath, "exactPath"); paths = ImmutableList.of(exactPath, exactPath); diff --git a/core/src/main/java/com/linecorp/armeria/server/ParameterizedPathMapping.java b/core/src/main/java/com/linecorp/armeria/server/ParameterizedPathMapping.java index 4ea9d8e52be..347cbe11320 100644 --- a/core/src/main/java/com/linecorp/armeria/server/ParameterizedPathMapping.java +++ b/core/src/main/java/com/linecorp/armeria/server/ParameterizedPathMapping.java @@ -16,6 +16,7 @@ package com.linecorp.armeria.server; +import static com.google.common.base.Preconditions.checkArgument; import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.concatPaths; import static java.util.Objects.requireNonNull; @@ -32,6 +33,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.linecorp.armeria.common.Flags; import com.linecorp.armeria.common.annotation.Nullable; /** @@ -105,6 +107,11 @@ final class ParameterizedPathMapping extends AbstractPathMapping { } private ParameterizedPathMapping(String prefix, String pathPattern) { + if (!Flags.allowSemicolonInPathComponent()) { + checkArgument(prefix.indexOf(';') < 0, "prefix: %s (expected not to have a ';')", prefix); + checkArgument(pathPattern.indexOf(';') < 0, + "pathPattern: %s (expected not to have a ';')", pathPattern); + } this.prefix = prefix; requireNonNull(pathPattern, "pathPattern"); diff --git a/core/src/main/java/com/linecorp/armeria/server/PrefixPathMapping.java b/core/src/main/java/com/linecorp/armeria/server/PrefixPathMapping.java index d8f631a646c..65754d3b8a2 100644 --- a/core/src/main/java/com/linecorp/armeria/server/PrefixPathMapping.java +++ b/core/src/main/java/com/linecorp/armeria/server/PrefixPathMapping.java @@ -16,6 +16,7 @@ package com.linecorp.armeria.server; +import static com.google.common.base.Preconditions.checkArgument; import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.concatPaths; import static com.linecorp.armeria.internal.server.RouteUtil.PREFIX; import static com.linecorp.armeria.internal.server.RouteUtil.ensureAbsolutePath; @@ -26,6 +27,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.linecorp.armeria.common.Flags; import com.linecorp.armeria.common.annotation.Nullable; final class PrefixPathMapping extends AbstractPathMapping { @@ -37,6 +39,8 @@ final class PrefixPathMapping extends AbstractPathMapping { private final String strVal; PrefixPathMapping(String prefix, boolean stripPrefix) { + checkArgument(Flags.allowSemicolonInPathComponent() || prefix.indexOf(';') < 0, + "prefix: %s (expected not to have a ';')", prefix); prefix = ensureAbsolutePath(prefix, "prefix"); if (!prefix.endsWith("/")) { prefix += '/'; 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 eb08575a197..55207d01068 100644 --- a/core/src/main/java/com/linecorp/armeria/server/RoutingContext.java +++ b/core/src/main/java/com/linecorp/armeria/server/RoutingContext.java @@ -16,6 +16,7 @@ package com.linecorp.armeria.server; +import static com.linecorp.armeria.internal.common.DefaultRequestTarget.removeMatrixVariables; import static java.util.Objects.requireNonNull; import java.util.List; @@ -146,6 +147,7 @@ default RoutingContext withPath(String path) { oldReqTarget.form(), oldReqTarget.scheme(), oldReqTarget.authority(), + removeMatrixVariables(path), path, oldReqTarget.query(), oldReqTarget.fragment()); diff --git a/core/src/test/java/com/linecorp/armeria/internal/common/DefaultRequestTargetTest.java b/core/src/test/java/com/linecorp/armeria/internal/common/DefaultRequestTargetTest.java index abfbb5a02f4..dc9aa7bed7e 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/common/DefaultRequestTargetTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/common/DefaultRequestTargetTest.java @@ -16,9 +16,11 @@ package com.linecorp.armeria.internal.common; import static com.google.common.base.Strings.emptyToNull; +import static com.linecorp.armeria.internal.common.DefaultRequestTarget.removeMatrixVariables; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import java.net.URISyntaxException; import java.util.Set; import java.util.stream.Stream; @@ -342,12 +344,14 @@ void shouldNormalizeAmpersand(Mode mode) { assertAccepted(parse(mode, "/%26?a=1%26a=2&b=3"), "/&", "a=1%26a=2&b=3"); } - @ParameterizedTest - @EnumSource(Mode.class) - void shouldNormalizeSemicolon(Mode mode) { - assertAccepted(parse(mode, "/;?a=b;c=d"), "/;", "a=b;c=d"); - // '%3B' in a query string should never be decoded into ';'. - assertAccepted(parse(mode, "/%3b?a=b%3Bc=d"), "/;", "a=b%3Bc=d"); + @Test + void serverShouldRemoveMatrixVariablesWhenNotAllowed() { + // Not allowed + assertAccepted(forServer("/;a=b?c=d;e=f"), "/", "c=d;e=f"); + // Allowed. + assertAccepted(forServer("/;a=b?c=d;e=f", true), "/;a=b", "c=d;e=f"); + // '%3B' should never be decoded into ';'. + assertAccepted(forServer("/%3B?a=b%3Bc=d"), "/%3B", "a=b%3Bc=d"); } @ParameterizedTest @@ -359,12 +363,25 @@ void shouldNormalizeEqualSign(Mode mode) { } @Test - void shouldReserveQuestionMark() { + void shouldReserveQuestionMark() throws URISyntaxException { // '%3F' must not be decoded into '?'. assertAccepted(forServer("/abc%3F.json?a=%3F"), "/abc%3F.json", "a=%3F"); assertAccepted(forClient("/abc%3F.json?a=%3F"), "/abc%3F.json", "a=%3F"); } + @Test + void reserveSemicolonWhenAllowed() { + // '%3B' is decoded into ';' when allowSemicolonInPathComponent is true. + assertAccepted(forServer("/abc%3B?a=%3B", true), "/abc;", "a=%3B"); + assertAccepted(forServer("/abc%3B?a=%3B"), "/abc%3B", "a=%3B"); + + assertAccepted(forServer("/abc%3B", true), "/abc;"); + assertAccepted(forServer("/abc%3B"), "/abc%3B"); + + // Client always decodes '%3B' into ';'. + assertAccepted(forClient("/abc%3B?a=%3B"), "/abc;", "a=%3B"); + } + @Test void serverShouldNormalizePoundSign() { // '#' must be encoded into '%23'. @@ -386,12 +403,12 @@ void clientShouldTreatPoundSignAsFragment() { @Test void serverShouldHandleReservedCharacters() { - assertAccepted(forServer("/#/:@!$&'()*+,;=?a=/#/:[]@!$&'()*+,;="), - "/%23/:@!$&'()*+,;=", - "a=/%23/:[]@!$&'()*+,;="); + assertAccepted(forServer("/#/:@!$&'()*+,=?a=/#/:[]@!$&'()*+,="), + "/%23/:@!$&'()*+,=", + "a=/%23/:[]@!$&'()*+,="); assertAccepted(forServer("/%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"), - "/%23%2F:@!$&'()*+,;=%3F", + "/%23%2F:@!$&'()*+,%3B=%3F", "a=%23%2F%3A%5B%5D%40%21%24%26%27%28%29%2A%2B%2C%3B%3D%3F"); } @@ -418,9 +435,9 @@ void shouldHandleDoubleQuote(Mode mode) { @ParameterizedTest @EnumSource(Mode.class) void shouldHandleSquareBracketsInPath(Mode mode) { - assertAccepted(parse(mode, "/@/:[]!$&'()*+,;="), "/@/:%5B%5D!$&'()*+,;="); - assertAccepted(parse(mode, "/%40%2F%3A%5B%5D%21%24%26%27%28%29%2A%2B%2C%3B%3D%3F"), - "/@%2F:%5B%5D!$&'()*+,;=%3F"); + assertAccepted(parse(mode, "/@/:[]!$&'()*+,="), "/@/:%5B%5D!$&'()*+,="); + assertAccepted(parse(mode, "/%40%2F%3A%5B%5D%21%24%26%27%28%29%2A%2B%2C%3D%3F"), + "/@%2F:%5B%5D!$&'()*+,=%3F"); } @ParameterizedTest @@ -496,6 +513,35 @@ void testToString(Mode mode) { } } + @Test + void testRemoveMatrixVariables() { + assertThat(removeMatrixVariables("/foo")).isEqualTo("/foo"); + assertThat(removeMatrixVariables("/foo;")).isEqualTo("/foo"); + assertThat(removeMatrixVariables("/foo/")).isEqualTo("/foo/"); + assertThat(removeMatrixVariables("/foo/bar")).isEqualTo("/foo/bar"); + assertThat(removeMatrixVariables("/foo/bar;")).isEqualTo("/foo/bar"); + assertThat(removeMatrixVariables("/foo/bar/")).isEqualTo("/foo/bar/"); + assertThat(removeMatrixVariables("/foo;/bar")).isEqualTo("/foo/bar"); + assertThat(removeMatrixVariables("/foo;/bar;")).isEqualTo("/foo/bar"); + assertThat(removeMatrixVariables("/foo;/bar/")).isEqualTo("/foo/bar/"); + assertThat(removeMatrixVariables("/foo;a=b/bar")).isEqualTo("/foo/bar"); + assertThat(removeMatrixVariables("/foo;a=b/bar;")).isEqualTo("/foo/bar"); + assertThat(removeMatrixVariables("/foo;a=b/bar/")).isEqualTo("/foo/bar/"); + assertThat(removeMatrixVariables("/foo;a=b/bar/baz")).isEqualTo("/foo/bar/baz"); + assertThat(removeMatrixVariables("/foo;a=b/bar/baz;")).isEqualTo("/foo/bar/baz"); + assertThat(removeMatrixVariables("/foo;a=b/bar/baz/")).isEqualTo("/foo/bar/baz/"); + assertThat(removeMatrixVariables("/foo;a=b/bar;/baz")).isEqualTo("/foo/bar/baz"); + assertThat(removeMatrixVariables("/foo;a=b/bar;/baz;")).isEqualTo("/foo/bar/baz"); + assertThat(removeMatrixVariables("/foo;a=b/bar;/baz/")).isEqualTo("/foo/bar/baz/"); + assertThat(removeMatrixVariables("/foo;a=b/bar;c=d/baz")).isEqualTo("/foo/bar/baz"); + assertThat(removeMatrixVariables("/foo;a=b/bar;c=d/baz;")).isEqualTo("/foo/bar/baz"); + assertThat(removeMatrixVariables("/foo;a=b/bar;c=d/baz/")).isEqualTo("/foo/bar/baz/"); + + // Invalid + assertThat(removeMatrixVariables("/;a=b")).isNull(); + assertThat(removeMatrixVariables("/prefix/;a=b")).isNull(); + } + private static void assertAccepted(@Nullable RequestTarget res, String expectedPath) { assertAccepted(res, expectedPath, null, null); } @@ -538,8 +584,8 @@ private static RequestTarget forServer(String rawPath) { } @Nullable - private static RequestTarget forServer(String rawPath, boolean allowDoubleDotsInQueryString) { - final RequestTarget res = DefaultRequestTarget.forServer(rawPath, allowDoubleDotsInQueryString); + private static RequestTarget forServer(String rawPath, boolean allowSemicolonInPathComponent) { + final RequestTarget res = DefaultRequestTarget.forServer(rawPath, allowSemicolonInPathComponent, false); if (res != null) { logger.info("forServer({}) => path: {}, query: {}", rawPath, res.path(), res.query()); } else { diff --git a/core/src/test/java/com/linecorp/armeria/server/MatrixVariablesTest.java b/core/src/test/java/com/linecorp/armeria/server/MatrixVariablesTest.java new file mode 100644 index 00000000000..e2efcdd231b --- /dev/null +++ b/core/src/test/java/com/linecorp/armeria/server/MatrixVariablesTest.java @@ -0,0 +1,49 @@ +/* + * 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.server; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import com.linecorp.armeria.common.AggregatedHttpResponse; +import com.linecorp.armeria.common.HttpResponse; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.testing.junit5.server.ServerExtension; +import com.linecorp.armeria.testing.server.ServiceRequestContextCaptor; + +class MatrixVariablesTest { + @RegisterExtension + static final ServerExtension server = new ServerExtension() { + @Override + protected void configure(ServerBuilder sb) throws Exception { + sb.service("/foo", (ctx, req) -> HttpResponse.of(200)); + } + }; + + @Test + void stripMatrixVariables() throws InterruptedException { + final AggregatedHttpResponse response = server.blockingWebClient().get("/foo;a=b"); + assertThat(response.headers().status()).isSameAs(HttpStatus.OK); + final ServiceRequestContextCaptor captor = server.requestContextCaptor(); + final ServiceRequestContext sctx = captor.poll(); + assertThat(sctx.path()).isEqualTo("/foo"); + assertThat(sctx.routingContext().requestTarget().maybePathWithMatrixVariables()) + .isEqualTo("/foo;a=b"); + } +} 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 1798e03cd0e..e50c78d00cd 100644 --- a/core/src/test/java/com/linecorp/armeria/server/RouteTest.java +++ b/core/src/test/java/com/linecorp/armeria/server/RouteTest.java @@ -176,6 +176,20 @@ void invalidRoutePath() { assertThatThrownBy(() -> Route.builder().path("foo:/bar")).isInstanceOf(IllegalArgumentException.class); } + @Test + void notAllowSemicolon() { + assertThatThrownBy(() -> Route.builder().path("/foo;")).isInstanceOf( + IllegalArgumentException.class); + assertThatThrownBy(() -> Route.builder().path("/foo/{bar};")).isInstanceOf( + IllegalArgumentException.class); + assertThatThrownBy(() -> Route.builder().path("/bar/:baz;")).isInstanceOf( + IllegalArgumentException.class); + assertThatThrownBy(() -> Route.builder().path("exact:/:foo/bar;")).isInstanceOf( + IllegalArgumentException.class); + assertThatThrownBy(() -> Route.builder().path("prefix:/bar/baz;")).isInstanceOf( + IllegalArgumentException.class); + } + @Test void testHeader() { final Route route = Route.builder() diff --git a/it/spring/boot3-jetty11/build.gradle b/it/spring/boot3-jetty11/build.gradle new file mode 100644 index 00000000000..552a270ba07 --- /dev/null +++ b/it/spring/boot3-jetty11/build.gradle @@ -0,0 +1,12 @@ +dependencies { + implementation project(':spring:boot3-starter') + implementation project(':spring:boot3-actuator-starter') + implementation project(':jetty11') + implementation libs.slf4j2.api + implementation libs.spring.boot3.starter.jetty + implementation(libs.spring.boot3.starter.web) { + exclude group: 'org.springframework.boot', module: 'spring-boot-starter-tomcat' + } + implementation libs.spring6.web + testImplementation libs.spring.boot3.starter.test +} diff --git a/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/ErrorHandlingController.java b/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/ErrorHandlingController.java new file mode 100644 index 00000000000..458e555acc5 --- /dev/null +++ b/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/ErrorHandlingController.java @@ -0,0 +1,72 @@ +/* + * Copyright 2021 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.spring.jetty; + +import java.util.Map; + +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.ResponseStatus; +import org.springframework.web.bind.annotation.RestController; + +import com.google.common.collect.ImmutableMap; + +@RestController +@RequestMapping("/error-handling") +public class ErrorHandlingController { + + @GetMapping("/runtime-exception") + public void runtimeException() { + throw new RuntimeException("runtime exception"); + } + + @GetMapping("/custom-exception") + public void customException() { + throw new CustomException(); + } + + @GetMapping("/exception-handler") + public void exceptionHandler() { + throw new BaseException("exception handler"); + } + + @GetMapping("/global-exception-handler") + public void globalExceptionHandler() { + throw new GlobalBaseException("global exception handler"); + } + + @ResponseStatus(code = HttpStatus.NOT_FOUND, reason = "custom not found") + private static class CustomException extends RuntimeException {} + + private static class BaseException extends RuntimeException { + BaseException(String message) { + super(message); + } + } + + @ExceptionHandler(BaseException.class) + public ResponseEntity> onBaseException(Throwable t) { + final Map body = ImmutableMap.builder() + .put("status", HttpStatus.INTERNAL_SERVER_ERROR.value()) + .put("message", t.getMessage()) + .build(); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(body); + } +} diff --git a/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/GlobalBaseException.java b/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/GlobalBaseException.java new file mode 100644 index 00000000000..a7c126823ac --- /dev/null +++ b/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/GlobalBaseException.java @@ -0,0 +1,23 @@ +/* + * Copyright 2021 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.spring.jetty; + +public class GlobalBaseException extends RuntimeException { + GlobalBaseException(String message) { + super(message); + } +} diff --git a/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/GlobalExceptionHandler.java b/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/GlobalExceptionHandler.java new file mode 100644 index 00000000000..30b7163d163 --- /dev/null +++ b/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/GlobalExceptionHandler.java @@ -0,0 +1,38 @@ +/* + * Copyright 2021 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.spring.jetty; + +import java.util.Map; + +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.bind.annotation.RestControllerAdvice; + +import com.google.common.collect.ImmutableMap; + +@RestControllerAdvice +class GlobalExceptionHandler { + + @ExceptionHandler(GlobalBaseException.class) + public ResponseEntity> onGlobalBaseException(Throwable t) { + final String message = t.getMessage(); + final Map body = ImmutableMap.of("status", HttpStatus.INTERNAL_SERVER_ERROR.value(), + "message", message); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(body); + } +} diff --git a/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/Greeting.java b/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/Greeting.java new file mode 100644 index 00000000000..21acb6ed03c --- /dev/null +++ b/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/Greeting.java @@ -0,0 +1,34 @@ +/* + * Copyright 2018 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.spring.jetty; + +public class Greeting { + + private final String content; + + /** + * Greeting model. + * @param content the content. + */ + public Greeting(String content) { + this.content = content; + } + + public String getContent() { + return content; + } +} diff --git a/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/GreetingController.java b/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/GreetingController.java new file mode 100644 index 00000000000..4065a1f8ea6 --- /dev/null +++ b/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/GreetingController.java @@ -0,0 +1,40 @@ +/* + * Copyright 2018 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.spring.jetty; + +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.RestController; + +@RestController +@RequestMapping("/greeting") +public class GreetingController { + private static final String template = "Hello, %s!"; + + /** + * Greeting endpoint. + * @param name name to greet. + * @return response the ResponseEntity. + */ + @GetMapping + public ResponseEntity greetingSync( + @RequestParam(value = "name", defaultValue = "World") String name) { + return ResponseEntity.ok(new Greeting(String.format(template, name))); + } +} diff --git a/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/MatrixVariablesController.java b/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/MatrixVariablesController.java new file mode 100644 index 00000000000..07a01d7cb30 --- /dev/null +++ b/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/MatrixVariablesController.java @@ -0,0 +1,39 @@ +/* + * 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.spring.jetty; + +import java.util.List; + +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.MatrixVariable; +import org.springframework.web.bind.annotation.RestController; + +import com.google.common.collect.ImmutableList; + +@RestController +public final class MatrixVariablesController { + + // GET /owners/42;q=11/pets/21;q=22 + // q1 = 11, q2 = 22 + + @GetMapping("/owners/{ownerId}/pets/{petId}") + List findPet( + @MatrixVariable(name = "q", pathVar = "ownerId") int q1, + @MatrixVariable(name = "q", pathVar = "petId") int q2) { + return ImmutableList.of(q1, q2); + } +} diff --git a/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/SpringJettyApplication.java b/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/SpringJettyApplication.java new file mode 100644 index 00000000000..0680be9038f --- /dev/null +++ b/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/SpringJettyApplication.java @@ -0,0 +1,85 @@ +/* + * Copyright 2018 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.spring.jetty; + +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.util.Loader; +import org.eclipse.jetty.webapp.WebAppContext; +import org.springframework.beans.factory.ObjectProvider; +import org.springframework.boot.SpringApplication; +import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; +import org.springframework.boot.web.context.WebServerApplicationContext; +import org.springframework.boot.web.embedded.jetty.JettyServerCustomizer; +import org.springframework.boot.web.embedded.jetty.JettyServletWebServerFactory; +import org.springframework.boot.web.embedded.jetty.JettyWebServer; +import org.springframework.boot.web.server.WebServer; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +import com.linecorp.armeria.server.jetty.JettyService; +import com.linecorp.armeria.spring.ArmeriaServerConfigurator; + +import jakarta.servlet.Servlet; + +@SpringBootApplication +public class SpringJettyApplication { + + /** + * Bean to configure Armeria Jetty service. + */ + @Bean + public ArmeriaServerConfigurator armeriaTomcat(WebServerApplicationContext applicationContext) { + final WebServer webServer = applicationContext.getWebServer(); + if (webServer instanceof JettyWebServer) { + final Server jettyServer = ((JettyWebServer) webServer).getServer(); + + return serverBuilder -> serverBuilder.service("prefix:/jetty/api/rest/v1", + JettyService.of(jettyServer)); + } + return serverBuilder -> {}; + } + + @Configuration(proxyBeanMethods = false) + @ConditionalOnClass({ Servlet.class, Server.class, Loader.class, WebAppContext.class }) + static class EmbeddedJetty { + + @Bean + JettyServletWebServerFactory jettyServletWebServerFactory( + ObjectProvider serverCustomizers) { + final JettyServletWebServerFactory factory = new ArmeriaJettyServletWebServerFactory(); + factory.getServerCustomizers().addAll(serverCustomizers.orderedStream().toList()); + return factory; + } + } + + static final class ArmeriaJettyServletWebServerFactory extends JettyServletWebServerFactory { + + @Override + protected JettyWebServer getJettyWebServer(Server server) { + return new JettyWebServer(server, true); + } + } + + /** + * Main method. + * @param args program args. + */ + public static void main(String[] args) { + SpringApplication.run(SpringJettyApplication.class, args); + } +} diff --git a/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/package-info.java b/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/package-info.java new file mode 100644 index 00000000000..8ccb0168cd6 --- /dev/null +++ b/it/spring/boot3-jetty11/src/main/java/com/linecorp/armeria/spring/jetty/package-info.java @@ -0,0 +1,20 @@ +/* + * Copyright 2018 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. + */ + +@NonNullByDefault +package com.linecorp.armeria.spring.jetty; + +import com.linecorp.armeria.common.annotation.NonNullByDefault; diff --git a/it/spring/boot3-jetty11/src/main/resources/application.properties b/it/spring/boot3-jetty11/src/main/resources/application.properties new file mode 100644 index 00000000000..e69de29bb2d diff --git a/it/spring/boot3-jetty11/src/main/resources/config/application.yml b/it/spring/boot3-jetty11/src/main/resources/config/application.yml new file mode 100644 index 00000000000..70f70399462 --- /dev/null +++ b/it/spring/boot3-jetty11/src/main/resources/config/application.yml @@ -0,0 +1,7 @@ +armeria: + ports: + - port: 0 + protocols: HTTP +server: + error: + include-message: always diff --git a/it/spring/boot3-jetty11/src/test/java/com/linecorp/armeria/spring/jetty/ActuatorAutoConfigurationHealthGroupTest.java b/it/spring/boot3-jetty11/src/test/java/com/linecorp/armeria/spring/jetty/ActuatorAutoConfigurationHealthGroupTest.java new file mode 100644 index 00000000000..4101704613b --- /dev/null +++ b/it/spring/boot3-jetty11/src/test/java/com/linecorp/armeria/spring/jetty/ActuatorAutoConfigurationHealthGroupTest.java @@ -0,0 +1,98 @@ +/* + * Copyright 2022 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.spring.jetty; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.IOException; +import java.util.Map; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.autoconfigure.actuate.metrics.AutoConfigureMetrics; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; +import org.springframework.boot.test.web.server.LocalManagementPort; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.ActiveProfiles; + +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; + +import com.linecorp.armeria.client.WebClient; +import com.linecorp.armeria.common.AggregatedHttpResponse; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.spring.LocalArmeriaPort; + +@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT) +@ActiveProfiles({ "local", "healthGroupTest" }) +@DirtiesContext +@AutoConfigureMetrics +@EnableAutoConfiguration +class ActuatorAutoConfigurationHealthGroupTest { + + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + private static final TypeReference> JSON_MAP = new TypeReference<>() {}; + + @LocalManagementPort + private int managementPort; + @LocalArmeriaPort + private int armeriaPort; + + private WebClient managementClient; + private WebClient armeriaClient; + + @BeforeEach + void setUp() { + managementClient = WebClient.builder("http://127.0.0.1:" + managementPort).build(); + armeriaClient = WebClient.builder("http://127.0.0.1:" + armeriaPort).build(); + } + + @Test + void testHealth() throws Exception { + final AggregatedHttpResponse res = managementClient.get("/internal/actuator/health").aggregate().join(); + assertUpStatus(res); + } + + @Test + void additionalPath() throws Exception { + String path = "/internal/actuator/health/foo"; + assertUpStatus(managementClient.get(path).aggregate().join()); + assertThat(armeriaClient.get(path).aggregate().join().status()).isSameAs(HttpStatus.NOT_FOUND); + + path = "/internal/actuator/health/bar"; + assertUpStatus(managementClient.get(path).aggregate().join()); + assertThat(armeriaClient.get(path).aggregate().join().status()).isSameAs(HttpStatus.NOT_FOUND); + + path = "/foohealth"; + assertUpStatus(managementClient.get(path).aggregate().join()); + assertThat(armeriaClient.get(path).aggregate().join().status()).isSameAs(HttpStatus.NOT_FOUND); + + // barhealth is bound to Armeria port. + path = "/barhealth"; + assertThat(managementClient.get(path).aggregate().join().status()).isSameAs(HttpStatus.NOT_FOUND); + assertUpStatus(armeriaClient.get(path).aggregate().join()); + } + + private static void assertUpStatus(AggregatedHttpResponse res) throws IOException { + assertThat(res.status()).isEqualTo(HttpStatus.OK); + assertThat(res.contentType().toString()).isEqualTo("application/vnd.spring-boot.actuator.v3+json"); + + final Map values = OBJECT_MAPPER.readValue(res.content().array(), JSON_MAP); + assertThat(values).containsEntry("status", "UP"); + } +} diff --git a/it/spring/boot3-jetty11/src/test/java/com/linecorp/armeria/spring/jetty/ErrorHandlingTest.java b/it/spring/boot3-jetty11/src/test/java/com/linecorp/armeria/spring/jetty/ErrorHandlingTest.java new file mode 100644 index 00000000000..771f3d50153 --- /dev/null +++ b/it/spring/boot3-jetty11/src/test/java/com/linecorp/armeria/spring/jetty/ErrorHandlingTest.java @@ -0,0 +1,63 @@ +/* + * Copyright 2021 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.spring.jetty; + +import static com.linecorp.armeria.spring.jetty.MatrixVariablesTest.JETTY_BASE_PATH; +import static net.javacrumbs.jsonunit.fluent.JsonFluentAssert.assertThatJson; +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; +import org.springframework.boot.test.web.client.TestRestTemplate; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; + +import com.linecorp.armeria.spring.LocalArmeriaPort; + +import jakarta.inject.Inject; + +@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT) +class ErrorHandlingTest { + + @LocalArmeriaPort + private int port; + + @Inject + private TestRestTemplate restTemplate; + + private static String jettyBaseUrlPath(int port) { + return "http://localhost:" + port + JETTY_BASE_PATH; + } + + @ParameterizedTest + @CsvSource({ + "/error-handling/runtime-exception, 500, jakarta.servlet.ServletException: " + + "Request processing failed: java.lang.RuntimeException: runtime exception", + "/error-handling/custom-exception, 404, custom not found", + "/error-handling/exception-handler, 500, exception handler", + "/error-handling/global-exception-handler, 500, global exception handler" + }) + void shouldReturnFormattedMessage(String path, int status, String message) throws Exception { + final ResponseEntity response = + restTemplate.getForEntity(jettyBaseUrlPath(port) + path, String.class); + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.valueOf(status)); + assertThatJson(response.getBody()).node("status").isEqualTo(status); + assertThatJson(response.getBody()).node("message").isEqualTo(message); + } +} diff --git a/it/spring/boot3-jetty11/src/test/java/com/linecorp/armeria/spring/jetty/MatrixVariablesTest.java b/it/spring/boot3-jetty11/src/test/java/com/linecorp/armeria/spring/jetty/MatrixVariablesTest.java new file mode 100644 index 00000000000..08702be7bfc --- /dev/null +++ b/it/spring/boot3-jetty11/src/test/java/com/linecorp/armeria/spring/jetty/MatrixVariablesTest.java @@ -0,0 +1,60 @@ +/* + * 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.spring.jetty; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; +import org.springframework.test.context.ActiveProfiles; + +import com.linecorp.armeria.client.WebClient; +import com.linecorp.armeria.common.AggregatedHttpResponse; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.spring.LocalArmeriaPort; + +/** + * Integration test for Matrix Variables. + */ +@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT) +@ActiveProfiles("testbed") +class MatrixVariablesTest { + + static final String JETTY_BASE_PATH = "/jetty/api/rest/v1"; + + @LocalArmeriaPort + int port; + + @Test + void matrixVariablesPreserved() throws Exception { + final WebClient client = WebClient.of("http://127.0.0.1:" + port); + final AggregatedHttpResponse response = client.blocking().get( + JETTY_BASE_PATH + "/owners/42;q=11/pets/21;q=22"); + assertThat(response.contentUtf8()).isEqualTo("[11,22]"); + } + + @Test + void wrongMatrixVariables() throws Exception { + final WebClient client = WebClient.of("http://127.0.0.1:" + port); + AggregatedHttpResponse response = client.blocking().get( + JETTY_BASE_PATH + ";/owners/42;q=11/pets/21;q=22"); + assertThat(response.status()).isSameAs(HttpStatus.BAD_REQUEST); + + response = client.blocking().get("/jetty;wrong=place/api/rest/v1/owners/42;q=11/pets/21;q=22"); + assertThat(response.status()).isSameAs(HttpStatus.BAD_REQUEST); + } +} diff --git a/it/spring/boot3-jetty11/src/test/java/com/linecorp/armeria/spring/jetty/SpringJettyApplicationItTest.java b/it/spring/boot3-jetty11/src/test/java/com/linecorp/armeria/spring/jetty/SpringJettyApplicationItTest.java new file mode 100644 index 00000000000..6884e5c1629 --- /dev/null +++ b/it/spring/boot3-jetty11/src/test/java/com/linecorp/armeria/spring/jetty/SpringJettyApplicationItTest.java @@ -0,0 +1,86 @@ +/* + * Copyright 2018 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.spring.jetty; + +import static com.linecorp.armeria.spring.jetty.MatrixVariablesTest.JETTY_BASE_PATH; +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; +import org.springframework.boot.test.web.client.TestRestTemplate; +import org.springframework.context.ApplicationContext; +import org.springframework.test.context.ActiveProfiles; + +import com.linecorp.armeria.server.Server; +import com.linecorp.armeria.server.ServerPort; + +import jakarta.inject.Inject; + +@ActiveProfiles("testbed") +@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT) +class SpringJettyApplicationItTest { + @Inject + private ApplicationContext applicationContext; + @Inject + private Server server; + private int httpPort; + @Inject + private TestRestTemplate restTemplate; + @Inject + private GreetingController greetingController; + + @BeforeEach + public void init() throws Exception { + httpPort = server.activePorts() + .values() + .stream() + .filter(ServerPort::hasHttp) + .findAny() + .get() + .localAddress() + .getPort(); + } + + @Test + void contextLoads() { + assertThat(greetingController).isNotNull(); + } + + @Test + void greetingShouldReturnDefaultMessage() throws Exception { + assertThat(restTemplate.getForObject("http://localhost:" + httpPort + JETTY_BASE_PATH + "/greeting", + String.class)) + .contains("Hello, World!"); + } + + @Test + void greetingShouldReturnUsersMessage() throws Exception { + assertThat(restTemplate.getForObject("http://localhost:" + httpPort + + JETTY_BASE_PATH + "/greeting?name=Armeria", + String.class)) + .contains("Hello, Armeria!"); + } + + @Test + void greetingShouldReturn404() throws Exception { + assertThat(restTemplate.getForEntity("http://localhost:" + httpPort + JETTY_BASE_PATH + "/greet", + Void.class) + .getStatusCode().value()).isEqualByComparingTo(404); + } +} diff --git a/it/spring/boot3-jetty11/src/test/resources/application-healthGroupTest.yml b/it/spring/boot3-jetty11/src/test/resources/application-healthGroupTest.yml new file mode 100644 index 00000000000..cc78089aa53 --- /dev/null +++ b/it/spring/boot3-jetty11/src/test/resources/application-healthGroupTest.yml @@ -0,0 +1,23 @@ +armeria: + ports: + - port: 0 + protocols: HTTP + +management: + server: + port: 0 + endpoints: + web: + exposure: + include: health, prometheus + base-path: /internal/actuator + endpoint: + health: + group: + foo: + include: ping + additional-path: "management:/foohealth" + bar: + include: ping + additional-path: "server:/barhealth" + diff --git a/it/spring/boot3-jetty11/src/test/resources/application-testbed.yml b/it/spring/boot3-jetty11/src/test/resources/application-testbed.yml new file mode 100644 index 00000000000..508748bac3a --- /dev/null +++ b/it/spring/boot3-jetty11/src/test/resources/application-testbed.yml @@ -0,0 +1,7 @@ +# This currently doesn't work. See https://github.com/line/armeria/issues/5039 +server.port: -1 +--- +armeria: + ports: + - port: 0 + protocols: HTTP diff --git a/it/spring/boot3-tomcat10/src/main/java/com/linecorp/armeria/spring/tomcat/MatrixVariablesController.java b/it/spring/boot3-tomcat10/src/main/java/com/linecorp/armeria/spring/tomcat/MatrixVariablesController.java new file mode 100644 index 00000000000..227c45452b1 --- /dev/null +++ b/it/spring/boot3-tomcat10/src/main/java/com/linecorp/armeria/spring/tomcat/MatrixVariablesController.java @@ -0,0 +1,39 @@ +/* + * 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.spring.tomcat; + +import java.util.List; + +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.MatrixVariable; +import org.springframework.web.bind.annotation.RestController; + +import com.google.common.collect.ImmutableList; + +@RestController +public final class MatrixVariablesController { + + // GET /owners/42;q=11/pets/21;q=22 + // q1 = 11, q2 = 22 + + @GetMapping("/owners/{ownerId}/pets/{petId}") + List findPet( + @MatrixVariable(name = "q", pathVar = "ownerId") int q1, + @MatrixVariable(name = "q", pathVar = "petId") int q2) { + return ImmutableList.of(q1, q2); + } +} diff --git a/it/spring/boot3-tomcat10/src/test/java/com/linecorp/armeria/spring/tomcat/ActuatorAutoConfigurationHealthGroupTest.java b/it/spring/boot3-tomcat10/src/test/java/com/linecorp/armeria/spring/tomcat/ActuatorAutoConfigurationHealthGroupTest.java index 5a0caa085da..5befaff6216 100644 --- a/it/spring/boot3-tomcat10/src/test/java/com/linecorp/armeria/spring/tomcat/ActuatorAutoConfigurationHealthGroupTest.java +++ b/it/spring/boot3-tomcat10/src/test/java/com/linecorp/armeria/spring/tomcat/ActuatorAutoConfigurationHealthGroupTest.java @@ -46,8 +46,7 @@ class ActuatorAutoConfigurationHealthGroupTest { private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); - private static final TypeReference> JSON_MAP = - new TypeReference>() {}; + private static final TypeReference> JSON_MAP = new TypeReference<>() {}; @LocalManagementPort private int managementPort; diff --git a/it/spring/boot3-tomcat10/src/test/java/com/linecorp/armeria/spring/tomcat/ErrorHandlingTest.java b/it/spring/boot3-tomcat10/src/test/java/com/linecorp/armeria/spring/tomcat/ErrorHandlingTest.java index fbd578beaf1..0b40ba25f24 100644 --- a/it/spring/boot3-tomcat10/src/test/java/com/linecorp/armeria/spring/tomcat/ErrorHandlingTest.java +++ b/it/spring/boot3-tomcat10/src/test/java/com/linecorp/armeria/spring/tomcat/ErrorHandlingTest.java @@ -16,6 +16,7 @@ package com.linecorp.armeria.spring.tomcat; +import static com.linecorp.armeria.spring.tomcat.MatrixVariablesTest.TOMCAT_BASE_PATH; import static net.javacrumbs.jsonunit.fluent.JsonFluentAssert.assertThatJson; import static org.assertj.core.api.Assertions.assertThat; @@ -34,8 +35,6 @@ @SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT) class ErrorHandlingTest { - private static final String TOMCAT_BASE_PATH = "/tomcat/api/rest/v1"; - @LocalArmeriaPort private int port; diff --git a/it/spring/boot3-tomcat10/src/test/java/com/linecorp/armeria/spring/tomcat/MatrixVariablesTest.java b/it/spring/boot3-tomcat10/src/test/java/com/linecorp/armeria/spring/tomcat/MatrixVariablesTest.java new file mode 100644 index 00000000000..f3f60a4d7c6 --- /dev/null +++ b/it/spring/boot3-tomcat10/src/test/java/com/linecorp/armeria/spring/tomcat/MatrixVariablesTest.java @@ -0,0 +1,60 @@ +/* + * 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.spring.tomcat; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; +import org.springframework.test.context.ActiveProfiles; + +import com.linecorp.armeria.client.WebClient; +import com.linecorp.armeria.common.AggregatedHttpResponse; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.spring.LocalArmeriaPort; + +/** + * Integration test for Matrix Variables. + */ +@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT) +@ActiveProfiles("testbed") +class MatrixVariablesTest { + + static final String TOMCAT_BASE_PATH = "/tomcat/api/rest/v1"; + + @LocalArmeriaPort + int port; + + @Test + void matrixVariablesPreserved() throws Exception { + final WebClient client = WebClient.of("http://127.0.0.1:" + port); + final AggregatedHttpResponse response = client.blocking().get( + TOMCAT_BASE_PATH + "/owners/42;q=11/pets/21;q=22"); + assertThat(response.contentUtf8()).isEqualTo("[11,22]"); + } + + @Test + void wrongMatrixVariables() throws Exception { + final WebClient client = WebClient.of("http://127.0.0.1:" + port); + AggregatedHttpResponse response = client.blocking().get( + TOMCAT_BASE_PATH + ";/owners/42;q=11/pets/21;q=22"); + assertThat(response.status()).isSameAs(HttpStatus.BAD_REQUEST); + + response = client.blocking().get("/tomcat;wrong=place/api/rest/v1/owners/42;q=11/pets/21;q=22"); + assertThat(response.status()).isSameAs(HttpStatus.BAD_REQUEST); + } +} diff --git a/it/spring/boot3-tomcat10/src/test/resources/application-testbed.yml b/it/spring/boot3-tomcat10/src/test/resources/application-testbed.yml new file mode 100644 index 00000000000..b1c8f5f29ec --- /dev/null +++ b/it/spring/boot3-tomcat10/src/test/resources/application-testbed.yml @@ -0,0 +1,7 @@ +# Prevent the embedded Tomcat from opening a TCP/IP port. +server.port: -1 +--- +armeria: + ports: + - port: 0 + protocols: HTTP diff --git a/jetty/jetty11/src/main/java/com/linecorp/armeria/server/jetty/JettyService.java b/jetty/jetty11/src/main/java/com/linecorp/armeria/server/jetty/JettyService.java index f91056a4228..b6f326a9ed2 100644 --- a/jetty/jetty11/src/main/java/com/linecorp/armeria/server/jetty/JettyService.java +++ b/jetty/jetty11/src/main/java/com/linecorp/armeria/server/jetty/JettyService.java @@ -16,6 +16,7 @@ package com.linecorp.armeria.server.jetty; +import static com.linecorp.armeria.internal.common.util.MappedPathUtil.mappedPath; import static java.util.Objects.requireNonNull; import java.nio.ByteBuffer; @@ -58,7 +59,9 @@ import com.linecorp.armeria.common.HttpResponse; import com.linecorp.armeria.common.HttpResponseWriter; 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.common.annotation.Nullable; @@ -211,6 +214,13 @@ void stop() { @Override public HttpResponse serve(ServiceRequestContext ctx, HttpRequest req) { + final String mappedPath = mappedPath(ctx); + if (mappedPath == null) { + return HttpResponse.of(HttpStatus.BAD_REQUEST, MediaType.PLAIN_TEXT_UTF_8, + "Invalid matrix variable: " + + ctx.routingContext().requestTarget().maybePathWithMatrixVariables()); + } + final ArmeriaConnector connector = this.connector; assert connector != null; @@ -242,7 +252,7 @@ public HttpResponse serve(ServiceRequestContext ctx, HttpRequest req) { }); final Request jReq = httpChannel.getRequest(); - fillRequest(ctx, aReq, jReq); + fillRequest(ctx, aReq, jReq, mappedPath); final SSLSession sslSession = ctx.sslSession(); final boolean needsReverseDnsLookup; if (sslSession != null) { @@ -284,11 +294,11 @@ public ExchangeType exchangeType(RoutingContext routingContext) { } private static void fillRequest( - ServiceRequestContext ctx, AggregatedHttpRequest aReq, Request jReq) { + ServiceRequestContext ctx, AggregatedHttpRequest aReq, Request jReq, String mappedPath) { DispatcherTypeUtil.setRequestType(jReq); jReq.setAsyncSupported(true, null); jReq.setSecure(ctx.sessionProtocol().isTls()); - jReq.setMetaData(toRequestMetadata(ctx, aReq)); + jReq.setMetaData(toRequestMetadata(ctx, aReq, mappedPath)); final HttpHeaders trailers = aReq.trailers(); if (!trailers.isEmpty()) { final HttpField[] httpFields = trailers.stream() @@ -298,7 +308,8 @@ private static void fillRequest( } } - private static MetaData.Request toRequestMetadata(ServiceRequestContext ctx, AggregatedHttpRequest aReq) { + private static MetaData.Request toRequestMetadata(ServiceRequestContext ctx, AggregatedHttpRequest aReq, + String mappedPath) { // Construct the HttpURI final StringBuilder uriBuf = new StringBuilder(); final RequestHeaders aHeaders = aReq.headers(); @@ -306,9 +317,14 @@ private static MetaData.Request toRequestMetadata(ServiceRequestContext ctx, Agg uriBuf.append(ctx.sessionProtocol().isTls() ? "https" : "http"); uriBuf.append("://"); uriBuf.append(aHeaders.authority()); - uriBuf.append(aHeaders.path()); - final HttpURI uri = HttpURI.build(HttpURI.build(uriBuf.toString()).path(ctx.mappedPath())) + final RequestTarget requestTarget = ctx.routingContext().requestTarget(); + if (requestTarget.query() != null) { + mappedPath = mappedPath + '?' + requestTarget.query(); + } + uriBuf.append(mappedPath); + + final HttpURI uri = HttpURI.build(HttpURI.build(uriBuf.toString())) .asImmutable(); final HttpField[] fields = aHeaders.stream().map(header -> { final AsciiString k = header.getKey(); diff --git a/jetty/jetty9/src/main/java/com/linecorp/armeria/server/jetty/JettyService.java b/jetty/jetty9/src/main/java/com/linecorp/armeria/server/jetty/JettyService.java index 42eeca0cbc3..6bfc53fee4f 100644 --- a/jetty/jetty9/src/main/java/com/linecorp/armeria/server/jetty/JettyService.java +++ b/jetty/jetty9/src/main/java/com/linecorp/armeria/server/jetty/JettyService.java @@ -17,6 +17,7 @@ package com.linecorp.armeria.server.jetty; import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.toHttp1Headers; +import static com.linecorp.armeria.internal.common.util.MappedPathUtil.mappedPath; import static java.util.Objects.requireNonNull; import java.lang.invoke.MethodHandle; @@ -54,6 +55,7 @@ import com.linecorp.armeria.common.HttpResponse; import com.linecorp.armeria.common.HttpResponseWriter; import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.common.MediaType; import com.linecorp.armeria.common.RequestHeaders; import com.linecorp.armeria.common.ResponseHeaders; import com.linecorp.armeria.common.ResponseHeadersBuilder; @@ -256,6 +258,12 @@ void stop() { @Override public HttpResponse serve(ServiceRequestContext ctx, HttpRequest req) { + final String mappedPath = mappedPath(ctx); + if (mappedPath == null) { + return HttpResponse.of(HttpStatus.BAD_REQUEST, MediaType.PLAIN_TEXT_UTF_8, + "Invalid matrix variable: " + + ctx.routingContext().requestTarget().maybePathWithMatrixVariables()); + } final ArmeriaConnector connector = this.connector; assert connector != null; diff --git a/settings.gradle b/settings.gradle index a6e1a4d2f8e..3b6ef6adace 100644 --- a/settings.gradle +++ b/settings.gradle @@ -144,6 +144,7 @@ includeWithFlags ':it:nio', 'java', 'relocate includeWithFlags ':it:okhttp', 'java', 'relocate' includeWithFlags ':it:resilience4j', 'java17', 'relocate' includeWithFlags ':it:server', 'java', 'relocate' +includeWithFlags ':it:spring:boot3-jetty11', 'java17', 'relocate' includeWithFlags ':it:spring:boot3-kotlin', 'java17', 'relocate', 'kotlin' includeWithFlags ':it:spring:boot3-mixed', 'java17', 'relocate' includeWithFlags ':it:spring:boot3-mixed-tomcat10', 'java17', 'relocate' diff --git a/spring/boot3-webflux-autoconfigure/build.gradle b/spring/boot3-webflux-autoconfigure/build.gradle index afbf8a8a806..9fc879e1c89 100644 --- a/spring/boot3-webflux-autoconfigure/build.gradle +++ b/spring/boot3-webflux-autoconfigure/build.gradle @@ -29,6 +29,7 @@ dependencies { testImplementation project(':spring:boot3-actuator-autoconfigure') testImplementation project(':thrift0.18') testImplementation libs.spring.boot3.starter.test + testImplementation libs.spring6.web // Added for sharing test suites with boot2 testImplementation libs.javax.inject } diff --git a/spring/boot3-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequest.java b/spring/boot3-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequest.java index d75ccd121d8..9a60200e8c5 100644 --- a/spring/boot3-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequest.java +++ b/spring/boot3-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequest.java @@ -41,6 +41,7 @@ import com.linecorp.armeria.common.HttpHeaderNames; import com.linecorp.armeria.common.HttpRequest; import com.linecorp.armeria.common.RequestHeaders; +import com.linecorp.armeria.common.RequestTarget; import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.server.ServiceRequestContext; @@ -60,7 +61,7 @@ final class ArmeriaServerHttpRequest extends AbstractServerHttpRequest { ArmeriaServerHttpRequest(ServiceRequestContext ctx, HttpRequest req, DataBufferFactoryWrapper factoryWrapper) { - super(uri(req), null, springHeaders(req.headers())); + super(uri(ctx, req), null, springHeaders(req.headers())); this.ctx = requireNonNull(ctx, "ctx"); this.req = req; @@ -76,13 +77,18 @@ private static HttpHeaders springHeaders(RequestHeaders headers) { return springHeaders; } - private static URI uri(HttpRequest req) { + private static URI uri(ServiceRequestContext ctx, HttpRequest req) { final String scheme = req.scheme(); final String authority = req.authority(); // Server side Armeria HTTP request always has the scheme and authority. assert scheme != null; assert authority != null; - return URI.create(scheme + "://" + authority + req.path()); + final RequestTarget requestTarget = ctx.routingContext().requestTarget(); + String path = requestTarget.maybePathWithMatrixVariables(); + if (requestTarget.query() != null) { + path = path + '?' + requestTarget.query(); + } + return URI.create(scheme + "://" + authority + path); } @Override diff --git a/spring/boot3-webflux-autoconfigure/src/test/java/com/linecorp/armeria/spring/web/reactive/MatrixVariablesTest.java b/spring/boot3-webflux-autoconfigure/src/test/java/com/linecorp/armeria/spring/web/reactive/MatrixVariablesTest.java new file mode 100644 index 00000000000..f952c2638b9 --- /dev/null +++ b/spring/boot3-webflux-autoconfigure/src/test/java/com/linecorp/armeria/spring/web/reactive/MatrixVariablesTest.java @@ -0,0 +1,68 @@ +/* + * 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.spring.web.reactive; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; +import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; +import org.springframework.boot.test.web.server.LocalServerPort; +import org.springframework.context.annotation.Configuration; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.MatrixVariable; +import org.springframework.web.bind.annotation.RestController; + +import com.linecorp.armeria.client.WebClient; +import com.linecorp.armeria.common.AggregatedHttpResponse; + +import reactor.core.publisher.Flux; + +/** + * Integration test for Matrix Variables. + */ +@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT) +class MatrixVariablesTest { + + @SpringBootApplication + @Configuration + static class TestConfiguration { + @RestController + static class TestController { + + // GET /owners/42;q=11/pets/21;q=22 + // q1 = 11, q2 = 22 + + @GetMapping("/owners/{ownerId}/pets/{petId}") + Flux findPet( + @MatrixVariable(name = "q", pathVar = "ownerId") int q1, + @MatrixVariable(name = "q", pathVar = "petId") int q2) { + return Flux.just(q1, q2); + } + } + } + + @LocalServerPort + int port; + + @Test + void foo() throws Exception { + final WebClient client = WebClient.of("http://127.0.0.1:" + port); + final AggregatedHttpResponse response = client.blocking().get("/owners/42;q=11/pets/21;q=22"); + assertThat(response.contentUtf8()).isEqualTo("[11,22]"); + } +} diff --git a/tomcat10/src/main/java/com/linecorp/armeria/server/tomcat/TomcatService.java b/tomcat10/src/main/java/com/linecorp/armeria/server/tomcat/TomcatService.java index 2eaeb273e96..9369d559168 100644 --- a/tomcat10/src/main/java/com/linecorp/armeria/server/tomcat/TomcatService.java +++ b/tomcat10/src/main/java/com/linecorp/armeria/server/tomcat/TomcatService.java @@ -17,6 +17,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.toHttp1Headers; +import static com.linecorp.armeria.internal.common.util.MappedPathUtil.mappedPath; import static java.util.Objects.requireNonNull; import java.io.File; @@ -378,6 +379,13 @@ public final HttpResponse serve(ServiceRequestContext ctx, HttpRequest req) thro return HttpResponse.of(HttpStatus.SERVICE_UNAVAILABLE); } + final String mappedPath = mappedPath(ctx); + if (mappedPath == null) { + return HttpResponse.of(HttpStatus.BAD_REQUEST, MediaType.PLAIN_TEXT_UTF_8, + "Invalid matrix variable: " + + ctx.routingContext().requestTarget().maybePathWithMatrixVariables()); + } + final HttpResponseWriter res = HttpResponse.streaming(); req.aggregate().handle((aReq, cause) -> { try { @@ -396,7 +404,7 @@ public final HttpResponse serve(ServiceRequestContext ctx, HttpRequest req) thro } final ArmeriaProcessor processor = createProcessor(coyoteAdapter); - final Request coyoteReq = convertRequest(ctx, aReq, processor.getRequest()); + final Request coyoteReq = convertRequest(ctx, mappedPath, aReq, processor.getRequest()); if (coyoteReq == null) { if (res.tryWrite(INVALID_AUTHORITY_HEADERS)) { if (res.tryWrite(INVALID_AUTHORITY_DATA)) { @@ -471,10 +479,8 @@ private static ArmeriaProcessor createProcessor(Adapter coyoteAdapter) throws Th } @Nullable - private Request convertRequest(ServiceRequestContext ctx, AggregatedHttpRequest req, + private Request convertRequest(ServiceRequestContext ctx, String mappedPath, AggregatedHttpRequest req, Request coyoteReq) throws Throwable { - final String mappedPath = ctx.mappedPath(); - coyoteReq.scheme().setString(req.scheme()); // Set the start time which is used by Tomcat access logging