You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Diwaker Gupta <no...@github.com> on 2013/08/01 19:04:54 UTC

[jclouds] Only decode strings that are actually encoded. (#82)

References JCLOUDS-217
References JCLOUDS-200
You can merge this Pull Request by running:

  git pull https://github.com/maginatics/jclouds feature/urlencode

Or you can view, comment on it, or merge it online at:

  https://github.com/jclouds/jclouds/pull/82

-- Commit Summary --

  * Only decode strings that are actually encoded.

-- File Changes --

    M core/src/main/java/org/jclouds/util/Strings2.java (7)
    M core/src/test/java/org/jclouds/util/Strings2Test.java (7)

-- Patch Links --

https://github.com/jclouds/jclouds/pull/82.patch
https://github.com/jclouds/jclouds/pull/82.diff


Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Andrew Phillips <no...@github.com>.
> I believe this should close JCLOUDS-200

Could you resubmit the change in question, then? Or should we simply be able to re-apply the original diff?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-23411244

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Andrew Phillips <no...@github.com>.
> jclouds » jclouds #257 UNSTABLE

These look like "real" failures, unfortunately:

https://buildhive.cloudbees.com/job/jclouds/job/jclouds/257/testReport/

@diwakergupta can you take a look?

Also, does this relate to https://github.com/jclouds/jclouds/pull/78 in any way? Would the idea be to change addQueryParam so it only decodes _encoded_ values and leaves others alone? In that case, could we pass the signature without having to encode it first?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21956864

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #541](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/541/) UNSTABLE
Looks like there's a problem with this pull request

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21957020

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #644](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/644/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-23371452

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Andrew Phillips <no...@github.com>.
> As a newcomer to the code base, things like '+' silently changing to space if you add another parameter just 
> seems wrong

It seems wrong to me too, but it was written by people smarter than me, so is probably there for a reason - especially since it's rather weird. I'd feel better if we knew more about _why_ this code works as it does and whether that reason is still valid before just ripping it out - especially on the evening of a release.

@adriancole Any comments you can make about the URL encoding logic here?

Glad to hear live blobstore tests don't seem to be affected. Would be nice to see some compute data, though.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21978467

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Andrew Phillips <no...@github.com>.
@diwakergupta: could you squash and rebase?

Thanks!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-23174742

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #183](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/183/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-23371876

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Andrew Gaul <no...@github.com>.
The number of regressions and amount churn around this set of pull requests makes me uncomfortable.  We should either find a way forward which minimizes risk or revert all the commits until after 1.6.2.  This release includes many fixes which users and we should not risk DOA release on a minor feature like configurable timeout signed URLs for AWS-S3.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21983404

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Diwaker Gupta <no...@github.com>.
@demobox understand the concern about how close this is to a release but do want to point out that without this change, url encoding is plain broken and some features (like S3 temp signed URLs) can't work without it. Swift seems to work fine because their signatures are not required to be URL encoded (they use base16 whereas AWS uses base64). For instance, try creating `Uris` using `UriBuilder` and pass in something like `http://foo.com/?bar+baz` -- because of the current decode behavior, we'll end up storing the endpoint as `http://foo.com/?bar baz`. Just my 2c.


---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21979224

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Andrew Phillips <no...@github.com>.
Thanks, @adriancole!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-22145663

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Diwaker Gupta <no...@github.com>.
AFAICT, the last two failures have nothing to do with my change.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21962202

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Adrian Cole <no...@github.com>.
incidentally, I just bumped into this, that summarizes the context as well http://blog.lunatech.com/2009/02/03/what-every-web-developer-must-know-about-url-encoding

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-22144728

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Diwaker Gupta <no...@github.com>.
Here's a simple example that demonstrates the problem:

```
--- core/src/test/java/org/jclouds/http/UrisTest.java
+++ core/src/test/java/org/jclouds/http/UrisTest.java
@@ -66,7 +66,7 @@ public class UrisTest {
    @DataProvider(name = "strings")
    public Object[][] createData() {
       return new Object[][] { { "normal" }, { "sp ace" }, { "qu?stion" }, { "unic₪de" }, { "path/foo" }, { "colon:" },
-            { "asteri*k" }, { "quote\"" }, { "great<r" }, { "lesst>en" }, { "p|pe" } };
+            { "asteri*k" }, { "quote\"" }, { "great<r" }, { "lesst>en" }, { "p|pe" }, {"pl+s"}};
    }
 
    @Test(dataProvider = "strings")

# Run testReplaceQueryIsEncoded, fails with:
java.lang.AssertionError: 
Expected :/redirect?foo=pl+s
Actual   :/redirect?foo=pl s
```

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21981592

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #169](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/169/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-23221184

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Andrew Phillips <no...@github.com>.
@diwakergupta: Actually, before merging: could you either rename HttpRequestTest.testAddingBase64EncodedQueryParamCausingPlusToUrlEncodedSpaceConversion (which now no longer checks for that), or remove it if it does not check anything that is not covered by the "standard" testAddBase64AndUrlEncodedQueryParams test? Thanks!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-23305589

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Diwaker Gupta <no...@github.com>.
@demobox I'm happy to provide evidence if there are specific concerns. Here's a simple experiment for you: construct a `HttpRequest` and specify an endpoint such as `http://localhost/foo?bar+baz`. Now invoke this `HttpRequest` with an injected `HttpClient` (which by default will run via `JavaUrlHttpCommandExecutorService`). Now check your web server logs and you'll see that the URL that is actually requested is `http://localhost/foo?bar baz`.

That said, IMO in this case the code speaks for itself. Here's `Strings2.urlEncode`:

```
   public static String urlEncode(String in, Iterable<Character> skipEncode) {
      if (isUrlEncoded(in))
         return in;
  ...
```

If you saw that and were writing a corresponding `urlDecode`, would you not check for `!isUrlEncoded`?

Anyways, I'll let you and @andrewgaul figure out what to do with this and the other patches. Feel free to revert if you judge that to be the best course of action. @andrewgaul I'm curious though, why do you think this patch makes the release DOA?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21988085

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Adrian Cole <no...@github.com>.
special encoding logic like "+" etc were often introduced to acomodate request signing behavior for clouds like azure, cloudstack, or s3.  If there are test failures in isolation, but not in all (even lab) provider tests, fair game to remove it.  Just note this when next run of live tests occur, so that you know if you need to do it again local to some provider.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-22038640

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Diwaker Gupta <no...@github.com>.
@demobox done

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-23220609

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Andrew Phillips <no...@github.com>.
Closed #82.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Diwaker Gupta <no...@github.com>.
@demobox I really don't have context on why the current behavior is what it is. As a newcomer to the code base, things like '+' silently changing to space if you add another parameter just seems wrong. Yes we've changed the decoding but it is now _correct_ (easy to verify against any number of URL encoders/decoders out there). As for live tests, I've run the blobstore live tests against several different blobstores without any issues. Can't comment on all components obviously.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21978148

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Andrew Phillips <no...@github.com>.
I'm curious about the changes in HttpRequestTest. Yes, we've got the tests to pass again, but we've changed the behaviour of the decoding now. Do we have some live tests to indicate that this won't cause any problems? The behaviour as it was was most likely added for a reason...

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21977728

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Andrew Gaul <no...@github.com>.
Reopened #82.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Andrew Phillips <no...@github.com>.
> I need this particular patch in 1.6.x so not including any cleanup work with it.

Sure. Just wanted to understand how things fit together, that's all ;-)

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21958948

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #257](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/257/) UNSTABLE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21956366

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #555](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/555/) UNSTABLE
Looks like there's a problem with this pull request

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21972689

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #629](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/629/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-23221173

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Diwaker Gupta <no...@github.com>.
Java 6 beehive passed, the Java 7 failure is unrelated. Would be great if
we can merge and backport to 1.6.x.


On Thu, Aug 1, 2013 at 2:55 PM, CloudBees pull request builder plugin <
notifications@github.com> wrote:

> jclouds-pull-requests #100<https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/100/>SUCCESS
> This pull request looks good
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/jclouds/jclouds/pull/82#issuecomment-21972776>
> .
>



-- 
http://floatingsun.net

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21972905

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #86](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/86/) UNSTABLE
Looks like there's a problem with this pull request

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21956893

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Andrew Phillips <no...@github.com>.
> but do want to point out that without this change, url encoding is plain broken

TL;DR: The current behaviour urlEncode may look broken to you and may, but it's clearly deliberate and not a coding error. Since someone with lots of HTTP experience went to the trouble to add it, I'm not in favour of ripping it out without understand why it's there or _lots_ of evidence that it's no longer required.

@diwakergupta Of course I see where you're coming from. It _seems_ broken to me too. But on the evidence (all tests passing and, until the signature feature request, no JIRA issues or user group questions) it's working pretty well. I know it's not the "standard" URI encoding that you and I might expect, but without understanding the reason for the "non-standard" encoding I'm reluctant to rip it out.

It's not as though there's an obvious coding bug in there - someone with a _lot_ of HTTP experience has gone to some lengths to put this behaviour in there deliberately.

As regards the test case failure: again, I don't know whether the `+` case is omitted by accident or deliberately. I also don't know what the expected result _should_ be in that case. Perhaps the test case would be written as
```
assertEquals(uriBuilder("/redirect").addQuery("foo", key).toString(), "/redirect?foo=" + key,replace('+', ' '));
```
to indicate the _intended_ behaviour of `+` being replaced by ` `?

Back to the immediate use case at hand: can get JCLOUDS-200 working without this change by encoding the URL first, or is it broken even then? If it can work after https://github.com/jclouds/jclouds/pull/78, I'd prefer to leave things as-is for 1.6.2-incubating and get more background on this before hopefully cleaning up the encoding for 1.6.3-incubating and 1.7.0.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21982414

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Adrian Cole <no...@github.com>.
looks like some in question were put in place to help signed urls work in different browsers

https://bitbucket.org/jmurty/jets3t/src/4e5fc1ce16c380d555734ac75803e89f77040708/src/org/jets3t/service/utils/RestUtils.java?at=default

Knowing above can help you come to a decision even if it breaks some folks.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-22038930

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #100](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/100/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21972776

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Andrew Gaul <no...@github.com>.
Closing and reopening retriggers CloudBees.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21968870

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Ignasi Barrera <no...@github.com>.
>I'd be happy to merge this into master and see what happens. As Adrian suggests, we can re-introduce the relevant logic as special-case behaviour for those providers where it turns out still to be required.

+1 to reintroduce it in the concrete providers that require the trick.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-23150541

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Andrew Phillips <no...@github.com>.
Based on @nacx and @andrewgaul's comments, I think the best course of action for now is:

* leave this PR open for now
* try to get hold of @adriancole to determine the origin of the "weird encoding". If it was added only for an S3 issue as @nacx's search suggests, then it should be easier to try to determine whether we need it or not, since @andrewgaul and @diwakergupta will be making plenty of connections against S3
* if we can't get hold of Adrian, potentially merge this for 1.7.0-SNAPSHOT and ask users to test the snapshot. That gives us a chance to revert if necessary without creating a DOA release.

I'm fine with leaving https://github.com/jclouds/jclouds/pull/78 in because it does not affect existing behaviour. Based on @diwakergupta's [comment](https://github.com/jclouds/jclouds/pull/82#issuecomment-21988773) I'm unclear on whether we can expect that feature to _work_, or not, but in the worst case 1.6.2-incubating will have a bug in JCLOUDS-200 that we can hopefully address in the next release

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-22005263

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Diwaker Gupta <no...@github.com>.
> can we get JCLOUDS-200 working without this change by encoding the URL first, or is it broken even then?

@demobox we can't -- it is broken because as soon as you hand a HTTP request that contains a '+' in the query string to jclouds `BaseHttpExecutorService`, it will mange the request and wrongly convert the '+' to a space before issuing the request. JCLOUDS-200 just fixes a similar problem on the request being _generated_ by jclouds.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21988773

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Diwaker Gupta <no...@github.com>.
@demobox is there way to "retrigger" this PR? The failure is unrelated to my change and all unit tests pass for me locally.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21965489

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Diwaker Gupta <no...@github.com>.
Thanks for the context @adriancole! That piece of code is several years old now and most browsers (including Chrome) do the right thing. Also IMO these generated URLs are typically consumed by other libraries and software and rarely directly by humans clicking (or pasting) a URL into a browser. Either way, the trade off is potentially supporting some allegedly broken behavior in some browsers vs. correctly encoding/decoding URIs.

Anyways, will defer to the jclouds devs to make the right choice.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-22041745

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Andrew Phillips <no...@github.com>.
> @@ -70,7 +70,6 @@ public void testAddFormParamAddsAnotherValue() {
>     // the following caused issues for the fgcp provider
>     // (see RequestAuthenticator#addQueryParamsToRequest)
>     // base64 symbols should be url encoded in query param
> -   // note that + ends up encoded as %20 (space), not %2B (plus)
>     public void testAddingBase64EncodedQueryParamCausingPlusToUrlEncodedSpaceConversion() {

See my main comment about the changes to HttpRequestTest. In fact, this test name isn't even valid any more because plus no _no longer_ becomes a "UrlEncodedSpace".

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82/files#r5544056

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Andrew Phillips <no...@github.com>.
Thanks, @adriancole! Now that we know more about the context, I'd be happy to merge this into master and see what happens. As Adrian suggests, we can re-introduce the relevant logic as special-case behaviour for those providers where it turns out still to be required.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-22043105

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Diwaker Gupta <no...@github.com>.
@demobox yes already looked at it, pushing a fix shortly. Yes it does relate to #78 and we can pass the signature without encoding it but that's an optimization and doesn't impact correctness. I can push that as a separate cleanup patch later. I need this particular patch in 1.6.x so not including any cleanup work with it.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21957622

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #340](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/340/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-23221407

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Andrew Gaul <no...@github.com>.
Closed #82.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Andrew Phillips <no...@github.com>.
Committed to [master](https://git-wip-us.apache.org/repos/asf?p=incubator-jclouds.git;a=commit;h=12f29fd8a9e50203acc12ccf794cf73c0abf5703).

@diwakergupta: does this unblock [JCLOUDS-200](https://issues.apache.org/jira/browse/JCLOUDS-200), or do we still need work for that..?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-23385231

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Andrew Phillips <no...@github.com>.
@diwakergupta Thanks! Just waiting for BuildHive and DEV@cloud...

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-23371168

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Ignasi Barrera <no...@github.com>.
After a google search I've only found this:
https://groups.google.com/forum/m/#!msg/jclouds-dev/hBzkDyT_XiY/mJXEq8l4fP4J

If you read Adrian's comment, it seems the encoding 'tricks' are intentional due to some S3 issue. I also agree with you @diwakergupta that the encoding might be broken, but I'd prefer to have some feedback from @adriancole before merging, so we can understand why current encoding works like and make sure we don't introduce regressions.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21990829

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Andrew Phillips <no...@github.com>.
Pending BuildHive, +1

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21955440

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #90](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/90/) FAILURE
Looks like there's a problem with this pull request

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21960402

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Diwaker Gupta <no...@github.com>.
@demobox I believe this should close JCLOUDS-200 but IIUC there was some concern that it might break other things, which remains to be seen.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-23389885

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by Andrew Phillips <no...@github.com>.
@nacx @andrewgaul Thoughts on merging this into 1.7.0-SNAPSHOT to see how this behaves? Or rather a 1.7.x iteration?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-23131767

Re: [jclouds] Only decode strings that are actually encoded. (#82)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #545](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/545/) FAILURE
Looks like there's a problem with this pull request

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/82#issuecomment-21960405