You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Timur Alperovich <no...@github.com> on 2015/08/27 22:57:31 UTC

[jclouds] Allow for strings with % characters in jclouds (#851)

Fixes handling of strings that may be encoded when passed to jclouds. Apache jclouds should not double-encode strings and should not guard against handling encoded strings. This patch makes a change ```jclouds/core``` to remove the check for encoded strings when calling ```Strings2.urlEncode()```. The changes to all of the other components are made to remove the reliance on this behavior. In jclouds blobstore, this further allows supporting strings with "%" characters -- a matter of a number of JIRA issues.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Percent encoding changes.
  * Fix encoding handling in S3.
  * Azure: Handle blobs with % characters correctly.
  * Do not use encoded strings in STS tests.
  * Fix a cloudstack test that encoded parameters.
  * Do not use encoded strings in EC2 tests.
  * Do not use encoded strings in SQS API tests.
  * Do not use encoded strings in AWS EC2 tests.
  * OpenStack Nova tests: do not pass encoded strings.

-- File Changes --

    M apis/cloudstack/src/test/java/org/jclouds/cloudstack/features/SSHKeyPairApiTest.java (6)
    M apis/ec2/src/main/java/org/jclouds/ec2/compute/EC2ComputeService.java (4)
    M apis/ec2/src/test/java/org/jclouds/ec2/features/AMIApiExpectTest.java (4)
    M apis/ec2/src/test/java/org/jclouds/ec2/features/AMIApiTest.java (56)
    M apis/ec2/src/test/java/org/jclouds/ec2/features/EC2ElasticBlockStoreApiExpectTest.java (19)
    M apis/ec2/src/test/java/org/jclouds/ec2/features/ElasticBlockStoreApiTest.java (24)
    M apis/ec2/src/test/java/org/jclouds/ec2/features/ElasticIPAddressApiTest.java (4)
    M apis/ec2/src/test/java/org/jclouds/ec2/features/InstanceApiTest.java (26)
    M apis/ec2/src/test/java/org/jclouds/ec2/features/KeyPairApiExpectTest.java (4)
    M apis/ec2/src/test/java/org/jclouds/ec2/features/SecurityGroupApiExpectTest.java (4)
    M apis/ec2/src/test/java/org/jclouds/ec2/features/SecurityGroupApiTest.java (12)
    M apis/ec2/src/test/java/org/jclouds/ec2/features/SubnetApiExpectTest.java (12)
    M apis/ec2/src/test/java/org/jclouds/ec2/features/WindowsApiExpectTest.java (4)
    M apis/ec2/src/test/java/org/jclouds/ec2/features/WindowsApiTest.java (16)
    M apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/features/ImageApiExpectTest.java (21)
    M apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/features/ServerApiExpectTest.java (14)
    M apis/s3/src/main/java/org/jclouds/s3/binders/BindAsHostPrefixIfConfigured.java (2)
    M apis/s3/src/main/java/org/jclouds/s3/xml/ListBucketHandler.java (11)
    M apis/sqs/src/test/java/org/jclouds/sqs/features/MessageApiExpectTest.java (70)
    M apis/sqs/src/test/java/org/jclouds/sqs/features/PermissionApiExpectTest.java (8)
    M apis/sqs/src/test/java/org/jclouds/sqs/features/QueueApiExpectTest.java (28)
    M apis/sts/src/test/java/org/jclouds/sts/STSApiExpectTest.java (24)
    M blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseContainerIntegrationTest.java (17)
    M core/src/main/java/org/jclouds/http/Uris.java (1)
    M core/src/main/java/org/jclouds/rest/binders/BindAsHostPrefix.java (5)
    M core/src/main/java/org/jclouds/util/Strings2.java (9)
    M core/src/test/java/org/jclouds/util/Strings2Test.java (6)
    M providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/features/AWSInstanceApiTest.java (2)
    M providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/features/AWSKeyPairApiTest.java (2)
    M providers/azureblob/src/main/java/org/jclouds/azureblob/xml/ContainerNameEnumerationResultsHandler.java (49)

-- Patch Links --

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

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Ignasi Barrera <no...@github.com>.
This is quite a sensible change that may break many clients.

You should also check all uses of the [SkipEncoding](https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/rest/annotations/SkipEncoding.java) annotation carefully and discuss if it is still needed. Then we need to make sure that the relevant live tests for the methods that are using it still succeed with your change.

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Ignasi Barrera <no...@github.com>.
I am personally uncomfortable with this patch, due to the huge impact it may have.

Also, I see the *path decode* has ben removed [here](https://github.com/jclouds/jclouds/pull/851/files#diff-22a8e223e58db063094e8c338d8c0b3eL147), but **not** in [this case](https://github.com/timuralp/jclouds/blob/percent-encoding/core/src/main/java/org/jclouds/http/Uris.java#L142). This seems not consistent and likely to produce a broken behaviour. Or I might be wrong, but if this is actually the case, then we definitely do not have a test suite that gives us the confidence I'd like to have to merge this kind of change.

Also, the latter `path` is called in several places (when building the URI based on the endpoint, for example). What would happen in that case? What happens to automatically generated endpoints we don't control? Many providers and APIs make use of the `@EndpointParam` and related annotations do dynamically compute the endpoint. How can we have an effective control on the encoding of the URLs in those cases?

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Andrew Gaul <no...@github.com>.
Do we have any other blockers to merging this?

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Andrew Phillips <no...@github.com>.
> @demobox good to go?

Looks good to me. If we communicate the change clearly in the next release, it should hopefully be pretty easy for users to figure out what is causing any new behaviour they're seeing.

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Ignasi Barrera <no...@github.com>.
I can help testing this, but remlving the SkipEncoding should be part of this PR, with all its implications. GCE and other providers make heavy use of it.
It shouldn't be merged until we are confident that all live tests are passing.

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Timur Alperovich <no...@github.com>.
I've been thinking of where the reliance on *not* encoding the string -- the crux of this change -- could have been successful. No obvious examples come to mind that would still function. For example, putting a key "%24" would actually put "$" -- the user would notice this when listing the bucket.

I think the biggest issue is that user's may need to update their code to *not* work around this issue.

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Timur Alperovich <no...@github.com>.
@nacx agreed that this may break compatibility with many consumers.

I'll look into the uses of SkipEncoding -- did not do so previously as it looked like the BlobStore providers did not rely on this. For the compute side, I will try to check some of the live tests (e.g. EC2), but I don't have credentials for a lot of providers.

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Andrew Gaul <no...@github.com>.
Given that five reporters filed issues, this is likely the most reported jclouds bug ever.  Relying on buggy and broken behavior ties us to jclouds internals like `RestAnnotationProcessor` which are long-term technical debt.  Instead of debating *if* we should merge an obvious improvement, we should discuss *how* to mitigate risk of merging a core patch.  Passing all integration tests on major providers should suffice.  This includes all blobstore, all rackspace, ec2, azure compute, and gce.

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Andrew Phillips <no...@github.com>.
> this further allows supporting strings with "%" characters -- a matter of a number of JIRA issues

Apart from the support for `%` characters, which I'd of course like to see, but for which I'm not aware of a _critical_ issue: could you give some more detail on the JIRA issues you're referring to?

I'm certainly not trying to defend the current behaviour as being "correct", but would like to get a better feeling for the concrete benefits of this, given that it's such a disruptive change.


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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Ignasi Barrera <no...@github.com>.
Having a deeper look I'd say I understand better the scope of the change and I'm a bit more comfortable with it. I plan to also run the GCE live tests early this week to give some more feedback.

Just to complete the PR and to be able to produce release notes that help users understand what would change for them, can you think about an example of something an end-user could be doing that would break with this change (assuming the current live tests keep passing)? Just to illustrate the different "don't do" once this is merged.

Thx for the detailed PR and commit messages, and your testing!

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Zack Shoylev <no...@github.com>.
@timuralp testBlobNameEscaping has three cases: "%20", " ", "%20 ", " %20"

Only "%20 " is failing on windows with the filesystem provider with this pr.

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Andrew Phillips <no...@github.com>.
> Four related/duplicated issues linked at JCLOUDS-217.

@andrewgaul: Thanks. As far as I can see, the tickets refer to two main issues, both related to blobstore:

1. supporting spaces
1. supporting `%`

The former especially seems like something that could have a lot of impact and be pretty annoying. Then again, the proposed change _definitely_ has a lot of impact - impact that goes beyond blobstore, too.

So in order to try to get a feel for the likely cost/benefit ratio of making a big change to core, it would help to understand "how bad" the two limitations above actually are in practice, how many users are impacted by them, and whether there are any workarounds.

Do we have any data on that?


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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Timur Alperovich <no...@github.com>.
I ran the EC2 live tests. There were some failures (10), but I observed those on master and on the percent-encoding branch. The vast majority of the tests does pass successfully. It appears that one reason for failures is that I did not setup the ssh key to be used with the EC2 instances and I'm not sure what jclouds expects in that regard -- did not investigate. I can try to figure it out if it may be of consequence.

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Zack Shoylev <no...@github.com>.
@timuralp It seems to pass tests on windows now. Thanks!

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Andrew Phillips <no...@github.com>.
> @@ -561,6 +562,30 @@ public void testDelimiterList() throws InterruptedException {
>        }
>     }
>  
> +   @DataProvider
> +   public Object[][] getBlobsToEscape() {
> +      ImmutableSet<String> testNames = ImmutableSet.of("%20", "%20 ", " %20", " ");
> +      Object[][] result = new Object[1][1];
> +      result[0][0] = testNames;
> +      return result;
> +   }
> +
> +   @Test(dataProvider = "getBlobsToEscape", groups = {"integration", "live"})
> +   public void testBlobNameEscaping(ImmutableSet<String> blobNames) throws InterruptedException {

[nit] `Set` instead of `ImmutableSet` here? We can take care of this during the merge, I think - no need to update the PR.

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Timur Alperovich <no...@github.com>.
@zack-shoylev thanks for the update. This is a little surprising to me. I'm going to setup a Windows dev environment to figure out what's going on.

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Zack Shoylev <no...@github.com>.
@andrewgaul I will have a look today. Does this affect the filesystem provider? I think it might also be problematic on windows.

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Timur Alperovich <no...@github.com>.
@zack-shoylev could you retry the Windows tests with the latest PR? I tried it on my Windows setup and there were no failures, but wanted to double check.

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Zack Shoylev <no...@github.com>.
I am having some issues in core while testing:

```
Failed tests:   testMapStringObjectWithAllValidValuesOneDeep(org.jclouds.json.JsonTest): expected:<{"string":"string","map":{"key":"value"},"list":["key","value"],"boolean":true,"number":1.0}> but was:<{"number":1.0,"boolean":true,"string":"string","list":["key","value"],"map":{"key":"value"}}>
  testAddDetailsAndPropagateOkWithValidRequestResponseWithSAXParseException(org.jclouds.http.functions.ParseSaxTest): expected:<request: GET http://foohost HTTP/1.1; response: HTTP/1.1 304 Not Modified; error at 1:1 in document systemId; cause: org.xml.sax.SAXParseException: foo> but was:<request: GET http://foohost HTTP/1.1; response: HTTP/1.1 304 Not Modified; error at 1:1 in document systemId; cause: org.xml.sax.SAXParseExceptionpublicId: publicId; systemId: systemId; lineNumber: 1; columnNumber: 1; foo>
  testCreatePostWithMethodBinderAndDefaults(org.jclouds.rest.internal.RestAnnotationProcessorTest): expected:<{"fooble":"data","rat":"atat"}> but was:<{"rat":"atat","fooble":"data"}>

Tests run: 734, Failures: 3, Errors: 0, Skipped: 0
```

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Andrew Gaul <no...@github.com>.
:+1: I prefer the component commits since we touch so many providers.

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Andrew Gaul <no...@github.com>.
Agreed that this pull request has a high likelihood of breaking some compute providers.  I think we should test this on the most popular ones, e.g., Azure, EC2, GCE, and OpenStack, then merge eagerly to give the lesser-used ones more opportunity to shake out bugs.  @nacx @zack-shoylev What do you think and could you help with some of this testing?

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Timur Alperovich <no...@github.com>.
@nacx the reason for the change in Uris.java is the current behavior of ```appendPath()```: decode the path and then call ```path()``` with the decoded string (possibly, prepending a "/" character). ```path()``` then, in turn, _also_ decodes the string (as you pointed out). That leads to a string that is decoded twice. I couldn't figure out the original intent of whoever introduced such behavior, but it doesn't seem right to me.

Here is an example that breaks with this today: appending a path of the form "%24$" (%24 is $), which is encoded "%2524%24". When passing this value to ```appendPath()``` it is then decoded as "%24$", but when we decode it one more time we end up with " $$", which is not the original path. We then generate a URI with the appended path being _%24%24_ I think making this change to ```appendPath()``` makes sense, but I might be missing something. This was turned up when I attempted to put and get a blob named "%20 " (and " %20").

I looked at where ```@EndpointParam``` is used and it looks like EC2 is a pretty heavy user of this. I'm going to run all of the live EC2 tests and report back with results. I looked at what it does and it's not clear to me where there would be a problem with its usage. Could you elaborate further what this PR makes incorrect or difficult?

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Timur Alperovich <no...@github.com>.
@nacx I looked at all the uses of ```SkipEncoding```.

We have to keep it in Azure BlobStore, as Azure expects its root container to be named ```$root```. Passing ```%24root``` does not work in that case, as the provider does not decode the name.

For GCE, I ran all of the live tests with ```SkipEncoding``` and without it. The same tests fail in both cases (6 of them right now) with the same errors. It seems to me that either we can remove skip encoding OR there are no live tests that verify that it is required.

The ```SkipEncoding``` is also used in Chef, OpenStack Cinder, and Trove, however, I don't have an easy way to run those live tests. I can supply a gist removing it from the API if someone could run the live tests. Let's take this on as a separate PR?

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Andrew Phillips <no...@github.com>.
> we should discuss how to mitigate risk of merging a core patch.

Agreed. If we all feel that our current integration test coverage is sufficient to give us the required level of confidence, I'm fine with that, too.

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Zack Shoylev <no...@github.com>.
I'll give it a try.

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Zack Shoylev <no...@github.com>.
Ok, I finally ran the right tests :) Still one issue:

```
testBlobNameEscaping(org.jclouds.filesystem.integration.FilesystemContainerIntegrationTest)  Time elapsed: 0.016 sec  <<< FAILURE!
java.nio.file.InvalidPathException: Trailing char < > at index 33: .\target\basedir\zack-blobstore6\
        at sun.nio.fs.WindowsPathParser.normalize(WindowsPathParser.java:191)
        at sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:153)
        at sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:77)
        at sun.nio.fs.WindowsPath.parse(WindowsPath.java:94)
        at sun.nio.fs.WindowsFileSystem.getPath(WindowsFileSystem.java:255)
        at java.io.File.toPath(File.java:2234)
        at org.jclouds.filesystem.strategy.internal.FilesystemStorageStrategyImpl.putBlob(FilesystemStorageStrategyImpl.java:452)
        at org.jclouds.blobstore.config.LocalBlobStore.putBlob(LocalBlobStore.java:535)
        at sun.reflect.GeneratedMethodAccessor12.invoke(Unknown Source)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:483)
        at com.google.inject.internal.DelegatingInvocationHandler.invoke(DelegatingInvocationHandler.java:37)
        at com.sun.proxy.$Proxy44.putBlob(Unknown Source)
        at org.jclouds.blobstore.integration.internal.BaseContainerIntegrationTest.testBlobNameEscaping(BaseContainerIntegrationTest.java:573)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:483)
        at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:84)
        at org.testng.internal.Invoker.invokeMethod(Invoker.java:714)
        at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:901)
        at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1231)
        at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:127)
        at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:111)
        at org.testng.TestRunner.privateRun(TestRunner.java:767)
        at org.testng.TestRunner.run(TestRunner.java:617)
        at org.testng.SuiteRunner.runTest(SuiteRunner.java:348)
        at org.testng.SuiteRunner.access$000(SuiteRunner.java:38)
        at org.testng.SuiteRunner$SuiteWorker.run(SuiteRunner.java:382)
        at org.testng.internal.thread.ThreadUtil$2.call(ThreadUtil.java:64)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
        at java.lang.Thread.run(Thread.java:744)
```

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Andrew Gaul <no...@github.com>.
@demobox Four related/duplicated issues linked 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/851#issuecomment-135827978

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Timur Alperovich <no...@github.com>.
@zack-shoylev do you know which particular blob this fails on? If it's " ", we may want to explicitly avoid testing that case on Windows (or, probably better, use a different character that would be escaped).

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Ignasi Barrera <no...@github.com>.
I've been trying to think about counter examples for compute and no relevant ones came to my mind.
I'd say go ahead and merge this (keeping all the commit messages when squashing if possible, as they provide a very good context). @demobox @andrewgaul good to go?

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

Re: [jclouds] JCLOUDS-217: Allow for strings with % characters in jclouds (#851)

Posted by Ignasi Barrera <no...@github.com>.
Sure! I just wanted to make sure we keep all commit messages. I think this is one of the PRs with better commit messages we've ever had :)

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