You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Andrew Phillips <no...@github.com> on 2013/07/30 02:32:56 UTC
[jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Submitted by Rodney Beede
You can merge this Pull Request by running:
git pull https://github.com/jclouds/jclouds fix-jclouds-155
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/76
-- Commit Summary --
* JCLOUDS-155: Making header handling in OpenStack case-insensitive
-- File Changes --
M apis/swift/src/test/java/org/jclouds/openstack/functions/ParseAuthenticationResponseFromHeadersTest.java (13)
M common/openstack/pom.xml (5)
M common/openstack/src/main/java/org/jclouds/openstack/functions/ParseAuthenticationResponseFromHeaders.java (17)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/76.patch
https://github.com/jclouds/jclouds/pull/76.diff
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Phillips <no...@github.com>.
> Let's see what DEV@cloud says...
Actually, I suspect this may not trigger on PRs coming from the same repo :-( Closing and re-opening instead...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76#issuecomment-21768345
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Gaul <no...@github.com>.
Blobstore integration tests succeed against CloudFiles except for:
https://issues.apache.org/jira/browse/JCLOUDS-179
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76#issuecomment-21954809
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Phillips <no...@github.com>.
> But methinks we also need an Expect test for this
Tried this, but it's surprisingly hard to find the correct place. I thought [SwiftBlobSignerExpectTest](https://github.com/jclouds/jclouds/blob/master/apis/swift/src/test/java/org/jclouds/openstack/swift/blobstore/SwiftBlobSignerExpectTest.java#L110) might be a good candidate, but it turns out that that response object isn't really processed at all :-(
Then I thought of creating an expect test for the [OpenStackAuthAsyncClient](https://github.com/jclouds/jclouds/blob/master/common/openstack/src/main/java/org/jclouds/openstack/internal/OpenStackAuthAsyncClient.java), but that ends up being pretty much a duplicate of the existing parser unit test.
@everett-toews @zack-shoylev Any suggestions?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76#issuecomment-21839598
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Everett Toews <no...@github.com>.
@demobox Have you had a chance to live test this?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76#issuecomment-21942988
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Phillips <no...@github.com>.
Just waiting for BuildHive et al after the rebase. 1.6.x version here: https://github.com/jclouds/jclouds/pull/91
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76#issuecomment-21969623
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Gaul <no...@github.com>.
+1
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76#issuecomment-21955651
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #99](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/99/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76#issuecomment-21973407
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Phillips <no...@github.com>.
> jclouds » jclouds #234 UNSTABLE
This one (`testloopUntilTrueOrThrowCancellationExceptionReturnsWhenPredicateIsTrueSecondTimeWhileNotCancelled`) looks unrelated. Let's see what DEV@cloud says...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76#issuecomment-21768286
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Phillips <no...@github.com>.
> public AuthenticationResponse apply(HttpResponse from) {
> releasePayload(from);
> - Builder<String, URI> builder = ImmutableMap.builder();
> - for (Entry<String, String> entry : from.getHeaders().entries()) {
> - if (entry.getKey().endsWith(URL_SUFFIX))
> - builder.put(entry.getKey(), getURI(entry.getValue()));
> +
> + // HTTP headers are case in-sensitive (RFC 2616) so we must allow for that when looking an header names for the URL keyword
> + //FIXME When Apache commons-collections 4 is out of Snapshot it will provide generics for CaseInsensitiveMap
> + final CaseInsensitiveMap services = new CaseInsensitiveMap();
> + for (final Entry<String, String> entry : from.getHeaders().entries()) {
> + if (entry.getKey().toLowerCase().endsWith(URL_SUFFIX.toLowerCase()))
> + services.put(entry.getKey(), getURI(entry.getValue()));
Would the following (avoiding the case insensitive map) not work here?
```
Builder<String, String> services = ImmutableMap.builder()
for (Entry<String, String> entry : from.getHeaders().entries()) {
String lowerCaseKey = entry.getKey().toLowerCase();
if (lowerCaseKey.endsWith(URL_SUFFIX.toLowerCase()) { // perhaps add a URL_SUFFIX_LOWER constant?
services.put(lowerCaseKey, getURI(entry.getValue()).toLowerCase());
}
...
Obviously, this will differ in that the map will not have case-insensitive _comparison_, simply lowercase-only entries.
@everett-toews: any idea where, if anywhere, the _usage_ of this map would need to be modified to be case-insensitive?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76/files#r5466201
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Everett Toews <no...@github.com>.
@demobox Yep. We'll give it a shot. Definitely want to get it into 1.6.2. We'll get to it today for sure. When do you plan to start cutting 1.6.2?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76#issuecomment-21944567
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Phillips <no...@github.com>.
Reopened #76.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #232](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/232/) 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/76#issuecomment-21763762
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Phillips <no...@github.com>.
> jclouds » jclouds #235 SUCCESS
Second time lucky. But methinks we also need an Expect test for this, since right now we don't really have something that documents the actual scenario we're trying to verify...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76#issuecomment-21785519
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Phillips <no...@github.com>.
> @@ -50,6 +50,11 @@
> <type>test-jar</type>
> <scope>test</scope>
> </dependency>
> + <dependency>
> + <groupId>commons-collections</groupId>
> + <artifactId>commons-collections</artifactId>
> + <version>3.2.1</version>
> + </dependency>
I can't image we're going add a dependency on commons-collections for this. Please find another way to implement `CaseInsensitiveMap`.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76/files#r5466081
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Phillips <no...@github.com>.
> .create("http://fooman:8080/v1/token"))));
> +
> +
> + // Additional test that verifies that the case insensitive header "X-Storage-Url" is actually replaced
> + // https://issues.apache.org/jira/browse/JCLOUDS-155
> + response = HttpResponse.builder().statusCode(204).message("No Content")
> + .addHeader("X-Auth-Token".toLowerCase(), "token")
> + .addHeader("X-Storage-Token".toLowerCase(), "token")
> + .addHeader("X-Storage-Url".toLowerCase(), "http://127.0.0.1:8080/v1/token").build();
> + md = parser.apply(response);
> + assertEquals(md, new AuthenticationResponse("token", ImmutableMap.<String, URI> of("X-Storage-Url".toLowerCase(), URI
Same here
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76/files#r5466070
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Phillips <no...@github.com>.
Closed #76.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #239](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/239/) 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/76#issuecomment-21841963
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Everett Toews <no...@github.com>.
Sorry for the delay. Just tested it with vanilla OpenStack Swift.
+1
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76#issuecomment-21966675
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #262](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/262/) 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/76#issuecomment-21971891
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Phillips <no...@github.com>.
> Builder<String, URI> builder = ImmutableMap.builder();
> for (Entry<String, String> entry : from.getHeaders().entries()) {
> - if (entry.getKey().endsWith(URL_SUFFIX))
> + if (entry.getKey().toLowerCase().endsWith(URL_SUFFIX.toLowerCase()))
Removed the whole piece about the case insensitive map. None of that is preserved anyway because `new AuthenticationResponse` makes a copy of the map.
Let's see what BuildHive has to say about this version...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76/files#r5467681
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Phillips <no...@github.com>.
> jclouds » jclouds #232 UNSTABLE
These test failures certainly seem related, although I can't as yet see how. They all follow the following pattern:
```
the following request is not configured:
----------------------------------------
PUT http://myhost:8080/auth/foo/bar.txt HTTP/1.1
X-Copy-From: /bar/foo.txt
X-Auth-Token: AUTH_tk36dabe83ca744cc296a98ec46089ec35
----------------------------------------
configured requests:
----------------------------------------
...
----------------------------------------
PUT http://myhost:8080/v1/AUTH_test/foo/bar.txt HTTP/1.1
X-Copy-From: /bar/foo.txt
X-Auth-Token: AUTH_tk36dabe83ca744cc296a98ec46089ec35
]
```
where `v1/AUTH_test` is somehow now no longer being sent...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76#issuecomment-21765165
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Phillips <no...@github.com>.
> .create("http://fooman:8080/v1/token"))));
> +
> +
> + // Additional test that verifies that the case insensitive header "X-Storage-Url" is actually replaced
> + // https://issues.apache.org/jira/browse/JCLOUDS-155
> + response = HttpResponse.builder().statusCode(204).message("No Content")
> + .addHeader("X-Auth-Token".toLowerCase(), "token")
> + .addHeader("X-Storage-Token".toLowerCase(), "token")
> + .addHeader("X-Storage-Url".toLowerCase(), "http://127.0.0.1:8080/v1/token").build();
Just make these lowercase? E.g.
```
.addHeader("x-auth-token", "token")
```
etc.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76/files#r5466065
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Phillips <no...@github.com>.
> Have you had a chance to live test this?
No, unfortunately not - no access to an OpenStack instance right now. If we want to get this into 1.6.2-incubating it'd be great if you or @zack-shoylev would have a chance to do this!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76#issuecomment-21944194
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Phillips <no...@github.com>.
Thanks, @andrewgaul. Just giving @everett-toews a bit more time, too, before merging.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76#issuecomment-21956584
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Gaul <no...@github.com>.
> public AuthenticationResponse apply(HttpResponse from) {
> releasePayload(from);
> - Builder<String, URI> builder = ImmutableMap.builder();
> - for (Entry<String, String> entry : from.getHeaders().entries()) {
> - if (entry.getKey().endsWith(URL_SUFFIX))
> - builder.put(entry.getKey(), getURI(entry.getValue()));
> +
> + // HTTP headers are case in-sensitive (RFC 2616) so we must allow for that when looking an header names for the URL keyword
> + //FIXME When Apache commons-collections 4 is out of Snapshot it will provide generics for CaseInsensitiveMap
> + final CaseInsensitiveMap services = new CaseInsensitiveMap();
> + for (final Entry<String, String> entry : from.getHeaders().entries()) {
> + if (entry.getKey().toLowerCase().endsWith(URL_SUFFIX.toLowerCase()))
> + services.put(entry.getKey(), getURI(entry.getValue()));
Can we do something like:
```
Map<String, String> map = new TreeMap<String, String>(String.CASE_INSENSITIVE_ORDER);
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76/files#r5466219
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Phillips <no...@github.com>.
@andrewgaul @everett-toews Kindly take a quick look - does this look good to go?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76#issuecomment-21938865
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Phillips <no...@github.com>.
> @@ -63,4 +63,17 @@ public void testReplaceLocalhost() {
> assertEquals(md, new AuthenticationResponse("token", ImmutableMap.<String, URI> of("X-Storage-Url", URI
> .create("http://fooman:8080/v1/token"))));
> }
> +
> + public void testHandleHeadersCaseInsensitively() {
> + ParseAuthenticationResponseFromHeaders parser = i.getInstance(ParseAuthenticationResponseFromHeaders.class);
> + parser = parser.setHostToReplace("fooman");
> +
> + HttpResponse response = HttpResponse.builder().statusCode(204).message("No Content")
> + .addHeader("x-auth-token", "token")
> + .addHeader("x-storage-token", "token")
> + .addHeader("x-storage-url", "http://127.0.0.1:8080/v1/token").build();
> + AuthenticationResponse md = parser.apply(response);
> + assertEquals(md, new AuthenticationResponse("token", ImmutableMap.<String, URI> of("x-storage-url".toLowerCase(), URI
> + .create("http://fooman:8080/v1/token"))));
> + }
Verified that this fails without the change below
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76/files#r5467673
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #234](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/234/) 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/76#issuecomment-21768095
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Gaul <no...@github.com>.
> public AuthenticationResponse apply(HttpResponse from) {
> releasePayload(from);
> - Builder<String, URI> builder = ImmutableMap.builder();
> - for (Entry<String, String> entry : from.getHeaders().entries()) {
> - if (entry.getKey().endsWith(URL_SUFFIX))
> - builder.put(entry.getKey(), getURI(entry.getValue()));
> +
> + // HTTP headers are case in-sensitive (RFC 2616) so we must allow for that when looking an header names for the URL keyword
> + //FIXME When Apache commons-collections 4 is out of Snapshot it will provide generics for CaseInsensitiveMap
> + final CaseInsensitiveMap services = new CaseInsensitiveMap();
> + for (final Entry<String, String> entry : from.getHeaders().entries()) {
> + if (entry.getKey().toLowerCase().endsWith(URL_SUFFIX.toLowerCase()))
> + services.put(entry.getKey(), getURI(entry.getValue()));
@everett-toews I believe a Comparator suffices:
https://gist.github.com/andrewgaul/6109765
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76/files#r5467553
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #238](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/238/) 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/76#issuecomment-21840778
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Everett Toews <no...@github.com>.
> public AuthenticationResponse apply(HttpResponse from) {
> releasePayload(from);
> - Builder<String, URI> builder = ImmutableMap.builder();
> - for (Entry<String, String> entry : from.getHeaders().entries()) {
> - if (entry.getKey().endsWith(URL_SUFFIX))
> - builder.put(entry.getKey(), getURI(entry.getValue()));
> +
> + // HTTP headers are case in-sensitive (RFC 2616) so we must allow for that when looking an header names for the URL keyword
> + //FIXME When Apache commons-collections 4 is out of Snapshot it will provide generics for CaseInsensitiveMap
> + final CaseInsensitiveMap services = new CaseInsensitiveMap();
> + for (final Entry<String, String> entry : from.getHeaders().entries()) {
> + if (entry.getKey().toLowerCase().endsWith(URL_SUFFIX.toLowerCase()))
> + services.put(entry.getKey(), getURI(entry.getValue()));
@demobox Just in the swift and cloudfiles* projects. We would have to do live testing before committing it (after the changes are finalized).
@andrewgaul In that case, isn't the case insensitivity only being in the TreeMap for ordering and not for when doing a get()?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76/files#r5467402
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #235](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/235/) 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/76#issuecomment-21769551
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #554](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/554/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76#issuecomment-21972360
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Phillips <no...@github.com>.
> When do you plan to start cutting 1.6.2?
The plan was to start this afternoon, but I'd rather wait a little bit and avoid a crazy last-minute rush.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76#issuecomment-21946266
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
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=375cb2075d22a26ff11fc6491fda57e347f207b4)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76#issuecomment-21976539
Re: [jclouds] JCLOUDS-155: Making header handling in OpenStack
case-insensitive (#76)
Posted by Andrew Phillips <no...@github.com>.
Closed #76.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/76