fix 405 && differnt httpmethod path doubleCheck (#12152)

This commit is contained in:
suncairong163 2023-04-21 10:39:00 +08:00 committed by GitHub
parent 3adfbecf25
commit f7c90572ec
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 173 additions and 11 deletions

View File

@ -31,6 +31,9 @@ public class PathMatcher {
private String[] pathSplits;
private boolean hasPathVariable;
private String contextPath;
private String httpMethod;
// for provider http method compare
private boolean needCompareMethod = true;
public PathMatcher(String path) {
@ -45,6 +48,10 @@ public class PathMatcher {
this.port = (port == null || port == -1 || port == 0) ? null : port;
}
public PathMatcher(String path, String version, String group, Integer port, String httpMethod) {
this(path, version, group, port);
setHttpMethod(httpMethod);
}
private void dealPathVariable(String path) {
this.pathSplits = path.split(SEPARATOR);
@ -88,8 +95,8 @@ public class PathMatcher {
}
public static PathMatcher getInvokeCreatePathMatcher(String path, String version, String group, Integer port) {
return new PathMatcher(path, version, group, port);
public static PathMatcher getInvokeCreatePathMatcher(String path, String version, String group, Integer port, String method) {
return new PathMatcher(path, version, group, port, method).noNeedHttpMethodCompare();
}
public boolean hasPathVariable() {
@ -100,6 +107,20 @@ public class PathMatcher {
return port;
}
public String getHttpMethod() {
return httpMethod;
}
public PathMatcher setHttpMethod(String httpMethod) {
this.httpMethod = httpMethod;
return this;
}
private PathMatcher noNeedHttpMethodCompare() {
this.needCompareMethod = false;
return this;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
@ -107,6 +128,7 @@ public class PathMatcher {
PathMatcher that = (PathMatcher) o;
return pathEqual(that)
&& Objects.equals(version, that.version)
&& (this.needCompareMethod ? Objects.equals(httpMethod, that.httpMethod) : true)
&& Objects.equals(group, that.group) && Objects.equals(port, that.port);
}
@ -200,6 +222,7 @@ public class PathMatcher {
", port=" + port +
", hasPathVariable=" + hasPathVariable +
", contextPath='" + contextPath + '\'' +
", httpMethod='" + httpMethod + '\'' +
'}';
}
}

View File

@ -206,6 +206,10 @@ public class RequestMetadata implements Serializable {
setPath(contextPathFromUrl + path);
}
public boolean methodAllowed(String method) {
return method != null && method.equals(this.method);
}
@Override
public boolean equals(Object o) {
if (this == o) {

View File

@ -110,7 +110,7 @@ public class ServiceRestMetadata implements Serializable {
public void addRestMethodMetadata(RestMethodMetadata restMethodMetadata) {
PathMatcher pathMather = new PathMatcher(restMethodMetadata.getRequest().getPath(),
this.getVersion(), this.getGroup(), this.getPort());
this.getVersion(), this.getGroup(), this.getPort(),restMethodMetadata.getRequest().getMethod());
addPathToServiceMap(pathMather, restMethodMetadata);
addMethodToServiceMap(restMethodMetadata);
getMeta().add(restMethodMetadata);

View File

@ -0,0 +1,48 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF 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
*
* http://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 org.apache.dubbo.metadata.rest.api;
import javax.ws.rs.Consumes;
import javax.ws.rs.DELETE;
import javax.ws.rs.GET;
import javax.ws.rs.POST;
import javax.ws.rs.Path;
import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
@Path("usingService")
@Consumes({MediaType.APPLICATION_JSON})
@Produces({MediaType.APPLICATION_JSON})
public interface JaxrsUsingService {
@GET
Response getUsers();
@POST
Response createUser(Object user);
@GET
@Path("{uid}")
Response getUserByUid(@PathParam("uid") String uid);
@DELETE
@Path("{uid}")
Response deleteUserByUid(@PathParam("uid") String uid);
}

View File

@ -16,13 +16,18 @@
*/
package org.apache.dubbo.metadata.rest.jaxrs;
import org.apache.dubbo.metadata.rest.PathMatcher;
import org.apache.dubbo.metadata.rest.RestMethodMetadata;
import org.apache.dubbo.metadata.rest.ServiceRestMetadata;
import org.apache.dubbo.metadata.rest.api.JaxrsRestDoubleCheckContainsPathVariableService;
import org.apache.dubbo.metadata.rest.api.JaxrsRestDoubleCheckService;
import org.apache.dubbo.metadata.rest.api.JaxrsUsingService;
import org.apache.dubbo.rpc.model.ApplicationModel;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import java.util.Map;
public class JaxrsRestDoubleCheckTest {
private JAXRSServiceRestMetadataResolver instance = new JAXRSServiceRestMetadataResolver(ApplicationModel.defaultModel());
@ -45,4 +50,27 @@ public class JaxrsRestDoubleCheckTest {
}
@Test
void testSameHttpMethodException() {
Assertions.assertDoesNotThrow(() -> {
ServiceRestMetadata resolve = new ServiceRestMetadata();
resolve.setServiceInterface(JaxrsUsingService.class.getName());
instance.resolve(JaxrsUsingService.class, resolve);
});
ServiceRestMetadata resolve = new ServiceRestMetadata();
resolve.setServiceInterface(JaxrsUsingService.class.getName());
instance.resolve(JaxrsUsingService.class, resolve);
Map<PathMatcher, RestMethodMetadata> pathContainPathVariableToServiceMap = resolve.getPathContainPathVariableToServiceMap();
RestMethodMetadata restMethodMetadata = pathContainPathVariableToServiceMap.get(PathMatcher.getInvokeCreatePathMatcher("/usingService/aaa", null, null, null, "TEST"));
Assertions.assertNotNull(restMethodMetadata);
}
}

View File

@ -65,10 +65,10 @@ public class PathAndInvokerMapper {
* @param port
* @return
*/
public InvokerAndRestMethodMetadataPair getRestMethodMetadata(String path, String version, String group, Integer port) {
public InvokerAndRestMethodMetadataPair getRestMethodMetadata(String path, String version, String group, Integer port,String method) {
PathMatcher pathMather = PathMatcher.getInvokeCreatePathMatcher(path, version, group, port);
PathMatcher pathMather = PathMatcher.getInvokeCreatePathMatcher(path, version, group, port,method);
// first search from pathToServiceMapNoPathVariable
if (pathToServiceMapNoPathVariable.containsKey(pathMather)) {

View File

@ -134,8 +134,9 @@ public class RestRPCInvocationUtil {
String path = request.getPath();
String version = request.getHeader(RestHeaderEnum.VERSION.getHeader());
String group = request.getHeader(RestHeaderEnum.GROUP.getHeader());
String method = request.getMethod();
return pathAndInvokerMapper.getRestMethodMetadata(path, version, group, null);
return pathAndInvokerMapper.getRestMethodMetadata(path, version, group, null, method);
}

View File

@ -104,6 +104,16 @@ public class NettyHttpHandler implements HttpHandler<NettyRequestFacade, NettyHt
RestMethodMetadata restMethodMetadata = restMethodMetadataPair.getRestMethodMetadata();
// method disallowed
if (!restMethodMetadata.getRequest().methodAllowed(request.getMethod())) {
nettyHttpResponse.sendError(405, "service require request method is : "
+ restMethodMetadata.getRequest().getMethod()
+ ", but current request method is: " + request.getMethod()
);
return;
}
// content-type support judge,throw unSupportException
acceptSupportJudge(request, restMethodMetadata.getReflectMethod().getReturnType());
@ -149,7 +159,7 @@ public class NettyHttpHandler implements HttpHandler<NettyRequestFacade, NettyHt
* @throws Exception
*/
private void writeResult(NettyHttpResponse nettyHttpResponse, RequestFacade request, Invoker invoker, Object value, Class returnType) throws Exception {
MediaType mediaType = getAcceptMediaType(request,returnType);
MediaType mediaType = getAcceptMediaType(request, returnType);
MessageCodecResultPair booleanMediaTypePair = HttpMessageCodecManager.httpMessageEncode(nettyHttpResponse.getOutputStream(), value, invoker.getUrl(), mediaType, returnType);
@ -162,7 +172,7 @@ public class NettyHttpHandler implements HttpHandler<NettyRequestFacade, NettyHt
* @param request
* @return
*/
private MediaType getAcceptMediaType(RequestFacade request,Class<?> returnType) {
private MediaType getAcceptMediaType(RequestFacade request, Class<?> returnType) {
String accept = request.getHeader(RestHeaderEnum.ACCEPT.getHeader());
MediaType mediaType = MediaTypeUtil.convertMediaType(returnType, accept);
return mediaType;
@ -176,7 +186,7 @@ public class NettyHttpHandler implements HttpHandler<NettyRequestFacade, NettyHt
private void acceptSupportJudge(RequestFacade requestFacade, Class<?> returnType) {
try {
// media type judge
getAcceptMediaType(requestFacade,returnType);
getAcceptMediaType(requestFacade, returnType);
} catch (UnSupportContentTypeException e) {
// return type judge
MediaType mediaType = HttpMessageCodecManager.typeSupport(returnType);

View File

@ -46,8 +46,9 @@ import org.apache.dubbo.rpc.protocol.rest.rest.AnotherUserRestService;
import org.apache.dubbo.rpc.protocol.rest.rest.AnotherUserRestServiceImpl;
import org.apache.dubbo.rpc.protocol.rest.rest.HttpMethodService;
import org.apache.dubbo.rpc.protocol.rest.rest.HttpMethodServiceImpl;
import org.apache.dubbo.rpc.protocol.rest.rest.RestDemoForTestException;
import org.apache.dubbo.rpc.protocol.rest.rest.RestDemoService;
import org.apache.dubbo.rpc.protocol.rest.rest.RestDemoServiceImpl;
import org.hamcrest.CoreMatchers;
import org.jboss.resteasy.specimpl.MultivaluedMapImpl;
import org.junit.jupiter.api.AfterEach;
@ -60,7 +61,6 @@ import java.util.HashMap;
import java.util.Map;
import static org.apache.dubbo.remoting.Constants.SERVER_KEY;
import static org.apache.dubbo.rpc.protocol.rest.Constants.EXCEPTION_MAPPER_KEY;
import static org.apache.dubbo.rpc.protocol.rest.Constants.EXTENSION_KEY;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.is;
@ -561,6 +561,40 @@ class JaxrsRestProtocolTest {
}
}
@Test
void test405() {
int availablePort = NetUtils.getAvailablePort();
URL url = URL.valueOf("rest://127.0.0.1:" + availablePort
+ "/?version=1.0.0&interface=org.apache.dubbo.rpc.protocol.rest.rest.RestDemoService&"
);
RestDemoServiceImpl server = new RestDemoServiceImpl();
url = this.registerProvider(url, server, RestDemoService.class);
Exporter<RestDemoService> exporter = protocol.export(proxy.getInvoker(server, RestDemoService.class, url));
URL consumer = URL.valueOf("rest://127.0.0.1:" + availablePort
+ "/?version=1.0.0&interface=org.apache.dubbo.rpc.protocol.rest.rest.RestDemoForTestException&"
);
consumer = this.registerProvider(consumer, server, RestDemoForTestException.class);
Invoker<RestDemoForTestException> invoker = protocol.refer(RestDemoForTestException.class, consumer);
RestDemoForTestException client = proxy.getProxy(invoker);
Assertions.assertThrows(RpcException.class, () -> {
client.testMethodDisallowed("aaa");
});
invoker.destroy();
exporter.unexport();
}
private URL registerProvider(URL url, Object impl, Class<?> interfaceClass) {
ServiceDescriptor serviceDescriptor = repository.registerService(interfaceClass);
ProviderModel providerModel = new ProviderModel(

View File

@ -20,6 +20,7 @@ import javax.ws.rs.Consumes;
import javax.ws.rs.GET;
import javax.ws.rs.POST;
import javax.ws.rs.Path;
import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
import javax.ws.rs.QueryParam;
import javax.ws.rs.core.MediaType;
@ -36,4 +37,8 @@ public interface RestDemoForTestException {
@Consumes({MediaType.TEXT_PLAIN})
@Path("/hello")
Integer test400(@QueryParam("a")String a,@QueryParam("b") String b);
@POST
@Path("{uid}")
String testMethodDisallowed(@PathParam("uid") String uid);
}

View File

@ -42,4 +42,8 @@ public interface RestDemoService {
Long testFormBody(@FormParam("number") Long number);
boolean isCalled();
@DELETE
@Path("{uid}")
String deleteUserByUid(@PathParam("uid") String uid);
}

View File

@ -42,6 +42,11 @@ public class RestDemoServiceImpl implements RestDemoService {
return called;
}
@Override
public String deleteUserByUid(String uid) {
return uid;
}
@Override
public Integer hello(Integer a, Integer b) {
context = RpcContext.getServerAttachment().getObjectAttachments();