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

commit 20889e489279ac0eaa176dd85da8615a8c98435e
Author: yaohaishi <[email protected]>
AuthorDate: Mon Sep 3 20:31:51 2018 +0800

    [SCB-886] fix path param encode and decode problem
---
 .../rest/codec/param/PathProcessorCreator.java     |  4 +-
 .../rest/definition/path/PathVarParamWriter.java   |  5 +-
 .../common/rest/codec/param/TestPathProcessor.java | 22 +++++--
 .../definition/path/PathVarParamWriterTest.java    | 73 ++++++++++++++++++++++
 .../foundation/common/http/TestHttpUtils.java      |  8 ++-
 5 files changed, 104 insertions(+), 8 deletions(-)

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 b69db9f..dbf459c 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 class PathProcessorCreator implements 
ParamValueProcessorCreator {
       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 95659e8..95f4016 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 class PathVarParamWriter extends 
AbstractUrlParamWriter {
 
   @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 0fd25ae..19cd51f 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,14 +77,30 @@ public class TestPathProcessor {
   }
 
   @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);
     Assert.assertEquals("path", processor.getProcessorType());
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 0000000..664c406
--- /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 6fc0c8c..98996ee 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.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 
-
 public class TestHttpUtils {
   @Rule
   public ExpectedException expectedException = ExpectedException.none();
@@ -88,6 +87,13 @@ public class TestHttpUtils {
   }
 
   @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);
     expectedException

Reply via email to