[ https://issues.apache.org/jira/browse/SCB-936?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16628166#comment-16628166 ]
ASF GitHub Bot commented on SCB-936: ------------------------------------ yhs0092 closed pull request #924: [SCB-936] encode slash in path param URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/924 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/definition/path/PathVarParamWriter.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/definition/path/PathVarParamWriter.java index 95f401698..3cde1c02f 100644 --- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/definition/path/PathVarParamWriter.java +++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/definition/path/PathVarParamWriter.java @@ -21,7 +21,7 @@ import org.apache.servicecomb.foundation.common.http.HttpUtils; /** - * 处理动态path + * Dynamically processing path */ public class PathVarParamWriter extends AbstractUrlParamWriter { public PathVarParamWriter(RestParam param) { @@ -31,7 +31,7 @@ public PathVarParamWriter(RestParam param) { @Override public void write(StringBuilder builder, Object[] args) throws Exception { String paramValue = getParamValue(args).toString(); - String encodedPathParam = HttpUtils.uriEncodePath(paramValue); + String encodedPathParam = HttpUtils.encodePathParam(paramValue); builder.append(encodedPathParam); } } diff --git a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/definition/path/PathVarParamWriterTest.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/definition/path/PathVarParamWriterTest.java index 664c40697..dbc87f9f2 100644 --- a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/definition/path/PathVarParamWriterTest.java +++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/definition/path/PathVarParamWriterTest.java @@ -53,6 +53,15 @@ public void writePathWithPercentage() throws Exception { Assert.assertEquals("/api/a%25%25bc", pathBuilder.toString()); } + @Test + public void writePathParamWithSlash() throws Exception { + PathVarParamWriter pathVarParamWriter = createPathVarParamWriter(); + StringBuilder pathBuilder = new StringBuilder(); + pathBuilder.append("/api/"); + pathVarParamWriter.write(pathBuilder, new String[] {"a/bc"}); + Assert.assertEquals("/api/a%2Fbc", pathBuilder.toString()); + } + @Test public void writeIntegerParam() throws Exception { PathVarParamWriter pathVarParamWriter = createPathVarParamWriter(); diff --git a/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/http/HttpUtils.java b/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/http/HttpUtils.java index ed94d7611..0c77d19b8 100644 --- a/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/http/HttpUtils.java +++ b/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/http/HttpUtils.java @@ -22,6 +22,8 @@ import org.springframework.util.StringUtils; +import com.google.common.net.UrlEscapers; + public final class HttpUtils { private HttpUtils() { } @@ -50,6 +52,25 @@ public static String parseParamFromHeaderValue(String headerValue, String paramN return null; } + /** + * <pre> + * foo://example.com:8042/over/there?name=ferret#nose + * \_/ \______________/\_________/ \_________/ \__/ + * | | | | | + * scheme authority path query fragment + * | _____________________|__ + * / \ / \ + * urn:example:animal:ferret:nose + * </pre> + * <p>the URI syntax components above is referred from <a href="https://tools.ietf.org/html/rfc3986#page-16">RFC3986</a>. + * This method is used to encode the entire path part(e.g. /over/there in the example).</p> + * <em>In order to keep the structure of path, slash '/' will not be encoded. If you want to encode '/' into {@code %2F}, + * please consider the {@link #encodePathParam(String)} + * </em> + * + * @param path the entire url path + * @return the encoded url path + */ public static String uriEncodePath(String path) { try { URI uri = new URI(null, null, path, null); @@ -59,6 +80,20 @@ public static String uriEncodePath(String path) { } } + /** + * Encode path params. For example, if the path of an operation is {@code /over/there/{pathParam}/tail}, this method + * should be used to encoded {@code {pathParam}}. In order to keep the path structure, the slash '/' will be encoded + * into {@code %2F} to avoid path matching problem. + * + * @see UrlEscapers#urlPathSegmentEscaper() + * + * @param pathParam the path param to be encoded + * @return the encoded path param + */ + public static String encodePathParam(String pathParam) { + return UrlEscapers.urlPathSegmentEscaper().escape(pathParam); + } + public static String uriDecodePath(String path) { if (path == null) { return null; diff --git a/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/http/TestHttpUtils.java b/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/http/TestHttpUtils.java index 98996ee70..43587fcfb 100644 --- a/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/http/TestHttpUtils.java +++ b/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/http/TestHttpUtils.java @@ -93,6 +93,30 @@ public void uriEncode_plus() { Assert.assertEquals("a+b", HttpUtils.uriDecodePath(encoded)); } + @Test + public void uriEncode_encodeEntirePath() { + String encoded = HttpUtils.uriEncodePath("a%%'+b/def"); + Assert.assertEquals("a%25%25'+b/def", encoded); + } + + @Test + public void pathParamEncode() { + Assert.assertEquals("a+b", HttpUtils.encodePathParam("a+b")); + Assert.assertEquals("a%25b", HttpUtils.encodePathParam("a%b")); + Assert.assertEquals("a%25%25b", HttpUtils.encodePathParam("a%%b")); + Assert.assertEquals("%3C%20%3E'%22%EF%BC%88)&%2F%20%20", HttpUtils.encodePathParam("< >'\"()&/ ")); + Assert.assertEquals("%E6%B5%8B%20%E8%AF%95", HttpUtils.encodePathParam("测 试")); + } + + /** + * SafeChar: the characters that are not encoded. + * This test is to show those safe chars excepting 0..9, a..z and A..Z + */ + @Test + public void pathParamEncode_SafeChar() { + Assert.assertEquals("-._~!$'()*,;&=@:+", HttpUtils.encodePathParam("-._~!$'()*,;&=@:+")); + } + @Test public void uriDecode_failed() { expectedException.expect(IllegalArgumentException.class); diff --git a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestDataTypePrimitive.java b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestDataTypePrimitive.java index 473fc5486..be75b8715 100644 --- a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestDataTypePrimitive.java +++ b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestDataTypePrimitive.java @@ -23,7 +23,7 @@ import org.apache.servicecomb.it.Consumers; import org.apache.servicecomb.it.junit.ITJUnitUtils; -import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Test; import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; @@ -80,17 +80,15 @@ private static String producerName; - @Before - public void prepare() { - if (!ITJUnitUtils.getProducerName().equals(producerName)) { - producerName = ITJUnitUtils.getProducerName(); - consumersPojo = new Consumers<>(producerName, "dataTypePojo", DataTypePojoIntf.class); - consumersJaxrs = new Consumers<>(producerName, "dataTypeJaxrs", DataTypeRestIntf.class); - consumersSpringmvc = new Consumers<>(producerName, "dataTypeSpringmvc", DataTypeRestIntf.class); - consumersPojo.init(ITJUnitUtils.getTransport()); - consumersJaxrs.init(ITJUnitUtils.getTransport()); - consumersSpringmvc.init(ITJUnitUtils.getTransport()); - } + @BeforeClass + public static void beforeClass() { + producerName = ITJUnitUtils.getProducerName(); + consumersPojo = new Consumers<>(producerName, "dataTypePojo", DataTypePojoIntf.class); + consumersJaxrs = new Consumers<>(producerName, "dataTypeJaxrs", DataTypeRestIntf.class); + consumersSpringmvc = new Consumers<>(producerName, "dataTypeSpringmvc", DataTypeRestIntf.class); + consumersPojo.init(ITJUnitUtils.getTransport()); + consumersJaxrs.init(ITJUnitUtils.getTransport()); + consumersSpringmvc.init(ITJUnitUtils.getTransport()); } @Test @@ -116,7 +114,7 @@ public void string_pojo_rt() { String expect = "serviceComb"; Map<String, String> map = new HashMap<>(); map.put("input", expect); - assertEquals(expect, (String) consumersPojo.getSCBRestTemplate().postForObject("/stringBody", map, String.class)); + assertEquals(expect, consumersPojo.getSCBRestTemplate().postForObject("/stringBody", map, String.class)); } @Test @@ -143,7 +141,7 @@ public void string_concat_pojo_rt() { map.put("str1", "service"); map.put("str2", "Comb"); assertEquals("serviceComb", - (String) consumersPojo.getSCBRestTemplate().postForObject("/stringConcat", map, String.class)); + consumersPojo.getSCBRestTemplate().postForObject("/stringConcat", map, String.class)); } @Test @@ -153,7 +151,7 @@ public void intPath_jaxrs_intf() { @Test public void stringPath_jaxrs_intf() { - String expect = "serviceComb"; + String expect = "serviceComb/serviceComb"; assertEquals(expect, consumersJaxrs.getIntf().stringPath(expect)); } @@ -164,9 +162,10 @@ public void intPath_jaxrs_rt() { @Test public void stringPath_jaxrs_rt() { - String expect = "serviceComb"; + // restTemplate does not encode slash, so we do it in advance + String expect = "serviceComb%2FserviceComb"; assertEquals(expect, - (String) consumersJaxrs.getSCBRestTemplate().getForObject("/stringPath/" + expect, String.class)); + consumersJaxrs.getSCBRestTemplate().getForObject("/stringPath/" + expect, String.class)); } @Test @@ -189,7 +188,7 @@ public void intQuery_jaxrs_rt() { public void stringQuery_jaxrs_rt() { String expect = "serviceComb"; assertEquals(expect, - (String) consumersJaxrs.getSCBRestTemplate().getForObject("/stringQuery?input=" + expect, String.class)); + consumersJaxrs.getSCBRestTemplate().getForObject("/stringQuery?input=" + expect, String.class)); } @Test @@ -212,8 +211,7 @@ protected void intHeader_rt(Consumers<DataTypeRestIntf> consumers) { HttpHeaders headers = new HttpHeaders(); headers.add("input", "10"); - @SuppressWarnings("rawtypes") - HttpEntity entity = new HttpEntity<>(null, headers); + HttpEntity<?> entity = new HttpEntity<>(null, headers); ResponseEntity<Integer> response = consumers.getSCBRestTemplate() .exchange("/intHeader", HttpMethod.GET, @@ -232,14 +230,13 @@ protected void stringHeader_rt(Consumers<DataTypeRestIntf> consumers) { HttpHeaders headers = new HttpHeaders(); headers.add("input", expect); - @SuppressWarnings("rawtypes") - HttpEntity entity = new HttpEntity<>(null, headers); + HttpEntity<?> entity = new HttpEntity<>(null, headers); ResponseEntity<String> response = consumers.getSCBRestTemplate() .exchange("/stringHeader", HttpMethod.GET, entity, String.class); - assertEquals(expect, (String) response.getBody()); + assertEquals(expect, response.getBody()); } @Test @@ -262,8 +259,7 @@ void intCookie_rt(Consumers<DataTypeRestIntf> consumers) { HttpHeaders headers = new HttpHeaders(); headers.add("Cookie", "input=10"); - @SuppressWarnings("rawtypes") - HttpEntity entity = new HttpEntity<>(null, headers); + HttpEntity<?> entity = new HttpEntity<>(null, headers); ResponseEntity<Integer> response = consumers.getSCBRestTemplate() .exchange("/intCookie", HttpMethod.GET, @@ -282,14 +278,13 @@ void stringCookie_rt(Consumers<DataTypeRestIntf> consumers) { HttpHeaders headers = new HttpHeaders(); headers.add("Cookie", "input=" + expect); - @SuppressWarnings("rawtypes") - HttpEntity entity = new HttpEntity<>(null, headers); + HttpEntity<?> entity = new HttpEntity<>(null, headers); ResponseEntity<String> response = consumers.getSCBRestTemplate() .exchange("/stringCookie", HttpMethod.GET, entity, String.class); - assertEquals(expect, (String) response.getBody()); + assertEquals(expect, response.getBody()); } @Test @@ -324,13 +319,13 @@ public void stringForm_jaxrs_rt() { HttpEntity<Map<String, String>> formEntiry = new HttpEntity<>(map); assertEquals(expect, - (String) consumersJaxrs.getSCBRestTemplate() + consumersJaxrs.getSCBRestTemplate() .postForEntity("/stringForm", formEntiry, String.class) .getBody()); //you can use another method to invoke it assertEquals(expect, - (String) consumersJaxrs.getSCBRestTemplate().postForEntity("/stringForm", map, String.class).getBody()); + consumersJaxrs.getSCBRestTemplate().postForEntity("/stringForm", map, String.class).getBody()); } @Test @@ -353,7 +348,7 @@ public void intBody_jaxrs_rt() { public void stringBody_jaxrs_rt() { String expect = "serviceComb"; assertEquals(expect, - (String) consumersJaxrs.getSCBRestTemplate().postForObject("/stringBody", expect, String.class)); + consumersJaxrs.getSCBRestTemplate().postForObject("/stringBody", expect, String.class)); } @Test @@ -373,7 +368,7 @@ public void add_jaxrs_rt() { @Test public void string_concat_jaxrs_rt() { - assertEquals("serviceComb", (String) consumersJaxrs.getSCBRestTemplate() + assertEquals("serviceComb", consumersJaxrs.getSCBRestTemplate() .getForObject("/stringConcat?str1=service&str2=Comb", String.class)); } @@ -384,7 +379,7 @@ public void intPath_springmvc_intf() { @Test public void stringPath_springmvc_intf() { - String expect = "serviceComb"; + String expect = "serviceComb/serviceComb"; assertEquals(expect, consumersSpringmvc.getIntf().stringPath(expect)); } @@ -395,9 +390,9 @@ public void intPath_springmvc_rt() { @Test public void stringPath_springmvc_rt() { - String expect = "serviceComb"; + String expect = "serviceComb%2FserviceComb"; assertEquals(expect, - (String) consumersSpringmvc.getSCBRestTemplate().getForObject("/stringPath/" + expect, String.class)); + consumersSpringmvc.getSCBRestTemplate().getForObject("/stringPath/" + expect, String.class)); } @Test @@ -420,7 +415,7 @@ public void intQuery_springmvc_rt() { public void stringQuery_springmvc_rt() { String expect = "serviceComb"; assertEquals(expect, - (String) consumersSpringmvc.getSCBRestTemplate().getForObject("/stringQuery?input=" + expect, String.class)); + consumersSpringmvc.getSCBRestTemplate().getForObject("/stringQuery?input=" + expect, String.class)); } @Test @@ -494,11 +489,11 @@ public void stringForm_springmvc_rt() { HttpEntity<Map<String, String>> formEntiry = new HttpEntity<>(map); assertEquals(expect, - (String) consumersSpringmvc.getSCBRestTemplate().postForEntity("/stringForm", formEntiry, String.class) + consumersSpringmvc.getSCBRestTemplate().postForEntity("/stringForm", formEntiry, String.class) .getBody()); assertEquals(expect, - (String) consumersSpringmvc.getSCBRestTemplate().postForEntity("/stringForm", map, String.class) + consumersSpringmvc.getSCBRestTemplate().postForEntity("/stringForm", map, String.class) .getBody()); } @@ -522,7 +517,7 @@ public void intBody_springmvc_rt() { public void stringBody_springmvc_rt() { String expect = "serviceComb"; assertEquals(expect, - (String) consumersSpringmvc.getSCBRestTemplate().postForObject("/stringBody", expect, String.class)); + consumersSpringmvc.getSCBRestTemplate().postForObject("/stringBody", expect, String.class)); } @Test @@ -543,7 +538,7 @@ public void add_springmvc_rt() { @Test public void string_concat_springmvc_rt() { assertEquals("serviceComb", - (String) consumersSpringmvc.getSCBRestTemplate() + consumersSpringmvc.getSCBRestTemplate() .getForObject("/stringConcat?str1=service&str2=Comb", String.class)); } } diff --git a/integration-tests/it-producer-deploy-springboot2-servlet/src/main/java/org/apache/servicecomb/it/SpringBoot2ServletApplication.java b/integration-tests/it-producer-deploy-springboot2-servlet/src/main/java/org/apache/servicecomb/it/SpringBoot2ServletApplication.java index 87236e3b1..6b19ad8f4 100644 --- a/integration-tests/it-producer-deploy-springboot2-servlet/src/main/java/org/apache/servicecomb/it/SpringBoot2ServletApplication.java +++ b/integration-tests/it-producer-deploy-springboot2-servlet/src/main/java/org/apache/servicecomb/it/SpringBoot2ServletApplication.java @@ -24,9 +24,18 @@ @SpringBootApplication @EnableServiceComb public class SpringBoot2ServletApplication { + + /** + * Allow encoded slash in path param(%2F). + * This property is set for {@code stringPath} tests in {@code TestDataTypePrimitive} + */ + private static final String TOMCAT_ALLOW_ENCODED_SLASH = "org.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH"; + public static void main(String[] args) { new CommandReceiver(); + System.setProperty(TOMCAT_ALLOW_ENCODED_SLASH, "true"); SpringApplication.run(SpringBoot2ServletApplication.class, args); + System.clearProperty(TOMCAT_ALLOW_ENCODED_SLASH); } } diff --git a/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/accesslog/AccessLogConfiguration.java b/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/accesslog/AccessLogConfiguration.java index 6f10955f1..eee43079f 100644 --- a/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/accesslog/AccessLogConfiguration.java +++ b/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/accesslog/AccessLogConfiguration.java @@ -29,7 +29,7 @@ public static final AccessLogConfiguration INSTANCE = new AccessLogConfiguration(); - public static final String DEFAULT_PATTERN = "%h - - %t %r %s %B"; + public static final String DEFAULT_PATTERN = "%h - - %t %r %s %B %D"; private AccessLogConfiguration() { diff --git a/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/accesslog/AccessLogConfigurationTest.java b/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/accesslog/AccessLogConfigurationTest.java index e4ae88691..0dd1af6f3 100644 --- a/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/accesslog/AccessLogConfigurationTest.java +++ b/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/accesslog/AccessLogConfigurationTest.java @@ -33,6 +33,6 @@ public void getAccessLogEnabled() { @Test public void getAccesslogPattern() { String result = AccessLogConfiguration.INSTANCE.getAccesslogPattern(); - assertEquals("%h - - %t %r %s %B", result); + assertEquals("%h - - %t %r %s %B %D", result); } } ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Encoded slash '/' is decoded in EdgeService, causing 404 error response > ----------------------------------------------------------------------- > > Key: SCB-936 > URL: https://issues.apache.org/jira/browse/SCB-936 > Project: Apache ServiceComb > Issue Type: New Feature > Reporter: YaoHaishi > Assignee: YaoHaishi > Priority: Major > > When users invoke a provider via EdgeService, and the path param of request > contains %2F(the encoded slash '/'), EdgeService will decode %2F into '/' and > pass it to backend service without encoding it, causing 404 response. -- This message was sent by Atlassian JIRA (v7.6.3#76005)