You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Ignasi Barrera <no...@github.com> on 2013/12/12 16:36:18 UTC

[jclouds] Properly set the request method in HTTPS connections (#230)

This pull request allows to properly set the request method in HTTPS connections.

See:
https://java.net/jira/browse/JERSEY-639
https://github.com/jersey/jersey/pull/45
You can merge this Pull Request by running:

  git pull https://github.com/nacx/jclouds patch-ssl

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

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

-- Commit Summary --

  * Properly set the request method in HTTPS connections

-- File Changes --

    M core/src/main/java/org/jclouds/http/internal/JavaUrlHttpCommandExecutorService.java (65)

-- Patch Links --

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

Re: [jclouds] Properly set the request method in HTTPS connections (#230)

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

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

Re: [jclouds] Properly set the request method in HTTPS connections (#230)

Posted by Andrew Phillips <no...@github.com>.
> +            propagate(e);
> +         }
> +         try {
> +            Field methodField = null;
> +            while (connectionClass != null) {
> +               try {
> +                  methodField = connectionClass.getDeclaredField("method");
> +               } catch (NoSuchFieldException e) {
> +                  connectionClass = connectionClass.getSuperclass();
> +                  continue;
> +               }
> +               methodField.setAccessible(true);
> +               methodField.set(httpURLConnection, method);
> +               break;
> +            }
> +         } catch (final Exception e) {

Can we catch a less general exception here?

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

Re: [jclouds] Properly set the request method in HTTPS connections (#230)

Posted by Everett Toews <no...@github.com>.
+1

This can really only be tested via a live test. I'll be adding a live test to https://github.com/jclouds/jclouds-labs-openstack/pull/60 for the update() method that uses PATCH.

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

Re: [jclouds] Properly set the request method in HTTPS connections (#230)

Posted by Everett Toews <no...@github.com>.
Okay. I'm going to go ahead and merge this and https://github.com/jclouds/jclouds-labs-openstack/pull/60 as is. Then I'll create another PR specifically for the ClaimApi.update() method that uses PATCH. That way if we have to revert this change quickly, I can revert the change for ClaimApi.update() without impacting the entire ClaimApi.

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

Re: [jclouds] Properly set the request method in HTTPS connections (#230)

Posted by Andrew Phillips <no...@github.com>.
@nacx: Thanks! If we end up creating a follow-up commit, mention "JCLOUDS-405" in the commit message?

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

Re: [jclouds] Properly set the request method in HTTPS connections (#230)

Posted by Andrew Phillips <no...@github.com>.
> @@ -227,6 +216,56 @@ protected HttpURLConnection convert(HttpRequest request) throws IOException, Int
>        return connection;
>     }
>  
> +   /**

Implementation rather than a Javadoc comment?

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

Re: [jclouds] Properly set the request method in HTTPS connections (#230)

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

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