Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/dubbo 3.2 rest fix http method pathmatcher #12890

Merged
merged 2 commits into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,6 @@ public interface CommonConstants {

String DUBBO_PACKABLE_METHOD_FACTORY = "dubbo.application.parameters." + PACKABLE_METHOD_FACTORY_KEY;

String SERVICE_DEPLOYER_ATTRIBUTE_KEY = "serviceDeployer";
String RESTEASY_NETTY_HTTP_REQUEST_ATTRIBUTE_KEY = "resteasyNettyHttpRequest";

String DUBBO_MANUAL_REGISTER_KEY = "dubbo.application.manual-register";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,17 @@ public void setContextPath(String contextPath) {
}

public static PathMatcher getInvokeCreatePathMatcher(String path, String version, String group, Integer port, String method) {
return new PathMatcher(path, version, group, port, method).noNeedHttpMethodCompare();
return new PathMatcher(path, version, group, port, method).compareHttpMethod(false);
}

public static PathMatcher getInvokeCreatePathMatcher(Method serviceMethod) {
return new PathMatcher(serviceMethod).setNeedCompareServiceMethod(true);
}

public static PathMatcher convertPathMatcher(PathMatcher pathMatcher) {
return getInvokeCreatePathMatcher(pathMatcher.path, pathMatcher.version, pathMatcher.group, pathMatcher.port, pathMatcher.httpMethod);
}

public boolean hasPathVariable() {
return hasPathVariable;
}
Expand All @@ -134,8 +138,8 @@ public PathMatcher setHttpMethod(String httpMethod) {
return this;
}

private PathMatcher noNeedHttpMethodCompare() {
this.needCompareHttpMethod = false;
public PathMatcher compareHttpMethod(boolean needCompareHttpMethod) {
this.needCompareHttpMethod = needCompareHttpMethod;
return this;
}

Expand Down Expand Up @@ -176,7 +180,7 @@ && httpMethodMatch(that) // http method compare
* @return
*/
private boolean httpMethodMatch(PathMatcher that) {
return !that.needCompareHttpMethod || !this.needCompareHttpMethod ? true: Objects.equals(this.httpMethod, that.httpMethod);
return !that.needCompareHttpMethod || !this.needCompareHttpMethod ? true : Objects.equals(this.httpMethod, that.httpMethod);
}

private boolean serviceMethodEqual(PathMatcher thatPathMatcher, PathMatcher thisPathMatcher) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,4 @@ public interface Constants {

String NETTY_HTTP = "netty_http";

// exception mapper
String EXCEPTION_MAPPER_KEY = "exception.mapper";




}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@
import org.apache.dubbo.rpc.protocol.rest.exception.DoublePathCheckException;
import org.apache.dubbo.rpc.protocol.rest.pair.InvokerAndRestMethodMetadataPair;

import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;

/**
Expand All @@ -36,6 +39,8 @@ public class PathAndInvokerMapper {
private final Map<PathMatcher, InvokerAndRestMethodMetadataPair> pathToServiceMapContainPathVariable = new ConcurrentHashMap<>();
private final Map<PathMatcher, InvokerAndRestMethodMetadataPair> pathToServiceMapNoPathVariable = new ConcurrentHashMap<>();

// for http method compare 405
private final Map<PathMatcher, Set<String>> pathMatcherToHttpMethodMap = new HashMap<>();

/**
* deploy path metadata
Expand Down Expand Up @@ -105,7 +110,7 @@ public void addPathMatcherToPathMap(PathMatcher pathMatcher,

InvokerAndRestMethodMetadataPair beforeMetadata = pathMatcherPairMap.get(pathMatcher);
// true when reExport
if (!invokerRestMethodMetadataPair.compareServiceMethod(beforeMetadata)){
if (!invokerRestMethodMetadataPair.compareServiceMethod(beforeMetadata)) {
throw new DoublePathCheckException(
"dubbo rest double path check error, current path is: " + pathMatcher
+ " ,and service method is: " + invokerRestMethodMetadataPair.getRestMethodMetadata().getReflectMethod()
Expand All @@ -116,8 +121,57 @@ public void addPathMatcherToPathMap(PathMatcher pathMatcher,

pathMatcherPairMap.put(pathMatcher, invokerRestMethodMetadataPair);

addPathMatcherToHttpMethodsMap(pathMatcher);


logger.info("dubbo rest deploy pathMatcher:" + pathMatcher + ", and service method is :" + invokerRestMethodMetadataPair.getRestMethodMetadata().getReflectMethod());
}

private void addPathMatcherToHttpMethodsMap(PathMatcher pathMatcher) {

PathMatcher newPathMatcher = PathMatcher.convertPathMatcher(pathMatcher);

if (!pathMatcherToHttpMethodMap.containsKey(newPathMatcher)) {
HashSet<String> httpMethods = new HashSet<>();

httpMethods.add(pathMatcher.getHttpMethod());

pathMatcherToHttpMethodMap.put(newPathMatcher, httpMethods);

}

Set<String> httpMethods = pathMatcherToHttpMethodMap.get(newPathMatcher);

httpMethods.add(newPathMatcher.getHttpMethod());

}

public boolean isHttpMethodAllowed(PathMatcher pathMatcher) {

PathMatcher newPathMatcher = PathMatcher.convertPathMatcher(pathMatcher);
if (!pathMatcherToHttpMethodMap.containsKey(newPathMatcher)) {
return false;
}


Set<String> httpMethods = pathMatcherToHttpMethodMap.get(newPathMatcher);

return httpMethods.contains(newPathMatcher.getHttpMethod());

}

public String pathHttpMethods(PathMatcher pathMatcher) {

PathMatcher newPathMatcher = PathMatcher.convertPathMatcher(pathMatcher);
if (!pathMatcherToHttpMethodMap.containsKey(newPathMatcher)) {
return null;
}


Set<String> httpMethods = pathMatcherToHttpMethodMap.get(newPathMatcher);

return httpMethods.toString();

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import org.apache.dubbo.metadata.rest.PathMatcher;
import org.apache.dubbo.metadata.rest.RestMethodMetadata;
import org.apache.dubbo.rpc.Invoker;
import org.apache.dubbo.rpc.RpcContext;
import org.apache.dubbo.rpc.RpcInvocation;
import org.apache.dubbo.rpc.protocol.rest.annotation.ParamParserManager;
import org.apache.dubbo.rpc.protocol.rest.annotation.param.parse.provider.ProviderParseContext;
Expand All @@ -38,9 +37,6 @@
import java.util.Arrays;
import java.util.List;

import static org.apache.dubbo.common.constants.CommonConstants.SERVICE_DEPLOYER_ATTRIBUTE_KEY;


public class RestRPCInvocationUtil {

private static final ErrorTypeAwareLogger logger = LoggerFactory.getErrorTypeAwareLogger(RestRPCInvocationUtil.class);
Expand Down Expand Up @@ -136,13 +132,8 @@ public static RpcInvocation createBaseRpcInvocation(RequestFacade request, RestM
* @param pathMatcher
* @return
*/
public static InvokerAndRestMethodMetadataPair getRestMethodMetadataAndInvokerPair(PathMatcher pathMatcher) {

ServiceDeployer serviceDeployer = (ServiceDeployer) RpcContext.getServiceContext().getObjectAttachment(SERVICE_DEPLOYER_ATTRIBUTE_KEY);
public static InvokerAndRestMethodMetadataPair getRestMethodMetadataAndInvokerPair(PathMatcher pathMatcher, ServiceDeployer serviceDeployer) {

if (serviceDeployer == null) {
return null;
}
return serviceDeployer.getPathAndInvokerMapper().getRestMethodMetadata(pathMatcher);
}

Expand All @@ -158,7 +149,7 @@ public static InvokerAndRestMethodMetadataPair getRestMethodMetadataAndInvokerPa

PathMatcher pathMather = createPathMatcher(request);

return getRestMethodMetadataAndInvokerPair(pathMather);
return getRestMethodMetadataAndInvokerPair(pathMather, request.getServiceDeployer());
}


Expand All @@ -173,28 +164,28 @@ public static Invoker getInvokerByRequest(RequestFacade request) {

PathMatcher pathMatcher = createPathMatcher(request);

return getInvoker(pathMatcher);
return getInvoker(pathMatcher, request.getServiceDeployer());
}


/**
* get invoker by service method
*
* <p>
* compare method`s name,param types
*
* @param serviceMethod
* @return
*/

public static Invoker getInvokerByServiceInvokeMethod(Method serviceMethod) {
public static Invoker getInvokerByServiceInvokeMethod(Method serviceMethod, ServiceDeployer serviceDeployer) {

if (serviceMethod == null) {
return null;
}

PathMatcher pathMatcher = PathMatcher.getInvokeCreatePathMatcher(serviceMethod);

InvokerAndRestMethodMetadataPair pair = getRestMethodMetadataAndInvokerPair(pathMatcher);
InvokerAndRestMethodMetadataPair pair = getRestMethodMetadataAndInvokerPair(pathMatcher, serviceDeployer);

if (pair == null) {
return null;
Expand All @@ -209,8 +200,8 @@ public static Invoker getInvokerByServiceInvokeMethod(Method serviceMethod) {
* @param pathMatcher
* @return
*/
public static Invoker getInvoker(PathMatcher pathMatcher) {
InvokerAndRestMethodMetadataPair pair = getRestMethodMetadataAndInvokerPair(pathMatcher);
public static Invoker getInvoker(PathMatcher pathMatcher, ServiceDeployer serviceDeployer) {
InvokerAndRestMethodMetadataPair pair = getRestMethodMetadataAndInvokerPair(pathMatcher, serviceDeployer);

if (pair == null) {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,4 +144,15 @@ private ExceptionMapper createExceptionMapper() {
}


public boolean isMethodAllowed(PathMatcher pathMatcher) {
return pathAndInvokerMapper.isHttpMethodAllowed(pathMatcher);
}

public boolean hashRestMethod(PathMatcher pathMatcher) {
return pathAndInvokerMapper.getRestMethodMetadata(pathMatcher) != null;
}

public String pathHttpMethods(PathMatcher pathMatcher) {
return pathAndInvokerMapper.pathHttpMethods(pathMatcher);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.apache.dubbo.common.extension.Activate;
import org.apache.dubbo.common.logger.ErrorTypeAwareLogger;
import org.apache.dubbo.common.logger.LoggerFactory;
import org.apache.dubbo.metadata.rest.PathMatcher;
import org.apache.dubbo.metadata.rest.RestMethodMetadata;
import org.apache.dubbo.metadata.rest.media.MediaType;
import org.apache.dubbo.rpc.Invoker;
Expand Down Expand Up @@ -72,26 +73,28 @@ private void doHandler(HttpRequest nettyHttpRequest,
RequestFacade request,
URL url,
ServiceDeployer serviceDeployer) throws Exception {
// acquire metadata by request
InvokerAndRestMethodMetadataPair restMethodMetadataPair = RestRPCInvocationUtil.getRestMethodMetadataAndInvokerPair(request);
PathMatcher pathMatcher = RestRPCInvocationUtil.createPathMatcher(request);

// path NoFound 404
if (restMethodMetadataPair == null) {
throw new PathNoFoundException("rest service Path no found, current path info:" + RestRPCInvocationUtil.createPathMatcher(request));
if (!serviceDeployer.hashRestMethod(pathMatcher)) {
throw new PathNoFoundException("rest service Path no found, current path info:" + pathMatcher);
}

Invoker invoker = restMethodMetadataPair.getInvoker();

RestMethodMetadata restMethodMetadata = restMethodMetadataPair.getRestMethodMetadata();

// method disallowed
if (!restMethodMetadata.getRequest().methodAllowed(request.getMethod())) {
if (!serviceDeployer.isMethodAllowed(pathMatcher)) {
nettyHttpResponse.sendError(405, "service require request method is : "
+ restMethodMetadata.getRequest().getMethod()
+ serviceDeployer.pathHttpMethods(pathMatcher)
+ ", but current request method is: " + request.getMethod()
);
return;
}
// compare http method and acquire metadata by request
InvokerAndRestMethodMetadataPair restMethodMetadataPair = RestRPCInvocationUtil.getRestMethodMetadataAndInvokerPair(pathMatcher.compareHttpMethod(true), serviceDeployer);

Invoker invoker = restMethodMetadataPair.getInvoker();

RestMethodMetadata restMethodMetadata = restMethodMetadataPair.getRestMethodMetadata();


// content-type support judge,throw unSupportException
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@
import java.util.ArrayList;
import java.util.List;

import static org.apache.dubbo.common.constants.CommonConstants.SERVICE_DEPLOYER_ATTRIBUTE_KEY;


/**
* netty http request handler
Expand Down Expand Up @@ -77,8 +75,6 @@ public void handle(NettyRequestFacade requestFacade, NettyHttpResponse nettyHttp

Object nettyHttpRequest = requestFacade.getRequest();

RpcContext.getServiceContext().setObjectAttachment(SERVICE_DEPLOYER_ATTRIBUTE_KEY, serviceDeployer);


try {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ protected void decode(ChannelHandlerContext ctx, io.netty.handler.codec.http.Ful
boolean keepAlive = HttpHeaders.isKeepAlive(request);

NettyHttpResponse nettyHttpResponse = new NettyHttpResponse(ctx, keepAlive);
NettyRequestFacade requestFacade = new NettyRequestFacade(request, ctx);
NettyRequestFacade requestFacade = new NettyRequestFacade(request, ctx,serviceDeployer);

executor.execute(() -> {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import io.netty.handler.codec.http.FullHttpRequest;
import io.netty.handler.codec.http.HttpContent;
import org.apache.dubbo.common.utils.IOUtils;
import org.apache.dubbo.rpc.protocol.rest.deploy.ServiceDeployer;

import java.io.IOException;
import java.util.ArrayList;
Expand All @@ -48,6 +49,12 @@ public NettyRequestFacade(Object request, ChannelHandlerContext context) {

}

public NettyRequestFacade(Object request, ChannelHandlerContext context, ServiceDeployer serviceDeployer) {
super((FullHttpRequest) request, serviceDeployer);
this.context = context;

}


protected void initHeaders() {
for (Map.Entry<String, String> header : request.headers()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@


import org.apache.dubbo.common.utils.StringUtils;
import org.apache.dubbo.rpc.protocol.rest.deploy.ServiceDeployer;

import java.io.IOException;
import java.util.ArrayList;
Expand All @@ -37,6 +38,7 @@ public abstract class RequestFacade<T> {
protected String path;
protected T request;
protected byte[] body = new byte[0];
protected ServiceDeployer serviceDeployer;

public RequestFacade(T request) {
this.request = request;
Expand All @@ -45,6 +47,11 @@ public RequestFacade(T request) {
parseBody();
}

public RequestFacade(T request, ServiceDeployer serviceDeployer) {
this(request);
this.serviceDeployer = serviceDeployer;
}

protected void initHeaders() {

}
Expand Down Expand Up @@ -131,5 +138,7 @@ public T getRequest() {

protected abstract void parseBody();


public ServiceDeployer getServiceDeployer() {
return serviceDeployer;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public class TestGetInvokerServiceImpl implements TestGetInvokerService {
@Override
public String getInvoker() {
Object request = RpcContext.getServiceContext().getRequest();
RequestFacade requestFacade = (RequestFacade) request;
Invoker invokerByRequest = RestRPCInvocationUtil.getInvokerByRequest((RequestFacade) request);


Expand All @@ -44,9 +45,9 @@ public String getInvoker() {

}

Invoker invokerByServiceInvokeMethod = RestRPCInvocationUtil.getInvokerByServiceInvokeMethod(hello);
Invoker invokerByServiceInvokeMethod = RestRPCInvocationUtil.getInvokerByServiceInvokeMethod(hello,requestFacade.getServiceDeployer());

Invoker invoker = RestRPCInvocationUtil.getInvokerByServiceInvokeMethod(hashcode);
Invoker invoker = RestRPCInvocationUtil.getInvokerByServiceInvokeMethod(hashcode,requestFacade.getServiceDeployer());


Assertions.assertEquals(invokerByRequest, invokerByServiceInvokeMethod);
Expand Down