You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by GitBox <gi...@apache.org> on 2020/02/07 06:50:14 UTC
[GitHub] [servicecomb-java-chassis] GuoYL123 opened a new pull request
#1561: [SCB-1748] support colon
GuoYL123 opened a new pull request #1561: [SCB-1748] support colon
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561
Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/SCB) filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes.
- [ ] Each commit in the pull request should have a meaningful subject line and body.
- [ ] Format the pull request title like `[SCB-XXX] Fixes bug in ApproximateQuantiles`, where you replace `SCB-XXX` with the appropriate JIRA issue.
- [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [ ] Run `mvn clean install -Pit` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
---
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] liubao68 commented on a change in pull
request #1561: [SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
liubao68 commented on a change in pull request #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#discussion_r377999033
##########
File path: foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/http/HttpUtils.java
##########
@@ -94,6 +97,14 @@ public static String encodePathParam(String pathParam) {
return UrlEscapers.urlPathSegmentEscaper().escape(pathParam);
}
+ public static String decodePathParam(String pathParam) throws UnsupportedEncodingException {
+ String res = uriDecodePath(pathParam);
+ if (StringUtils.isEmpty(res)) {
+ return URLDecoder.decode(pathParam, "UTF-8");
+ }
+ return res;
+ }
+
public static String uriDecodePath(String path) {
if (path == null) {
Review comment:
This function seems not correct. According to the java docs, URL(path) will not parse the path, users should use
```new URI(null, null, path, null);``` to parse the parse
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] liubao68 commented on a change in pull
request #1561: [SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
liubao68 commented on a change in pull request #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#discussion_r377998726
##########
File path: foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/http/HttpUtils.java
##########
@@ -94,6 +97,14 @@ public static String encodePathParam(String pathParam) {
return UrlEscapers.urlPathSegmentEscaper().escape(pathParam);
}
+ public static String decodePathParam(String pathParam) throws UnsupportedEncodingException {
+ String res = uriDecodePath(pathParam);
+ if (StringUtils.isEmpty(res)) {
+ return URLDecoder.decode(pathParam, "UTF-8");
Review comment:
Is ``` URLDecoder.decode(pathParam, "UTF-8");``` is enough? I think it is enough to use URLEncoder and URLDecoder to encode/decode path segments, and use URI to encode/decode path.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] liubao68 commented on a change in pull
request #1561: [SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
liubao68 commented on a change in pull request #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#discussion_r376267878
##########
File path: common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/PathProcessorCreator.java
##########
@@ -52,6 +54,9 @@ public Object getValue(HttpServletRequest request) {
if (value == null) {
return null;
}
+ if (value.contains(":")) {
+ return convertValue(URLDecoder.decode(value, "UTF-8"), targetType);
Review comment:
I think this modification is not correct. You can add a test case for HttpUtils, like
```
@Test
public void uriEncode_sss() {
String encoded = HttpUtils.uriEncodePath("a:b");
Assert.assertEquals("a:b", encoded);
Assert.assertEquals("a:b", HttpUtils.uriDecodePath(encoded));
}
```
And make this case pass.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] GuoYL123 removed a comment on issue
#1561: [SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
GuoYL123 removed a comment on issue #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#issuecomment-583260868
WIP
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] liubao68 merged pull request #1561:
[SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
liubao68 merged pull request #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] coveralls edited a comment on issue
#1561: [SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#issuecomment-583273345
[](https://coveralls.io/builds/28680194)
Coverage increased (+0.007%) to 84.955% when pulling **f324478374a65502ee4280efe0fdb77f14e20dc2 on GuoYL123:maohao** into **ca69ceeec7a6eae824ad749dba15e8a6b9389ba3 on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] liubao68 commented on a change in pull
request #1561: [SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
liubao68 commented on a change in pull request #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#discussion_r377997205
##########
File path: common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/PathProcessorCreator.java
##########
@@ -17,7 +17,9 @@
package org.apache.servicecomb.common.rest.codec.param;
+import java.io.UnsupportedEncodingException;
import java.lang.reflect.Type;
+import java.net.URLDecoder;
Review comment:
remove unused import
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] coveralls edited a comment on issue
#1561: [SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#issuecomment-583273345
[](https://coveralls.io/builds/28621884)
Coverage increased (+0.003%) to 85.258% when pulling **bcdbfc4668ca42addfba988bde611a8efc67d752 on GuoYL123:maohao** into **4b2ac34d24e17745e6ce4f5a36e7bd89b0a96c94 on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] GuoYL123 commented on a change in pull
request #1561: [SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
GuoYL123 commented on a change in pull request #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#discussion_r378034428
##########
File path: foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/http/HttpUtils.java
##########
@@ -94,6 +97,14 @@ public static String encodePathParam(String pathParam) {
return UrlEscapers.urlPathSegmentEscaper().escape(pathParam);
}
+ public static String decodePathParam(String pathParam) throws UnsupportedEncodingException {
+ String res = uriDecodePath(pathParam);
+ if (StringUtils.isEmpty(res)) {
+ return URLDecoder.decode(pathParam, "UTF-8");
Review comment:
`URLDecoder.decode(pathParam, "UTF-8") `can not handle the '+' .
there is a ut show that : org.apache.servicecomb.common.rest.codec.param.TestPathProcessor#testGetPlus.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] GuoYL123 commented on a change in pull
request #1561: [SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
GuoYL123 commented on a change in pull request #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#discussion_r376846160
##########
File path: common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/PathProcessorCreator.java
##########
@@ -52,6 +54,9 @@ public Object getValue(HttpServletRequest request) {
if (value == null) {
return null;
}
+ if (value.contains(":")) {
+ return convertValue(URLDecoder.decode(value, "UTF-8"), targetType);
Review comment:
Add decodePathParam method.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] liubao68 commented on a change in pull
request #1561: [SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
liubao68 commented on a change in pull request #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#discussion_r376302692
##########
File path: common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/PathProcessorCreator.java
##########
@@ -52,6 +54,9 @@ public Object getValue(HttpServletRequest request) {
if (value == null) {
return null;
}
+ if (value.contains(":")) {
+ return convertValue(URLDecoder.decode(value, "UTF-8"), targetType);
Review comment:
Maybe this test case is not correct. Checked the [RFC](https://www.ietf.org/rfc/rfc2396.txt)
```
3.3. Path Component
The path component contains data, specific to the authority (or the
scheme if there is no authority component), identifying the resource
within the scope of that scheme and authority.
path = [ abs_path | opaque_part ]
path_segments = segment *( "/" segment )
segment = *pchar *( ";" param )
param = *pchar
pchar = unreserved | escaped |
":" | "@" | "&" | "=" | "+" | "$" | ","
The path may consist of a sequence of path segments separated by a
single slash "/" character. Within a path segment, the characters
"/", ";", "=", and "?" are reserved. Each path segment may include a
sequence of parameters, indicated by the semicolon ";" character.
The parameters are not significant to the parsing of relative
references.
```
you can also query @wujimin for the reason of the test case.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] GuoYL123 commented on a change in pull
request #1561: [SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
GuoYL123 commented on a change in pull request #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#discussion_r378035068
##########
File path: foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/http/HttpUtils.java
##########
@@ -94,6 +97,14 @@ public static String encodePathParam(String pathParam) {
return UrlEscapers.urlPathSegmentEscaper().escape(pathParam);
}
+ public static String decodePathParam(String pathParam) throws UnsupportedEncodingException {
+ String res = uriDecodePath(pathParam);
+ if (StringUtils.isEmpty(res)) {
+ return URLDecoder.decode(pathParam, "UTF-8");
+ }
+ return res;
+ }
+
public static String uriDecodePath(String path) {
if (path == null) {
Review comment:
use `new URI(null, null, path, null);` will cause ut failed:
org.apache.servicecomb.common.rest.codec.param.TestPathProcessor#testGetSpaceEncoded
`new URI(null, null, path, null);` can not convert '%20' to ' '
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] coveralls commented on issue #1561:
[SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
coveralls commented on issue #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#issuecomment-583273345
[](https://coveralls.io/builds/28584089)
Coverage decreased (-0.008%) to 85.247% when pulling **5c1733733facffe4a4c78d0d90e29d4bb43f0ae7 on GuoYL123:maohao** into **4b2ac34d24e17745e6ce4f5a36e7bd89b0a96c94 on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] coveralls edited a comment on issue
#1561: [SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#issuecomment-583273345
[](https://coveralls.io/builds/28584305)
Coverage decreased (-0.005%) to 85.25% when pulling **5c1733733facffe4a4c78d0d90e29d4bb43f0ae7 on GuoYL123:maohao** into **4b2ac34d24e17745e6ce4f5a36e7bd89b0a96c94 on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] GuoYL123 commented on issue #1561:
[SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
GuoYL123 commented on issue #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#issuecomment-583260868
WIP
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] coveralls edited a comment on issue
#1561: [SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#issuecomment-583273345
[](https://coveralls.io/builds/28682428)
Coverage decreased (-0.003%) to 84.945% when pulling **d12e34122c0e8723df67c802eaf618360cdf7760 on GuoYL123:maohao** into **ca69ceeec7a6eae824ad749dba15e8a6b9389ba3 on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] GuoYL123 commented on a change in pull
request #1561: [SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
GuoYL123 commented on a change in pull request #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#discussion_r378035293
##########
File path: common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/PathProcessorCreator.java
##########
@@ -17,7 +17,9 @@
package org.apache.servicecomb.common.rest.codec.param;
+import java.io.UnsupportedEncodingException;
import java.lang.reflect.Type;
+import java.net.URLDecoder;
Review comment:
done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] liubao68 commented on a change in pull
request #1561: [SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
liubao68 commented on a change in pull request #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#discussion_r376305439
##########
File path: common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/PathProcessorCreator.java
##########
@@ -52,6 +54,9 @@ public Object getValue(HttpServletRequest request) {
if (value == null) {
return null;
}
+ if (value.contains(":")) {
+ return convertValue(URLDecoder.decode(value, "UTF-8"), targetType);
Review comment:
I checked the code again. May be "encodePathParam" and "decodePathParam" should be used here. You need check details usage of "uriEncodePath" and "uriDecodePath". They are different.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] GuoYL123 commented on issue #1561:
[SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
GuoYL123 commented on issue #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#issuecomment-585144747
Thanks for the suggestion .And I found ` StringUtils.uriDecode(value, StandardCharsets.UTF_8)` can resolve the problem.
Maybe `HttpUtils.uriDecodePath` is not correct , but i can avoid to touch it. :)
Should I delete this unit test?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] coveralls edited a comment on issue
#1561: [SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#issuecomment-583273345
[](https://coveralls.io/builds/28682420)
Coverage increased (+0.006%) to 84.953% when pulling **d12e34122c0e8723df67c802eaf618360cdf7760 on GuoYL123:maohao** into **ca69ceeec7a6eae824ad749dba15e8a6b9389ba3 on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] liubao68 commented on issue #1561:
[SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
liubao68 commented on issue #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#issuecomment-585186805
```org.springframework.web.util.UriUtils``` has very nice api design. We can refer this and use the correct one in the right place. ```HttpUtils``` can disign it's API like this class.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] liubao68 commented on issue #1561:
[SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
liubao68 commented on issue #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#issuecomment-583287954
[This](https://stackoverflow.com/questions/724043/http-url-address-encoding-in-java) can be a reference
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] liubao68 commented on issue #1561:
[SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
liubao68 commented on issue #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#issuecomment-585060007
I think the current modification is not correct, need more analyse. For reference, you can look at
```org.springframework.web.util.UriUtils```
and This test case seems not correct
```
@Test
public void uriDecode_failed() {
expectedException.expect(IllegalArgumentException.class);
expectedException
.expectMessage(Matchers.is("uriDecode failed, path=\":\"."));
expectedException.expectCause(Matchers.instanceOf(URISyntaxException.class));
HttpUtils.uriDecodePath(":");
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] liubao68 commented on a change in pull
request #1561: [SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
liubao68 commented on a change in pull request #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#discussion_r376302692
##########
File path: common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/PathProcessorCreator.java
##########
@@ -52,6 +54,9 @@ public Object getValue(HttpServletRequest request) {
if (value == null) {
return null;
}
+ if (value.contains(":")) {
+ return convertValue(URLDecoder.decode(value, "UTF-8"), targetType);
Review comment:
Maybe this test case is not correct. Checked the [RFC](https://www.ietf.org/rfc/rfc2396.txt)
```
3.3. Path Component
The path component contains data, specific to the authority (or the
scheme if there is no authority component), identifying the resource
within the scope of that scheme and authority.
path = [ abs_path | opaque_part ]
path_segments = segment *( "/" segment )
segment = *pchar *( ";" param )
param = *pchar
pchar = unreserved | escaped |
":" | "@" | "&" | "=" | "+" | "$" | ","
The path may consist of a sequence of path segments separated by a
single slash "/" character. Within a path segment, the characters
"/", ";", "=", and "?" are reserved. Each path segment may include a
sequence of parameters, indicated by the semicolon ";" character.
The parameters are not significant to the parsing of relative
references.
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-java-chassis] GuoYL123 commented on a change in pull
request #1561: [SCB-1748] support colon in path value
Posted by GitBox <gi...@apache.org>.
GuoYL123 commented on a change in pull request #1561: [SCB-1748] support colon in path value
URL: https://github.com/apache/servicecomb-java-chassis/pull/1561#discussion_r376275152
##########
File path: common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/PathProcessorCreator.java
##########
@@ -52,6 +54,9 @@ public Object getValue(HttpServletRequest request) {
if (value == null) {
return null;
}
+ if (value.contains(":")) {
+ return convertValue(URLDecoder.decode(value, "UTF-8"), targetType);
Review comment:
I notic there is a case org.apache.servicecomb.foundation.common.http.TestHttpUtils#uriDecode_failed
to show a error when the param of HttpUtils.uriDecodePath contain ":".
I think modify the HttpUtils.uriDecodePat will change the previous logic.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services