This is an automated email from the ASF dual-hosted git repository.
liubao pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/incubator-servicecomb-java-chassis.git
The following commit(s) were added to refs/heads/master by this push:
new 37230e4 [SCB-936] encode slash in path param
37230e4 is described below
commit 37230e4f06772b066750169c291f04408c4e88ca
Author: yhs0092 <[email protected]>
AuthorDate: Fri Sep 28 15:57:04 2018 +0800
[SCB-936] encode slash in path param
---
.../rest/definition/path/PathVarParamWriter.java | 4 +-
.../definition/path/PathVarParamWriterTest.java | 9 +++
.../foundation/common/http/HttpUtils.java | 35 ++++++++++++
.../foundation/common/http/TestHttpUtils.java | 24 ++++++++
.../it/extend/engine/ITSCBRestTemplate.java | 9 ++-
.../it/testcase/TestDataTypePrimitive.java | 66 ++++++++++++++--------
.../it/SpringBoot2ServletApplication.java | 9 +++
.../vertx/accesslog/AccessLogConfiguration.java | 2 +-
.../accesslog/AccessLogConfigurationTest.java | 2 +-
9 files changed, 133 insertions(+), 27 deletions(-)
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 95f4016..3cde1c0 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.common.rest.definition.RestParam;
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 class PathVarParamWriter extends
AbstractUrlParamWriter {
@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 664c406..dbc87f9 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
@@ -54,6 +54,15 @@ public class PathVarParamWriterTest {
}
@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();
StringBuilder pathBuilder = new StringBuilder();
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 ed94d76..0c77d19 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 java.net.URISyntaxException;
import org.springframework.util.StringUtils;
+import com.google.common.net.UrlEscapers;
+
public final class HttpUtils {
private HttpUtils() {
}
@@ -50,6 +52,25 @@ public final class HttpUtils {
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 final class HttpUtils {
}
}
+ /**
+ * 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 98996ee..43587fc 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
@@ -94,6 +94,30 @@ public class TestHttpUtils {
}
@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);
expectedException
diff --git
a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/extend/engine/ITSCBRestTemplate.java
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/extend/engine/ITSCBRestTemplate.java
index 444ffc0..9956e50 100644
---
a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/extend/engine/ITSCBRestTemplate.java
+++
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/extend/engine/ITSCBRestTemplate.java
@@ -24,13 +24,16 @@ import
org.apache.servicecomb.serviceregistry.consumer.MicroserviceVersionRule;
import org.apache.servicecomb.serviceregistry.definition.DefinitionConst;
public class ITSCBRestTemplate extends CseRestTemplate {
+
+ private String urlPrefix;
+
public ITSCBRestTemplate(String producerName, String schemaId) {
MicroserviceVersionRule microserviceVersionRule =
RegistryUtils.getServiceRegistry().getAppManager()
.getOrCreateMicroserviceVersionRule(RegistryUtils.getAppId(),
producerName,
DefinitionConst.VERSION_RULE_ALL);
MicroserviceVersionMeta microserviceVersionMeta =
microserviceVersionRule.getLatestMicroserviceVersion();
SchemaMeta schemaMeta =
microserviceVersionMeta.getMicroserviceMeta().ensureFindSchemaMeta(schemaId);
- String urlPrefix = String.format("cse://%s/%s", producerName,
schemaMeta.getSwagger().getBasePath());
+ urlPrefix = String.format("cse://%s/%s", producerName,
schemaMeta.getSwagger().getBasePath());
setUriTemplateHandler(new ITUriTemplateHandler(urlPrefix));
setRequestFactory(new ITClientHttpRequestFactory());
@@ -39,4 +42,8 @@ public class ITSCBRestTemplate extends CseRestTemplate {
public void setTransport(String transport) {
((ITClientHttpRequestFactory) getRequestFactory()).setTransport(transport);
}
+
+ public String getUrlPrefix() {
+ return urlPrefix;
+ }
}
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 da9f294..83423ce 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
@@ -18,17 +18,20 @@ package org.apache.servicecomb.it.testcase;
import static org.junit.Assert.assertEquals;
+import java.net.URI;
import java.util.HashMap;
import java.util.Map;
import org.apache.servicecomb.it.Consumers;
+import org.apache.servicecomb.it.extend.engine.ITSCBRestTemplate;
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;
import org.springframework.http.HttpMethod;
import org.springframework.http.ResponseEntity;
+import org.springframework.web.util.UriComponentsBuilder;
public class TestDataTypePrimitive {
interface DataTypePojoIntf {
@@ -99,17 +102,15 @@ public class TestDataTypePrimitive {
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
@@ -204,7 +205,7 @@ public class TestDataTypePrimitive {
@Test
public void stringPath_jaxrs_intf() {
- String expect = "serviceComb";
+ String expect = "serviceComb/serviceComb";
assertEquals(expect, consumersJaxrs.getIntf().stringPath(expect));
}
@@ -227,6 +228,19 @@ public class TestDataTypePrimitive {
}
@Test
+ public void stringPath_jaxrs_rt_with_encoded_slash() {
+ String requestPathParam = "serviceComb%2FserviceComb";
+ String expectResponse = "serviceComb/serviceComb";
+ // build request uri to avoid Spring's encoding path
+ URI requestUri = UriComponentsBuilder
+ .fromUriString(((ITSCBRestTemplate)
consumersJaxrs.getSCBRestTemplate()).getUrlPrefix()
+ + "/stringPath/" + requestPathParam)
+ .build(true).toUri();
+ assertEquals(expectResponse,
+ consumersJaxrs.getSCBRestTemplate().getForObject(requestUri,
String.class));
+ }
+
+ @Test
public void intQuery_jaxrs_intf() {
assertEquals(10, consumersJaxrs.getIntf().intQuery(10));
}
@@ -290,8 +304,7 @@ public class TestDataTypePrimitive {
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,
@@ -324,8 +337,7 @@ public class TestDataTypePrimitive {
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,
@@ -364,8 +376,7 @@ public class TestDataTypePrimitive {
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,
@@ -398,8 +409,7 @@ public class TestDataTypePrimitive {
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,
@@ -546,7 +556,7 @@ public class TestDataTypePrimitive {
@Test
public void stringPath_springmvc_intf() {
- String expect = "serviceComb";
+ String expect = "serviceComb/serviceComb";
assertEquals(expect, consumersSpringmvc.getIntf().stringPath(expect));
}
@@ -569,6 +579,18 @@ public class TestDataTypePrimitive {
}
@Test
+ public void stringPath_springmvc_rt_with_encoded_slash() {
+ String requestPathParam = "serviceComb%2FserviceComb";
+ String expectResponse = "serviceComb/serviceComb";
+ URI requestUri = UriComponentsBuilder
+ .fromUriString(((ITSCBRestTemplate)
consumersSpringmvc.getSCBRestTemplate()).getUrlPrefix()
+ + "/stringPath/" + requestPathParam)
+ .build(true).toUri();
+ assertEquals(expectResponse,
+ consumersSpringmvc.getSCBRestTemplate().getForObject(requestUri,
String.class));
+ }
+
+ @Test
public void intQuery_springmvc_intf() {
assertEquals(10, consumersSpringmvc.getIntf().intQuery(10));
}
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 87236e3..6b19ad8 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 @@ import
org.springframework.boot.autoconfigure.SpringBootApplication;
@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 6f10955..eee4307 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 final class AccessLogConfiguration {
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 e4ae886..0dd1af6 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 class AccessLogConfigurationTest {
@Test
public void getAccesslogPattern() {
String result = AccessLogConfiguration.INSTANCE.getAccesslogPattern();
- assertEquals("%h - - %t %r %s %B", result);
+ assertEquals("%h - - %t %r %s %B %D", result);
}
}
