From e810ab849e7cdf883380e7b4eac855ddb3d94d63 Mon Sep 17 00:00:00 2001 From: cdfive <31885791+cdfive@users.noreply.github.com> Date: Wed, 12 Jun 2019 09:36:25 +0800 Subject: [PATCH] Fix multiple slash command name issue in sentinel-transport-netty-http module (#817) --- pom.xml | 5 + .../command/netty/HttpServerHandler.java | 12 +- .../MultipleSlashNameCommandTestHandler.java | 32 +++++ .../command/netty/HttpServerHandlerTest.java | 118 ++++++++++++------ .../netty/HttpServerInitializerTest.java | 11 +- .../command/netty/HttpServerTest.java | 29 +++-- ...libaba.csp.sentinel.command.CommandHandler | 1 + .../heartbeat/SimpleHttpHeartbeatSender.java | 2 +- 8 files changed, 143 insertions(+), 67 deletions(-) create mode 100644 sentinel-transport/sentinel-transport-netty-http/src/test/java/com/alibaba/csp/sentinel/transport/command/handler/MultipleSlashNameCommandTestHandler.java create mode 100755 sentinel-transport/sentinel-transport-netty-http/src/test/resources/META-INF/services/com.alibaba.csp.sentinel.command.CommandHandler diff --git a/pom.xml b/pom.xml index 944bfc7a..7abd2cd6 100755 --- a/pom.xml +++ b/pom.xml @@ -125,6 +125,11 @@ sentinel-transport-simple-http ${project.version} + + com.alibaba.csp + sentinel-transport-netty-http + ${project.version} + com.alibaba.csp sentinel-transport-common diff --git a/sentinel-transport/sentinel-transport-netty-http/src/main/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServerHandler.java b/sentinel-transport/sentinel-transport-netty-http/src/main/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServerHandler.java index 956ecad3..36146c2f 100755 --- a/sentinel-transport/sentinel-transport-netty-http/src/main/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServerHandler.java +++ b/sentinel-transport/sentinel-transport-netty-http/src/main/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServerHandler.java @@ -221,11 +221,15 @@ public class HttpServerHandler extends SimpleChannelInboundHandler { if (StringUtil.isEmpty(uri)) { return ""; } - String[] arr = uri.split("/"); - if (arr.length < 2) { - return ""; + + // Remove the / of the uri as the target(command name) + // Usually the uri is start with / + int start = uri.indexOf('/'); + if (start != -1) { + return uri.substring(start + 1); } - return arr[1]; + + return uri; } private CommandHandler getHandler(String commandName) { diff --git a/sentinel-transport/sentinel-transport-netty-http/src/test/java/com/alibaba/csp/sentinel/transport/command/handler/MultipleSlashNameCommandTestHandler.java b/sentinel-transport/sentinel-transport-netty-http/src/test/java/com/alibaba/csp/sentinel/transport/command/handler/MultipleSlashNameCommandTestHandler.java new file mode 100644 index 00000000..7baf60d8 --- /dev/null +++ b/sentinel-transport/sentinel-transport-netty-http/src/test/java/com/alibaba/csp/sentinel/transport/command/handler/MultipleSlashNameCommandTestHandler.java @@ -0,0 +1,32 @@ +/* + * Copyright 1999-2018 Alibaba Group Holding Ltd. + * + * Licensed 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 com.alibaba.csp.sentinel.transport.command.handler; + +import com.alibaba.csp.sentinel.command.CommandHandler; +import com.alibaba.csp.sentinel.command.CommandRequest; +import com.alibaba.csp.sentinel.command.CommandResponse; +import com.alibaba.csp.sentinel.command.annotation.CommandMapping; + +/** + * @author cdfive + */ +@CommandMapping(name = "aa/bb/cc", desc = "a test handler with multiple / in its name") +public class MultipleSlashNameCommandTestHandler implements CommandHandler { + @Override + public CommandResponse handle(CommandRequest request) { + return CommandResponse.ofSuccess("MultipleSlashNameCommandTestHandler result"); + } +} diff --git a/sentinel-transport/sentinel-transport-netty-http/src/test/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServerHandlerTest.java b/sentinel-transport/sentinel-transport-netty-http/src/test/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServerHandlerTest.java index 3102c217..2baf4f05 100644 --- a/sentinel-transport/sentinel-transport-netty-http/src/test/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServerHandlerTest.java +++ b/sentinel-transport/sentinel-transport-netty-http/src/test/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServerHandlerTest.java @@ -23,6 +23,8 @@ import com.alibaba.csp.sentinel.slots.block.flow.FlowRule; import com.alibaba.csp.sentinel.slots.block.flow.FlowRuleManager; import com.alibaba.csp.sentinel.transport.CommandCenter; import com.alibaba.csp.sentinel.transport.command.NettyHttpCommandCenter; +import com.alibaba.csp.sentinel.transport.command.handler.MultipleSlashNameCommandTestHandler; +import com.alibaba.fastjson.JSON; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; import io.netty.channel.embedded.EmbeddedChannel; @@ -34,6 +36,7 @@ import org.junit.Test; import java.lang.reflect.Field; import java.nio.charset.Charset; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; @@ -45,7 +48,6 @@ import static org.junit.Assert.assertEquals; * Test cases for {@link HttpServerHandler}. * * @author cdfive - * @date 2018-12-17 */ public class HttpServerHandlerTest { @@ -59,7 +61,7 @@ public class HttpServerHandlerTest { @BeforeClass public static void beforeClass() throws Exception { - // don't execute InitExecutor.doInit, to avoid CommandCenter SPI loaded + // Don't execute InitExecutor.doInit, to avoid CommandCenter SPI loaded Field[] declaredFields = InitExecutor.class.getDeclaredFields(); for (Field declaredField : declaredFields) { if (declaredField.getName().equals("initialized")) { @@ -68,23 +70,26 @@ public class HttpServerHandlerTest { } } - // create NettyHttpCommandCenter to create HttpServer + // Create NettyHttpCommandCenter to create HttpServer CommandCenter commandCenter = new NettyHttpCommandCenter(); - // call beforeStart to register handlers + // Call beforeStart to register handlers commandCenter.beforeStart(); } @Before public void before() { - // the same Handlers in order as the ChannelPipeline in HttpServerInitializer + // The same Handlers in order as the ChannelPipeline in HttpServerInitializer HttpRequestDecoder httpRequestDecoder = new HttpRequestDecoder(); HttpObjectAggregator httpObjectAggregator = new HttpObjectAggregator(1024 * 1024); HttpResponseEncoder httpResponseEncoder = new HttpResponseEncoder(); HttpServerHandler httpServerHandler = new HttpServerHandler(); - // create new EmbeddedChannel every method call + // Create new EmbeddedChannel every method call embeddedChannel = new EmbeddedChannel(httpRequestDecoder, httpObjectAggregator, httpResponseEncoder, httpServerHandler); + + // Clear flow rules + FlowRuleManager.loadRules(Collections.EMPTY_LIST); } @Test @@ -92,9 +97,9 @@ public class HttpServerHandlerTest { String httpRequestStr = "GET / HTTP/1.1" + CRLF + "Host: localhost:8719" + CRLF + CRLF; - String body = "Invalid command"; + String expectedBody = "Invalid command"; - processError(httpRequestStr, body); + processError(httpRequestStr, expectedBody); } @Test @@ -102,44 +107,49 @@ public class HttpServerHandlerTest { String httpRequestStr = "GET /aaa HTTP/1.1" + CRLF + "Host: localhost:8719" + CRLF + CRLF; - String body = String.format("Unknown command \"%s\"", "aaa"); + String expectedBody = String.format("Unknown command \"%s\"", "aaa"); - processError(httpRequestStr, body); + processError(httpRequestStr, expectedBody); } + /** + * {@link com.alibaba.csp.sentinel.command.handler.VersionCommandHandler} + */ @Test public void testVersionCommand() { String httpRequestStr = "GET /version HTTP/1.1" + CRLF + "Host: localhost:8719" + CRLF + CRLF; - String body = Constants.SENTINEL_VERSION; + String expectedBody = Constants.SENTINEL_VERSION; - processSuccess(httpRequestStr, body); + processSuccess(httpRequestStr, expectedBody); } + /** + * {@link com.alibaba.csp.sentinel.command.handler.FetchActiveRuleCommandHandler} + */ @Test - public void testGetRuleCommandInvalidType() { + public void testFetchActiveRuleCommandInvalidType() { String httpRequestStr = "GET /getRules HTTP/1.1" + CRLF + "Host: localhost:8719" + CRLF + CRLF; - String body = "invalid type"; + String expectedBody = "invalid type"; - processFailed(httpRequestStr, body); + processFailed(httpRequestStr, expectedBody); } @Test - public void testGetRuleCommandFlowEmptyRule() { + public void testFetchActiveRuleCommandEmptyRule() { String httpRequestStr = "GET /getRules?type=flow HTTP/1.1" + CRLF + "Host: localhost:8719" + CRLF + CRLF; - String body = "[]"; + String expectedBody = "[]"; - processSuccess(httpRequestStr, body); + processSuccess(httpRequestStr, expectedBody); } -// FIXME byteBuf.toString can't get body response now, need to find another way -// @Test - public void testGetRuleCommandFlowSomeRules() { + @Test + public void testFetchActiveRuleCommandSomeFlowRules() { List rules = new ArrayList(); FlowRule rule1 = new FlowRule(); rule1.setResource("key"); @@ -152,60 +162,86 @@ public class HttpServerHandlerTest { String httpRequestStr = "GET /getRules?type=flow HTTP/1.1" + CRLF + "Host: localhost:8719" + CRLF + CRLF; - String body = ""; - processSuccess(httpRequestStr, body); + // body json + /* + String expectedBody = "[{\"clusterMode\":false,\"controlBehavior\":0,\"count\":20.0" + + ",\"grade\":1,\"limitApp\":\"default\",\"maxQueueingTimeMs\":500" + + ",\"resource\":\"key\",\"strategy\":0,\"warmUpPeriodSec\":10}]"; + */ + String expectedBody = JSON.toJSONString(rules); + + processSuccess(httpRequestStr, expectedBody); } - private void processError(String httpRequestStr, String body) { - processError(httpRequestStr, BAD_REQUEST, body); + /** + * {@link MultipleSlashNameCommandTestHandler} + * + * Test command whose mapping path and command name contain multiple / + */ + @Test + public void testMultipleSlashNameCommand() { + String httpRequestStr = "GET /aa/bb/cc HTTP/1.1" + CRLF + + "Host: localhost:8719" + CRLF + + CRLF; + String expectedBody = "MultipleSlashNameCommandTestHandler result"; + + processSuccess(httpRequestStr, expectedBody); } - private void processError(String httpRequestStr, HttpResponseStatus status, String body) { + private void processError(String httpRequestStr, String expectedBody) { + processError(httpRequestStr, BAD_REQUEST, expectedBody); + } + + private void processError(String httpRequestStr, HttpResponseStatus status, String expectedBody) { String httpResponseStr = processResponse(httpRequestStr); - assertErrorStatusAndBody(status, body, httpResponseStr); + assertErrorStatusAndBody(status, expectedBody, httpResponseStr); } - private void processSuccess(String httpRequestStr, String body) { - process(httpRequestStr, OK, body); + private void processSuccess(String httpRequestStr, String expectedBody) { + process(httpRequestStr, OK, expectedBody); } - private void processFailed(String httpRequestStr, String body) { - process(httpRequestStr, BAD_REQUEST, body); + private void processFailed(String httpRequestStr, String expectedBody) { + process(httpRequestStr, BAD_REQUEST, expectedBody); } - private void process(String httpRequestStr, HttpResponseStatus status, String body) { + private void process(String httpRequestStr, HttpResponseStatus status, String expectedBody) { String responseStr = processResponse(httpRequestStr); - assertStatusAndBody(status, body, responseStr); + assertStatusAndBody(status, expectedBody, responseStr); } private String processResponse(String httpRequestStr) { embeddedChannel.writeInbound(Unpooled.wrappedBuffer(httpRequestStr.getBytes(SENTINEL_CHARSET))); - ByteBuf byteBuf = embeddedChannel.readOutbound(); + StringBuilder sb = new StringBuilder(); + + ByteBuf byteBuf; + while ((byteBuf = embeddedChannel.readOutbound()) != null) { + sb.append(byteBuf.toString(SENTINEL_CHARSET)); + } - String responseStr = byteBuf.toString(SENTINEL_CHARSET); - return responseStr; + return sb.toString(); } - private void assertErrorStatusAndBody(HttpResponseStatus status, String body, String httpResponseStr) { + private void assertErrorStatusAndBody(HttpResponseStatus status, String expectedBody, String httpResponseStr) { StringBuilder text = new StringBuilder(); text.append(HttpVersion.HTTP_1_1.toString()).append(' ').append(status.toString()).append(CRLF); text.append("Content-Type: text/plain; charset=").append(SENTINEL_CHARSET_NAME).append(CRLF); text.append(CRLF); - text.append(body); + text.append(expectedBody); assertEquals(text.toString(), httpResponseStr); } - private void assertStatusAndBody(HttpResponseStatus status, String body, String httpResponseStr) { + private void assertStatusAndBody(HttpResponseStatus status, String expectedBody, String httpResponseStr) { StringBuilder text = new StringBuilder(); text.append(HttpVersion.HTTP_1_1.toString()).append(' ').append(status.toString()).append(CRLF); text.append("Content-Type: text/plain; charset=").append(SENTINEL_CHARSET_NAME).append(CRLF); - text.append("content-length: " + body.length()).append(CRLF); + text.append("content-length: " + expectedBody.length()).append(CRLF); text.append("connection: close").append(CRLF); text.append(CRLF); - text.append(body); + text.append(expectedBody); assertEquals(text.toString(), httpResponseStr); } diff --git a/sentinel-transport/sentinel-transport-netty-http/src/test/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServerInitializerTest.java b/sentinel-transport/sentinel-transport-netty-http/src/test/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServerInitializerTest.java index ba43448c..68c0fe77 100644 --- a/sentinel-transport/sentinel-transport-netty-http/src/test/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServerInitializerTest.java +++ b/sentinel-transport/sentinel-transport-netty-http/src/test/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServerInitializerTest.java @@ -30,30 +30,29 @@ import static org.mockito.Mockito.*; * Test cases for {@link HttpServerInitializer}. * * @author cdfive - * @date 2018-12-19 */ public class HttpServerInitializerTest { @Test public void testInitChannel() throws Exception { - // mock Objects + // Mock Objects HttpServerInitializer httpServerInitializer = mock(HttpServerInitializer.class); SocketChannel socketChannel = mock(SocketChannel.class); ChannelPipeline channelPipeline = mock(ChannelPipeline.class); - // mock SocketChannel#pipeline() method + // Mock SocketChannel#pipeline() method when(socketChannel.pipeline()).thenReturn(channelPipeline); // HttpServerInitializer#initChannel(SocketChannel) call real method doCallRealMethod().when(httpServerInitializer).initChannel(socketChannel); - // start test for HttpServerInitializer#initChannel(SocketChannel) + // Start test for HttpServerInitializer#initChannel(SocketChannel) httpServerInitializer.initChannel(socketChannel); - // verify 4 times calling ChannelPipeline#addLast() method + // Verify 4 times calling ChannelPipeline#addLast() method verify(channelPipeline, times(4)).addLast(any(ChannelHandler.class)); - // verify the order of calling ChannelPipeline#addLast() method + // Verify the order of calling ChannelPipeline#addLast() method InOrder inOrder = inOrder(channelPipeline); inOrder.verify(channelPipeline).addLast(any(HttpRequestDecoder.class)); inOrder.verify(channelPipeline).addLast(any(HttpObjectAggregator.class)); diff --git a/sentinel-transport/sentinel-transport-netty-http/src/test/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServerTest.java b/sentinel-transport/sentinel-transport-netty-http/src/test/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServerTest.java index dbb213c1..a82d0fca 100644 --- a/sentinel-transport/sentinel-transport-netty-http/src/test/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServerTest.java +++ b/sentinel-transport/sentinel-transport-netty-http/src/test/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServerTest.java @@ -31,7 +31,6 @@ import static org.junit.Assert.*; * Test cases for {@link HttpServer}. * * @author cdfive - * @date 2018-12-19 */ public class HttpServerTest { @@ -39,20 +38,20 @@ public class HttpServerTest { @BeforeClass public static void beforeClass() { - // note: clear handlerMap first, as other test case may init HttpServer.handlerMap + // Note: clear handlerMap first, as other test case may init HttpServer.handlerMap // if not, run mvn test, the next assertEquals(0, HttpServer.handlerMap.size()) may fail HttpServer.handlerMap.clear(); - // create new HttpServer + // Create new HttpServer httpServer = new HttpServer(); - // no handler in handlerMap at first + // No handler in handlerMap at first assertEquals(0, HttpServer.handlerMap.size()); } @Before public void before() { - // clear handlerMap every method call + // Clear handlerMap every method call HttpServer.handlerMap.clear(); } @@ -61,37 +60,37 @@ public class HttpServerTest { String commandName; CommandHandler handler; - // if commandName is null, no handler added in handlerMap + // If commandName is null, no handler added in handlerMap commandName = null; handler = new VersionCommandHandler(); httpServer.registerCommand(commandName, handler); assertEquals(0, HttpServer.handlerMap.size()); - // if commandName is "", no handler added in handlerMap + // If commandName is "", no handler added in handlerMap commandName = ""; handler = new VersionCommandHandler(); httpServer.registerCommand(commandName, handler); assertEquals(0, HttpServer.handlerMap.size()); - // if handler is null, no handler added in handlerMap + // If handler is null, no handler added in handlerMap commandName = "version"; handler = null; httpServer.registerCommand(commandName, handler); assertEquals(0, HttpServer.handlerMap.size()); - // add one handler, commandName:version, handler:VersionCommandHandler + // Add one handler, commandName:version, handler:VersionCommandHandler commandName = "version"; handler = new VersionCommandHandler(); httpServer.registerCommand(commandName, handler); assertEquals(1, HttpServer.handlerMap.size()); - // add the same name Handler, no handler added in handlerMap + // Add the same name Handler, no handler added in handlerMap commandName = "version"; handler = new VersionCommandHandler(); httpServer.registerCommand(commandName, handler); assertEquals(1, HttpServer.handlerMap.size()); - // add another handler, commandName:basicInfo, handler:BasicInfoCommandHandler + // Add another handler, commandName:basicInfo, handler:BasicInfoCommandHandler commandName = "basicInfo"; handler = new BasicInfoCommandHandler(); httpServer.registerCommand(commandName, handler); @@ -102,16 +101,16 @@ public class HttpServerTest { public void testRegisterCommands() { Map handlerMap = null; - // if handlerMap is null, no handler added in handlerMap + // If handlerMap is null, no handler added in handlerMap httpServer.registerCommands(handlerMap); assertEquals(0, HttpServer.handlerMap.size()); - // add handler from CommandHandlerProvider + // Add handler from CommandHandlerProvider handlerMap = CommandHandlerProvider.getInstance().namedHandlers(); httpServer.registerCommands(handlerMap); - // check same size + // Check same size assertEquals(handlerMap.size(), HttpServer.handlerMap.size()); - // check not same reference + // Check not same reference assertTrue(handlerMap != HttpServer.handlerMap); } } diff --git a/sentinel-transport/sentinel-transport-netty-http/src/test/resources/META-INF/services/com.alibaba.csp.sentinel.command.CommandHandler b/sentinel-transport/sentinel-transport-netty-http/src/test/resources/META-INF/services/com.alibaba.csp.sentinel.command.CommandHandler new file mode 100755 index 00000000..3c64c0a2 --- /dev/null +++ b/sentinel-transport/sentinel-transport-netty-http/src/test/resources/META-INF/services/com.alibaba.csp.sentinel.command.CommandHandler @@ -0,0 +1 @@ +com.alibaba.csp.sentinel.transport.command.handler.MultipleSlashNameCommandTestHandler \ No newline at end of file diff --git a/sentinel-transport/sentinel-transport-simple-http/src/main/java/com/alibaba/csp/sentinel/transport/heartbeat/SimpleHttpHeartbeatSender.java b/sentinel-transport/sentinel-transport-simple-http/src/main/java/com/alibaba/csp/sentinel/transport/heartbeat/SimpleHttpHeartbeatSender.java index a230c125..aa4e571a 100755 --- a/sentinel-transport/sentinel-transport-simple-http/src/main/java/com/alibaba/csp/sentinel/transport/heartbeat/SimpleHttpHeartbeatSender.java +++ b/sentinel-transport/sentinel-transport-simple-http/src/main/java/com/alibaba/csp/sentinel/transport/heartbeat/SimpleHttpHeartbeatSender.java @@ -100,7 +100,7 @@ public class SimpleHttpHeartbeatSender implements HeartbeatSender { try { String ipsStr = TransportConfig.getConsoleServer(); if (StringUtil.isEmpty(ipsStr)) { - RecordLog.warn("[NettyHttpHeartbeatSender] Dashboard server address not configured"); + RecordLog.warn("[SimpleHttpHeartbeatSender] Dashboard server address not configured"); return newAddrs; }