[
https://issues.apache.org/jira/browse/SCB-886?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16605311#comment-16605311
]
ASF GitHub Bot commented on SCB-886:
------------------------------------
liubao68 closed pull request #888: [SCB-886] Fix path param encode decode
problem
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/888
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/codec/param/PathProcessorCreator.java
b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/PathProcessorCreator.java
index b69db9f3e..dbf459c80 100644
---
a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/PathProcessorCreator.java
+++
b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/PathProcessorCreator.java
@@ -18,13 +18,13 @@
package org.apache.servicecomb.common.rest.codec.param;
import java.lang.reflect.Type;
-import java.net.URLDecoder;
import java.util.Map;
import javax.servlet.http.HttpServletRequest;
import org.apache.servicecomb.common.rest.RestConst;
import org.apache.servicecomb.common.rest.codec.RestClientRequest;
+import org.apache.servicecomb.foundation.common.http.HttpUtils;
import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.type.TypeFactory;
@@ -52,7 +52,7 @@ public Object getValue(HttpServletRequest request) throws
Exception {
if (value == null) {
return null;
}
- return convertValue(URLDecoder.decode(value, "UTF-8"), targetType);
+ return convertValue(HttpUtils.uriDecodePath(value), targetType);
}
@Override
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 95659e8dc..95f401698 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
@@ -18,6 +18,7 @@
package org.apache.servicecomb.common.rest.definition.path;
import org.apache.servicecomb.common.rest.definition.RestParam;
+import org.apache.servicecomb.foundation.common.http.HttpUtils;
/**
* 处理动态path
@@ -29,6 +30,8 @@ public PathVarParamWriter(RestParam param) {
@Override
public void write(StringBuilder builder, Object[] args) throws Exception {
- builder.append(getParamValue(args));
+ String paramValue = getParamValue(args).toString();
+ String encodedPathParam = HttpUtils.uriEncodePath(paramValue);
+ builder.append(encodedPathParam);
}
}
diff --git
a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestPathProcessor.java
b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestPathProcessor.java
index 0fd25aec4..19cd51f7e 100644
---
a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestPathProcessor.java
+++
b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestPathProcessor.java
@@ -17,8 +17,6 @@
package org.apache.servicecomb.common.rest.codec.param;
-import java.net.URLEncoder;
-import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.Map;
@@ -79,13 +77,29 @@ public void testGetValuePathNormal() throws Exception {
}
@Test
- public void testGetValuePathEncoded() throws Exception {
+ public void testGetSpaceEncoded() throws Exception {
prepareGetValue("name", String.class);
- pathVars.put("name", URLEncoder.encode("a b",
StandardCharsets.UTF_8.name()));
+ pathVars.put("name", "a%20b");
Assert.assertEquals("a b", processor.getValue(request));
}
+ @Test
+ public void testGetPlus() throws Exception {
+ prepareGetValue("name", String.class);
+ pathVars.put("name", "a+b");
+
+ Assert.assertEquals("a+b", processor.getValue(request));
+ }
+
+ @Test
+ public void testGetPercentage() throws Exception {
+ prepareGetValue("name", String.class);
+ pathVars.put("name", "%25%25");
+
+ Assert.assertEquals("%%", processor.getValue(request));
+ }
+
@Test
public void testGetProcessorType() {
createProcessor("name", String.class);
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
new file mode 100644
index 000000000..664c40697
--- /dev/null
+++
b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/definition/path/PathVarParamWriterTest.java
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.servicecomb.common.rest.definition.path;
+
+import org.apache.servicecomb.common.rest.definition.RestParam;
+import org.junit.Assert;
+import org.junit.Test;
+
+import mockit.Mock;
+import mockit.MockUp;
+
+public class PathVarParamWriterTest {
+
+ @Test
+ public void writePlainPath() throws Exception {
+ PathVarParamWriter pathVarParamWriter = createPathVarParamWriter();
+
+ StringBuilder pathBuilder = new StringBuilder();
+ pathVarParamWriter.write(pathBuilder, new Object[] {"abc"});
+ Assert.assertEquals("abc", pathBuilder.toString());
+ }
+
+ @Test
+ public void writePathWithSpace() throws Exception {
+ PathVarParamWriter pathVarParamWriter = createPathVarParamWriter();
+
+ StringBuilder pathBuilder = new StringBuilder();
+ pathVarParamWriter.write(pathBuilder, new String[] {"a 20bc"});
+ Assert.assertEquals("a%2020bc", pathBuilder.toString());
+ }
+
+ @Test
+ public void writePathWithPercentage() throws Exception {
+ PathVarParamWriter pathVarParamWriter = createPathVarParamWriter();
+ StringBuilder pathBuilder = new StringBuilder();
+ pathBuilder.append("/api/");
+ pathVarParamWriter.write(pathBuilder, new String[] {"a%%bc"});
+ Assert.assertEquals("/api/a%25%25bc", pathBuilder.toString());
+ }
+
+ @Test
+ public void writeIntegerParam() throws Exception {
+ PathVarParamWriter pathVarParamWriter = createPathVarParamWriter();
+ StringBuilder pathBuilder = new StringBuilder();
+ pathVarParamWriter.write(pathBuilder, new Integer[] {12});
+ Assert.assertEquals("12", pathBuilder.toString());
+ }
+
+ private PathVarParamWriter createPathVarParamWriter() {
+ RestParam restParam = new MockUp<RestParam>() {
+ @Mock
+ Object getValue(Object[] args) {
+ return args[0];
+ }
+ }.getMockInstance();
+ return new PathVarParamWriter(restParam);
+ }
+}
\ No newline at end of file
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 6fc0c8c75..98996ee70 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
@@ -26,7 +26,6 @@
import org.junit.Test;
import org.junit.rules.ExpectedException;
-
public class TestHttpUtils {
@Rule
public ExpectedException expectedException = ExpectedException.none();
@@ -87,6 +86,13 @@ public void uriEncode_failed() {
HttpUtils.uriEncodePath(":");
}
+ @Test
+ public void uriEncode_plus() {
+ String encoded = HttpUtils.uriEncodePath("a+b");
+ Assert.assertEquals("a+b", encoded);
+ Assert.assertEquals("a+b", HttpUtils.uriDecodePath(encoded));
+ }
+
@Test
public void uriDecode_failed() {
expectedException.expect(IllegalArgumentException.class);
diff --git
a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
index 10868018c..32f1764d4 100644
---
a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
+++
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
@@ -29,9 +29,9 @@
import org.apache.servicecomb.it.testcase.base.TestDataTypeJaxrs;
import org.apache.servicecomb.it.testcase.base.TestDataTypePojo;
import org.apache.servicecomb.it.testcase.base.TestDataTypeSpringmvc;
+import org.apache.servicecomb.it.testcase.base.TestParamCodec;
import org.apache.servicecomb.it.testcase.support.ProducerDevMode;
-
public class ConsumerMain {
private static ResultPrinter resultPrinter = new ResultPrinter();
@@ -109,6 +109,7 @@ private static void testStandalone() throws Throwable {
testDataType();
ITJUnitUtils.runWithHighwayAndRest(TestTrace.class);
ITJUnitUtils.run(TestTraceEdge.class);
+ ITJUnitUtils.runWithHighwayAndRest(TestParamCodec.class);
ITJUnitUtils.getParents().pop();
deploys.getBaseProducer().stop();
diff --git
a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/base/TestParamCodec.java
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/base/TestParamCodec.java
new file mode 100644
index 000000000..d6ec50cc1
--- /dev/null
+++
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/base/TestParamCodec.java
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.servicecomb.it.testcase.base;
+
+import static org.junit.Assert.assertEquals;
+
+import org.apache.servicecomb.it.Consumers;
+import org.apache.servicecomb.it.junit.ITJUnitUtils;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class TestParamCodec {
+ interface ParamCodecSchemaIntf {
+ String spaceCharCodec(String pathVal, String q);
+ }
+
+ static Consumers<ParamCodecSchemaIntf> consumers = new
Consumers<>("paramCodec", ParamCodecSchemaIntf.class);
+
+ @BeforeClass
+ public static void classSetup() {
+ consumers.init(ITJUnitUtils.getTransport());
+ }
+
+ @Test
+ public void spaceCharEncode_intf() {
+ String paramString = "a%2B+%20b%% %20c";
+ String result = consumers.getIntf().spaceCharCodec(paramString,
paramString);
+ assertEquals(paramString + " +%20%% " + paramString + " true", result);
+ }
+
+ @Test
+ public void spaceCharEncode_rt() {
+ String paramString = "a%2B+%20b%% %20c";
+ String result = consumers.getSCBRestTemplate()
+ .getForObject("/spaceCharCodec/" + paramString + "?q=" + paramString,
String.class);
+ assertEquals(paramString + " +%20%% " + paramString + " true", result);
+ }
+}
diff --git
a/integration-tests/it-producer/src/main/java/org/apache/servicecomb/it/schema/ParamCodecSchema.java
b/integration-tests/it-producer/src/main/java/org/apache/servicecomb/it/schema/ParamCodecSchema.java
new file mode 100644
index 000000000..2620ecb54
--- /dev/null
+++
b/integration-tests/it-producer/src/main/java/org/apache/servicecomb/it/schema/ParamCodecSchema.java
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.servicecomb.it.schema;
+
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+
+import org.apache.servicecomb.provider.rest.common.RestSchema;
+
+@RestSchema(schemaId = "paramCodec")
+@Path("/base/v1/paramCodec")
+public class ParamCodecSchema {
+ /**
+ * Test path param and query param encode&decode
+ */
+ @Path("spaceCharCodec/{pathVal}")
+ @GET
+ public String spaceCharCodec(@PathParam("pathVal") String pathVal,
@QueryParam("q") String q) {
+ String expectedParamString = "a%2B+%20b%% %20c";
+ return pathVal + " +%20%% " + q + " " +
(expectedParamString.equals(pathVal) && expectedParamString.equals(q));
+ }
+}
----------------------------------------------------------------
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]
> Path param is not encoded and decoded correctly
> -----------------------------------------------
>
> Key: SCB-886
> URL: https://issues.apache.org/jira/browse/SCB-886
> Project: Apache ServiceComb
> Issue Type: Bug
> Components: Java-Chassis
> Reporter: YaoHaishi
> Assignee: YaoHaishi
> Priority: Major
>
> We find some problems:
> # for provider side, "%20" and "+" are both decoded to space char " ". But
> for path param, only "%20" should be decoded to space, and "+" should not be
> converted. See discussion in [QueryStringDecoder does not decode correctly
> path part with '+' (plus) sign in
> it|https://github.com/netty/netty/issues/6745]
> # for consumer side, currently path param is not encoded.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
