[
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)
