You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Ka-Hing Cheung <no...@github.com> on 2015/05/28 01:15:45 UTC
[jclouds] tighten the isUrlEncoded check (#755)
ideally we shouldn't need this function and instead never double
encode strings, but auditing for that is beyond what I have time
for. currently, putBlob(" ") and putBlob("%20") behave the same
way which is arguably incorrect
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds/pull/755
-- Commit Summary --
* tighten the isUrlEncoded check
-- File Changes --
M core/src/main/java/org/jclouds/util/Strings2.java (29)
M core/src/test/java/org/jclouds/util/Strings2Test.java (4)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/755.patch
https://github.com/jclouds/jclouds/pull/755.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/755
Re: [jclouds] tighten the isUrlEncoded check (#755)
Posted by Andrew Gaul <no...@github.com>.
Pushed to master as 82ab88d5896a6628f3fbb42a3c4bce39ce843fa1.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/755#issuecomment-106114624
Re: [jclouds] tighten the isUrlEncoded check (#755)
Posted by Andrew Gaul <no...@github.com>.
Reverted in c22afc5a55db1539d2e796f7244f9d2b3da57b5a.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/755#issuecomment-106130839
Re: [jclouds] tighten the isUrlEncoded check (#755)
Posted by Andrew Gaul <no...@github.com>.
Closed #755.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/755#event-315489047
Re: [jclouds] tighten the isUrlEncoded check (#755)
Posted by Ka-Hing Cheung <no...@github.com>.
Closed #755.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/755#event-317495999
Re: [jclouds] tighten the isUrlEncoded check (#755)
Posted by Andrew Gaul <no...@github.com>.
>
> public static boolean isUrlEncoded(String in) {
> - return URL_ENCODED_PATTERN.matcher(in).matches();
> + if (!URL_VALID_PATTERN.matcher(in).matches()) {
> + return false;
> + }
> +
> + // ensure that all % are followed by 2 hexadecimal characters
> + int percentIdx = 0;
> + while ((percentIdx = in.indexOf('%', percentIdx)) != -1) {
> + if (percentIdx + 2 >= in.length()) {
> + return false;
> + }
> + if (!isHexadecimal(in.charAt(percentIdx + 1)) ||
> + !isHexadecimal(in.charAt(percentIdx + 2))) {
> + System.err.println("% is followed by non-alphanumeric " + in.substring(percentIdx, percentIdx + 2));
Remove -- we cannot spam `System.err`. Optionally introduce a logger instead.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/755/files#r31191727
Re: [jclouds] tighten the isUrlEncoded check (#755)
Posted by Ka-Hing Cheung <no...@github.com>.
giving up, the code is buggy but there are many dependencies on its buggy behaviors
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/755#issuecomment-106895525
Re: [jclouds] tighten the isUrlEncoded check (#755)
Posted by Andrew Gaul <no...@github.com>.
Reopened #755.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/755#event-315503174
Re: [jclouds] tighten the isUrlEncoded check (#755)
Posted by Andrew Gaul <no...@github.com>.
This commit causes failures in core for me:
```
BindMapToStringPayloadTest.testDecodes:76 expected [{"changePassword":{"adminPass":"foo"}}] but found [%7B"changePassword":%7B"adminPass":"foo"%7D%7D]
HttpRequestTest.testAddBase64AndUrlEncodedQueryParams:78 expected [GET http://goo.com:443?header1=ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789%2B/%3D&header2=ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789%2B/%3D HTTP/1.1] but found [GET http://goo.com:443?header1=ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789%20/%3D&header2=ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/= HTTP/1.1]
HttpRequestTest.testEncodesOnlyOnce:55 expected [{method=GET, endpoint=http://goo.com:443?header=hello%3F, headers={}}] but found [{method=GET, endpoint=http://goo.com:443?header=hello?, headers={}}]
RestAnnotationProcessorTest.testCreateGetRequest:2133 expected [/qu%3Fstion] but found [/qu]
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/755#issuecomment-106120072
Re: [jclouds] tighten the isUrlEncoded check (#755)
Posted by Andrew Gaul <no...@github.com>.
More reports of URL encoding issues at [JCLOUDS-217](https://issues.apache.org/jira/browse/JCLOUDS-217).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/755#issuecomment-106109575