[
https://issues.apache.org/jira/browse/SCB-936?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16631497#comment-16631497
]
ASF GitHub Bot commented on SCB-936:
------------------------------------
liubao68 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/extend/engine/ITSCBRestTemplate.java
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/extend/engine/ITSCBRestTemplate.java
index 444ffc0bf..9956e503f 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.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 ITSCBRestTemplate(String producerName, String
schemaId) {
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 da9f29430..83423cebd 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 @@
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 @@
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 void doublePath_jaxrs_intf() {
@Test
public void stringPath_jaxrs_intf() {
- String expect = "serviceComb";
+ String expect = "serviceComb/serviceComb";
assertEquals(expect, consumersJaxrs.getIntf().stringPath(expect));
}
@@ -226,6 +227,19 @@ public void stringPath_jaxrs_rt() {
consumersJaxrs.getSCBRestTemplate().getForObject("/stringPath/" +
expect, String.class));
}
+ @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 @@ 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,
@@ -324,8 +337,7 @@ 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,
@@ -364,8 +376,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,
@@ -398,8 +409,7 @@ 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,
@@ -546,7 +556,7 @@ public void doublePath_springmvc_intf() {
@Test
public void stringPath_springmvc_intf() {
- String expect = "serviceComb";
+ String expect = "serviceComb/serviceComb";
assertEquals(expect, consumersSpringmvc.getIntf().stringPath(expect));
}
@@ -568,6 +578,18 @@ public void stringPath_springmvc_rt() {
consumersSpringmvc.getSCBRestTemplate().getForObject("/stringPath/" +
expect, String.class));
}
+ @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 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:
[email protected]
> 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)
