Skip to content

Commit

Permalink
Improve trailing slash handling (#1408)
Browse files Browse the repository at this point in the history
  • Loading branch information
TeslaCN committed Aug 25, 2020
1 parent d23a9f6 commit f306017
Show file tree
Hide file tree
Showing 18 changed files with 481 additions and 25 deletions.
13 changes: 12 additions & 1 deletion elasticjob-infra/elasticjob-restful/pom.xml
Expand Up @@ -55,5 +55,16 @@
<scope>test</scope>
</dependency>
</dependencies>

<build>
<resources>
<resource>
<directory>src/main/resources</directory>
</resource>
</resources>
<testResources>
<testResource>
<directory>src/test/resources</directory>
</testResource>
</testResources>
</build>
</project>
Expand Up @@ -41,6 +41,12 @@ public final class NettyRestfulServiceConfiguration {
@Setter
private String host;

/**
* If trailing slash sensitive, <code>/foo/bar</code> is not equals to <code>/foo/bar/</code>.
*/
@Setter
private boolean trailingSlashSensitive;

private final List<RestfulController> controllerInstances = new ArrayList<>();

private final Map<Class<? extends Throwable>, ExceptionHandler<? extends Throwable>> exceptionHandlers = new HashMap<>();
Expand Down
Expand Up @@ -37,7 +37,7 @@
String method();

/**
* Path pattern of this handler.
* Path pattern of this handler. Starts with '/'.
* Such as <code>/app/{jobName}/enable</code>.
*
* @return Path pattern
Expand Down
Expand Up @@ -33,7 +33,7 @@ public final class RegexPathMatcher implements PathMatcher {

private static final String PATH_SEPARATOR = "/";

private static final Pattern PATH_PATTERN = Pattern.compile("^(?:/|(/[^/{}?]+|/\\{[^/{}?]+})+)$");
private static final Pattern PATH_PATTERN = Pattern.compile("^/(([^/{}]+|\\{[^/{}]+})(/([^/{}]+|\\{[^/{}]+}))*/?)?$");

private static final Pattern TEMPLATE_PATTERN = Pattern.compile("\\{(?<template>[^/]+)}");

Expand Down
Expand Up @@ -22,7 +22,6 @@
import io.netty.channel.ChannelHandler.Sharable;
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelInboundHandlerAdapter;
import io.netty.handler.codec.UnsupportedMessageTypeException;
import io.netty.handler.codec.http.FullHttpRequest;
import io.netty.handler.codec.http.HttpUtil;
import io.netty.handler.codec.http.QueryStringDecoder;
Expand Down Expand Up @@ -102,9 +101,6 @@ private Object[] prepareArguments(final FullHttpRequest httpRequest, final Mappi
String mimeType = Optional.ofNullable(HttpUtil.getMimeType(httpRequest))
.orElseGet(() -> HttpUtil.getMimeType(Http.DEFAULT_CONTENT_TYPE)).toString();
RequestBodyDeserializer deserializer = RequestBodyDeserializerFactory.getRequestBodyDeserializer(mimeType);
if (null == deserializer) {
throw new UnsupportedMessageTypeException(MessageFormat.format("Unsupported MIME type [{0}]", mimeType));
}
Object parsedBodyValue = deserializer.deserialize(targetType, bytes);
parsedValue = parsedBodyValue;
Preconditions.checkArgument(nullable || null != parsedBodyValue, "Missing request body");
Expand Down
Expand Up @@ -44,16 +44,24 @@
@Slf4j
public final class HttpRequestDispatcher extends ChannelInboundHandlerAdapter {

private static final String TRAILING_SLASH = "/";

private final HandlerMappingRegistry mappingRegistry = new HandlerMappingRegistry();

public HttpRequestDispatcher(final List<RestfulController> restfulControllers) {
private final boolean trailingSlashSensitive;

public HttpRequestDispatcher(final List<RestfulController> restfulControllers, final boolean trailingSlashSensitive) {
this.trailingSlashSensitive = trailingSlashSensitive;
initMappingRegistry(restfulControllers);
}

@Override
public void channelRead(final ChannelHandlerContext ctx, final Object msg) throws Exception {
log.debug("{}", msg);
FullHttpRequest request = (FullHttpRequest) msg;
if (!trailingSlashSensitive) {
request.setUri(appendTrailingSlashIfAbsent(request.uri()));
}
MappingContext<Handler> mappingContext = mappingRegistry.getMappingContext(request);
if (null == mappingContext) {
throw new HandlerNotFoundException(request.uri());
Expand All @@ -72,10 +80,26 @@ private void initMappingRegistry(final List<RestfulController> restfulController
continue;
}
HttpMethod httpMethod = HttpMethod.valueOf(mapping.method());
String pattern = mapping.path();
String fullPathPattern = contextPath + pattern;
String path = mapping.path();
String fullPathPattern = resolveFullPath(contextPath, path);
if (!trailingSlashSensitive) {
fullPathPattern = appendTrailingSlashIfAbsent(fullPathPattern);
}
mappingRegistry.addMapping(httpMethod, fullPathPattern, new Handler(restfulController, method));
}
}
}

private String resolveFullPath(final String contextPath, final String pattern) {
return Optional.ofNullable(contextPath).orElse("") + pattern;
}

private String appendTrailingSlashIfAbsent(final String uri) {
String[] split = uri.split("\\?");
if (1 == split.length) {
return uri.endsWith(TRAILING_SLASH) ? uri : uri + TRAILING_SLASH;
}
String path = split[0];
return path.endsWith(TRAILING_SLASH) ? uri : path + TRAILING_SLASH + "?" + split[1];
}
}
Expand Up @@ -38,7 +38,7 @@ public final class RestfulServiceChannelInitializer extends ChannelInitializer<C
private final ExceptionHandling exceptionHandling;

public RestfulServiceChannelInitializer(final NettyRestfulServiceConfiguration configuration) {
httpRequestDispatcher = new HttpRequestDispatcher(configuration.getControllerInstances());
httpRequestDispatcher = new HttpRequestDispatcher(configuration.getControllerInstances(), configuration.isTrailingSlashSensitive());
handlerParameterDecoder = new HandlerParameterDecoder();
handleMethodExecutor = new HandleMethodExecutor();
exceptionHandling = new ExceptionHandling(configuration.getExceptionHandlers());
Expand Down
Expand Up @@ -59,12 +59,17 @@ public void assertPathMatch() {
public void assertValidatePathPattern() {
PathMatcher pathMatcher = new RegexPathMatcher();
assertTrue(pathMatcher.isValidPathPattern("/"));
assertTrue(pathMatcher.isValidPathPattern("/app"));
assertTrue(pathMatcher.isValidPathPattern("/app/job"));
assertTrue(pathMatcher.isValidPathPattern("/app/job/"));
assertTrue(pathMatcher.isValidPathPattern("/app/{jobName}"));
assertTrue(pathMatcher.isValidPathPattern("/{appName}/{jobName}/status"));
assertFalse(pathMatcher.isValidPathPattern("/app/jobName}"));
assertFalse(pathMatcher.isValidPathPattern("/app/{jobName"));
assertFalse(pathMatcher.isValidPathPattern("/app/{job}Name"));
assertFalse(pathMatcher.isValidPathPattern("/app//jobName"));
assertFalse(pathMatcher.isValidPathPattern("//app/jobName"));
assertFalse(pathMatcher.isValidPathPattern("app/jobName"));
assertFalse(pathMatcher.isValidPathPattern(""));
}
}
@@ -0,0 +1,37 @@
/*
* 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.shardingsphere.elasticjob.restful.controller;

import lombok.extern.slf4j.Slf4j;
import org.apache.shardingsphere.elasticjob.restful.Http;
import org.apache.shardingsphere.elasticjob.restful.RestfulController;
import org.apache.shardingsphere.elasticjob.restful.annotation.Mapping;

@Slf4j
public class IndexController implements RestfulController {

/**
* A mapping declare path implicit, meaning it mapped index.
*
* @return a string
*/
@Mapping(method = Http.GET)
public String index() {
return "hello, elastic-job";
}
}
@@ -0,0 +1,50 @@
/*
* 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.shardingsphere.elasticjob.restful.controller;

import org.apache.shardingsphere.elasticjob.restful.Http;
import org.apache.shardingsphere.elasticjob.restful.RestfulController;
import org.apache.shardingsphere.elasticjob.restful.annotation.ContextPath;
import org.apache.shardingsphere.elasticjob.restful.annotation.Mapping;
import org.apache.shardingsphere.elasticjob.restful.annotation.Returning;

@ContextPath("/trailing")
public final class TrailingSlashTestController implements RestfulController {

/**
* A mapping without trailing slash.
*
* @return a string
*/
@Mapping(method = Http.GET, path = "/slash")
@Returning(contentType = "text/plain; charset=utf-8")
public String withoutTrailingSlash() {
return "without trailing slash";
}

/**
* A mapping with trailing slash.
*
* @return a string
*/
@Mapping(method = Http.GET, path = "/slash/")
@Returning(contentType = "text/plain; charset=utf-8")
public String withTrailingSlash() {
return "with trailing slash";
}
}
@@ -0,0 +1,117 @@
/*
* 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.shardingsphere.elasticjob.restful.pipeline;

import io.netty.buffer.ByteBuf;
import io.netty.buffer.ByteBufUtil;
import io.netty.buffer.Unpooled;
import io.netty.channel.embedded.EmbeddedChannel;
import io.netty.handler.codec.http.DefaultFullHttpRequest;
import io.netty.handler.codec.http.DefaultHttpHeaders;
import io.netty.handler.codec.http.FullHttpRequest;
import io.netty.handler.codec.http.FullHttpResponse;
import io.netty.handler.codec.http.HttpHeaders;
import io.netty.handler.codec.http.HttpMethod;
import io.netty.handler.codec.http.HttpVersion;
import io.netty.handler.codec.http.QueryStringEncoder;
import org.apache.shardingsphere.elasticjob.restful.Http;
import org.apache.shardingsphere.elasticjob.restful.RestfulController;
import org.apache.shardingsphere.elasticjob.restful.annotation.Mapping;
import org.apache.shardingsphere.elasticjob.restful.annotation.Param;
import org.apache.shardingsphere.elasticjob.restful.annotation.ParamSource;
import org.apache.shardingsphere.elasticjob.restful.annotation.RequestBody;
import org.junit.Before;
import org.junit.Test;

import java.util.Collections;

import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertThat;

public class HandlerParameterDecoderTest {

private EmbeddedChannel channel;

@Before
public void setUp() {
HttpRequestDispatcher httpRequestDispatcher = new HttpRequestDispatcher(Collections.singletonList(new DecoderTestController()), false);
HandlerParameterDecoder handlerParameterDecoder = new HandlerParameterDecoder();
HandleMethodExecutor handleMethodExecutor = new HandleMethodExecutor();
channel = new EmbeddedChannel(httpRequestDispatcher, handlerParameterDecoder, handleMethodExecutor);
}

@Test
public void assertDecodeParameters() {
QueryStringEncoder queryStringEncoder = new QueryStringEncoder("/myApp/C");
queryStringEncoder.addParam("cron", "0 * * * * ?");
queryStringEncoder.addParam("integer", "30");
queryStringEncoder.addParam("bool", "true");
queryStringEncoder.addParam("long", "3000");
queryStringEncoder.addParam("double", "23.33");
String uri = queryStringEncoder.toString();
ByteBuf body = Unpooled.wrappedBuffer("BODY".getBytes());
HttpHeaders headers = new DefaultHttpHeaders();
headers.set("Message", "some_message");
FullHttpRequest httpRequest = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, uri, body, headers, headers);
channel.writeInbound(httpRequest);
FullHttpResponse httpResponse = channel.readOutbound();
assertThat(httpResponse.status().code(), is(200));
assertThat(new String(ByteBufUtil.getBytes(httpResponse.content())), is("ok"));
}

public static class DecoderTestController implements RestfulController {

/**
* A handle method for decode testing.
*
* @param appName string from path
* @param ch character from path
* @param cron cron from query
* @param message message from header
* @param body from request body
* @param integer integer from query
* @param bool boolean from query
* @param longValue long from query
* @param doubleValue double from query
* @return OK
*/
@Mapping(method = Http.GET, path = "/{appName}/{ch}")
public String handle(
final @Param(source = ParamSource.PATH, name = "appName") String appName,
final @Param(source = ParamSource.PATH, name = "ch") char ch,
final @Param(source = ParamSource.QUERY, name = "cron") String cron,
final @Param(source = ParamSource.HEADER, name = "Message") String message,
final @RequestBody String body,
final @Param(source = ParamSource.QUERY, name = "integer") int integer,
final @Param(source = ParamSource.QUERY, name = "bool") Boolean bool,
final @Param(source = ParamSource.QUERY, name = "long") Long longValue,
final @Param(source = ParamSource.QUERY, name = "double") double doubleValue
) {
assertThat(appName, is("myApp"));
assertThat(ch, is('C'));
assertThat(cron, is("0 * * * * ?"));
assertThat(message, is("some_message"));
assertThat(body, is("BODY"));
assertThat(integer, is(30));
assertThat(bool, is(true));
assertThat(longValue, is(3000L));
assertThat(doubleValue, is(23.33));
return "ok";
}
}
}
Expand Up @@ -48,6 +48,7 @@ public class HttpClient {
* @param request HTTP request
* @param consumer HTTP response consumer
* @param timeoutSeconds Wait for consume
* @throws InterruptedException interrupted
*/
@SneakyThrows
public static void request(final String host, final int port, final FullHttpRequest request, final Consumer<FullHttpResponse> consumer, final Long timeoutSeconds) {
Expand All @@ -66,8 +67,11 @@ protected void initChannel(final Channel ch) throws Exception {
.addLast(new SimpleChannelInboundHandler<FullHttpResponse>() {
@Override
protected void channelRead0(final ChannelHandlerContext ctx, final FullHttpResponse httpResponse) throws Exception {
consumer.accept(httpResponse);
countDownLatch.countDown();
try {
consumer.accept(httpResponse);
} finally {
countDownLatch.countDown();
}
}
});
}
Expand Down
Expand Up @@ -31,7 +31,7 @@ public class HttpRequestDispatcherTest {

@Test(expected = HandlerNotFoundException.class)
public void assertDispatcherHandlerNotFound() {
EmbeddedChannel channel = new EmbeddedChannel(new HttpRequestDispatcher(Lists.newArrayList(new JobController())));
EmbeddedChannel channel = new EmbeddedChannel(new HttpRequestDispatcher(Lists.newArrayList(new JobController()), false));
FullHttpRequest fullHttpRequest = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/hello/myJob/myCron");
channel.writeInbound(fullHttpRequest);
}
Expand Down

0 comments on commit f306017

Please sign in to comment.