From 73eafc159c6f38f6c52856b80c77150d2f1c5fe1 Mon Sep 17 00:00:00 2001 From: Sean Yang Date: Mon, 25 Nov 2024 12:43:41 +0800 Subject: [PATCH] Fix route matching order mistake --- .../filter/RestExtensionExecutionFilter.java | 2 +- .../rest/mapping/condition/PathCondition.java | 4 +++- .../rest/mapping/condition/PathExpression.java | 2 +- .../tri/rest/mapping/condition/PathSegment.java | 12 ++++++------ .../tri/rest/openapi/DefaultOpenAPIService.java | 16 ++++++++-------- .../tri/rest/openapi/SchemaResolver.java | 2 +- .../mapping/condition/PathExpressionTest.groovy | 1 + 7 files changed, 21 insertions(+), 18 deletions(-) diff --git a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/filter/RestExtensionExecutionFilter.java b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/filter/RestExtensionExecutionFilter.java index d49198ebee8..a1851fe0a48 100644 --- a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/filter/RestExtensionExecutionFilter.java +++ b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/filter/RestExtensionExecutionFilter.java @@ -171,7 +171,7 @@ private RestFilter[] matchFilters(RestFilter[] filters, String path) { if (size > 1) { Collections.sort(matches); } - if (matches.get(size - 1).getValue()) { + if (matches.get(0).getValue()) { continue; } } diff --git a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathCondition.java b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathCondition.java index 5291c504a72..ab1e7147746 100644 --- a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathCondition.java +++ b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathCondition.java @@ -105,7 +105,9 @@ public PathCondition match(HttpRequest request) { } } if (matches != null) { - Collections.sort(matches); + if (matches.size() > 1) { + Collections.sort(matches); + } Set result = CollectionUtils.newLinkedHashSet(matches.size()); for (int i = 0, size = matches.size(); i < size; i++) { result.add(matches.get(i).getPath()); diff --git a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathExpression.java b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathExpression.java index c3fa4360f5b..183b718b423 100644 --- a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathExpression.java +++ b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathExpression.java @@ -104,7 +104,7 @@ public int compareTo(PathExpression other) { return result; } } - return otherSize - size; + return size - otherSize; } @Override diff --git a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathSegment.java b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathSegment.java index 44c50eb852b..e19dbbaf12d 100644 --- a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathSegment.java +++ b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathSegment.java @@ -193,9 +193,9 @@ public int compareTo(PathSegment other) { return comparison; } } - int size = variables == null ? 0 : variables.size(); - int otherSize = other.variables == null ? 0 : other.variables.size(); - return otherSize - size; + int size = variables == null ? 99 : variables.size(); + int otherSize = other.variables == null ? 99 : other.variables.size(); + return size - otherSize; } public enum Type { @@ -212,18 +212,18 @@ public enum Type { LITERAL(1), /** * A wildcard segment. - * E.g.: 't?st*uv' and '/foo/*/bar' + * E.g.: 't?st*uv' */ WILDCARD, /** * A wildcard matching suffix. * Transient type used for parsing, will not be present in the PathExpression - * E.g.: '/foo/**' and '/**' and '/{*bar}' + * E.g.: '/foo/**' or '/**' or '/{*bar}' */ WILDCARD_TAIL, /** * A template variable segment. - * E.g.: '{foo}' + * E.g.: '{foo}' or '/foo/*/bar' */ VARIABLE(10), /** diff --git a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/openapi/DefaultOpenAPIService.java b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/openapi/DefaultOpenAPIService.java index 808dadc90fa..99344f184be 100644 --- a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/openapi/DefaultOpenAPIService.java +++ b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/openapi/DefaultOpenAPIService.java @@ -25,8 +25,7 @@ import org.apache.dubbo.remoting.http12.HttpRequest; import org.apache.dubbo.remoting.http12.HttpResponse; import org.apache.dubbo.remoting.http12.HttpResult; -import org.apache.dubbo.remoting.http12.HttpStatus; -import org.apache.dubbo.remoting.http12.exception.HttpStatusException; +import org.apache.dubbo.remoting.http12.exception.HttpResultPayloadException; import org.apache.dubbo.remoting.http12.message.MediaType; import org.apache.dubbo.remoting.http12.rest.OpenAPIRequest; import org.apache.dubbo.rpc.RpcContext; @@ -176,7 +175,6 @@ private List resolveOpenAPIs() { @Override public String getDocument(OpenAPIRequest request) { String path = null; - HttpResult result; try { request = Helper.formatRequest(request); @@ -187,13 +185,14 @@ public String getDocument(OpenAPIRequest request) { path = RequestUtils.getPathVariable(httpRequest, "path"); if (StringUtils.isEmpty(path)) { - throw HttpResult.found(PathUtils.join(httpRequest.path(), "swagger-ui/index.html")).toPayload(); + String url = PathUtils.join(httpRequest.path(), "swagger-ui/index.html"); + throw HttpResult.found(url).toPayload(); } path = '/' + path; List> matches = tree.matchRelaxed(path); if (matches.isEmpty()) { - throw new HttpStatusException(HttpStatus.NOT_FOUND.getCode()); + throw HttpResult.notFound().toPayload(); } Collections.sort(matches); @@ -204,13 +203,14 @@ public String getDocument(OpenAPIRequest request) { } httpRequest.setAttribute(OpenAPIRequest.class.getName(), request); httpRequest.setAttribute(RestConstants.URI_TEMPLATE_VARIABLES_ATTRIBUTE, match.getVariableMap()); - result = match.getValue().handle(path, httpRequest, httpResponse); + throw match.getValue().handle(path, httpRequest, httpResponse).toPayload(); + } catch (HttpResultPayloadException e) { + throw e; } catch (Throwable t) { - Level level = ExceptionUtils.resolveLogLevel(t); + Level level = ExceptionUtils.resolveLogLevel(ExceptionUtils.unwrap(t)); LOG.log(level, "Failed to processing OpenAPI request {} for path: '{}'", request, path, t); throw t; } - throw result.toPayload(); } private String handleDocument(OpenAPIRequest request) { diff --git a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/openapi/SchemaResolver.java b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/openapi/SchemaResolver.java index 724c976a582..d67febed9fe 100644 --- a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/openapi/SchemaResolver.java +++ b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/openapi/SchemaResolver.java @@ -250,7 +250,7 @@ private boolean isClassExcluded(Class clazz) { } else if (size > 1) { Collections.sort(matches); } - return matches.get(size - 1).getValue(); + return matches.get(0).getValue(); } public static void addPath(RadixTree tree, String path) { diff --git a/dubbo-rpc/dubbo-rpc-triple/src/test/groovy/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathExpressionTest.groovy b/dubbo-rpc/dubbo-rpc-triple/src/test/groovy/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathExpressionTest.groovy index 1061fcea658..19ad12542fc 100644 --- a/dubbo-rpc/dubbo-rpc-triple/src/test/groovy/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathExpressionTest.groovy +++ b/dubbo-rpc/dubbo-rpc-triple/src/test/groovy/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathExpressionTest.groovy @@ -206,6 +206,7 @@ class PathExpressionTest extends Specification { '/one' | '/one' | 0 '/one' | '/two' | 0 '/one/{two}/three' | '/one/{t}/three' | 0 + '/one/two' | '/one/{two}' | -1 '/one/{two}/three' | '/one/*/three' | -1 '/one/*/three' | '/one/**/three' | -1 '/one/two' | '/one' | -1