You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@curator.apache.org by sclasen <gi...@git.apache.org> on 2014/06/04 18:14:12 UTC

[GitHub] curator pull request: initial BasicAuthExhibitorRestClient

GitHub user sclasen opened a pull request:

    https://github.com/apache/curator/pull/10

    initial BasicAuthExhibitorRestClient 

    that can also skip ssl cert validation if required
    
    Also might be nice to provide a convenience method in ExhibitorEnsembleProvider, something like
    
    ```java
    pubic static ExhibitorEnsembleProvider fromURL(URL vip, boolean validateSsl)
    ```
    
    which would create the `Exhibitors` from the hostname, extract the userInfo, port and path from the url

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/sclasen/curator-1 master

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/curator/pull/10.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #10
    
----
commit 1e527dd213293801844f6f8394bf53520d852d7b
Author: Scott Clasen <sc...@heroku.com>
Date:   2014-06-04T16:09:44Z

    initial BasicAuthExhibitorRestClient that can also skip ssl cert validation if required

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] curator pull request: initial BasicAuthExhibitorRestClient

Posted by madrob <gi...@git.apache.org>.
Github user madrob commented on a diff in the pull request:

    https://github.com/apache/curator/pull/10#discussion_r17336746
  
    --- Diff: curator-client/src/main/java/org/apache/curator/ensemble/exhibitor/BasicAuthExhibitorRestClient.java ---
    @@ -0,0 +1,65 @@
    +package org.apache.curator.ensemble.exhibitor;
    --- End diff --
    
    This file needs an Apache Licence header.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] curator pull request: initial BasicAuthExhibitorRestClient

Posted by Randgalt <gi...@git.apache.org>.
Github user Randgalt commented on a diff in the pull request:

    https://github.com/apache/curator/pull/10#discussion_r13522676
  
    --- Diff: curator-client/src/main/java/org/apache/curator/ensemble/exhibitor/BasicAuthExhibitorRestClient.java ---
    @@ -0,0 +1,72 @@
    +package org.apache.curator.ensemble.exhibitor;
    +
    +import org.apache.curator.utils.CloseableUtils;
    +import sun.misc.BASE64Encoder;
    +
    +import javax.net.ssl.*;
    +import java.io.BufferedInputStream;
    +import java.io.InputStream;
    +import java.net.HttpURLConnection;
    +import java.net.URI;
    +import java.security.cert.CertificateException;
    +import java.security.cert.X509Certificate;
    +
    +public class BasicAuthExhibitorRestClient  implements ExhibitorRestClient
    +{
    +    private final boolean useSsl;
    +    private final boolean validateSsl;
    +    private final String userInfo;
    +
    +    public BasicAuthExhibitorRestClient(boolean useSsl, boolean validateSsl, String userInfo)
    +    {
    +        this.useSsl = useSsl;
    +        this.validateSsl = validateSsl;
    +        this.userInfo = userInfo;
    +    }
    +
    +    @Override
    +    public String getRaw(String hostname, int port, String uriPath, String mimeType) throws Exception
    +    {
    +        URI uri = new URI(useSsl ? "https" : "http", null, hostname, port, uriPath, null, null);
    +        HttpURLConnection connection = (HttpURLConnection)uri.toURL().openConnection();
    +        if (useSsl && !validateSsl) {
    +            X509TrustManager trustAllCert = new X509TrustManager() {
    +                public void checkClientTrusted(X509Certificate[] x509Certificates, String s) throws CertificateException {}
    +
    +                public void checkServerTrusted(X509Certificate[] x509Certificates, String s) throws CertificateException {}
    +
    +                public X509Certificate[] getAcceptedIssuers() { return null; }
    +            };
    +            SSLContext sc = SSLContext.getInstance("SSL");
    +            sc.init(null, new TrustManager[]{trustAllCert}, new java.security.SecureRandom());
    +            ((HttpsURLConnection)connection).setSSLSocketFactory(sc.getSocketFactory());
    +            ((HttpsURLConnection)connection).setHostnameVerifier(new HostnameVerifier(){
    +                public boolean verify(String host,  SSLSession session){
    +                    return true;
    +                }
    +            });
    +        }
    +        connection.addRequestProperty("Accept", mimeType);
    +        connection.addRequestProperty("Authorization", "Basic " + new BASE64Encoder().encode(userInfo.getBytes()));
    +
    +        StringBuilder       str = new StringBuilder();
    --- End diff --
    
    You can use Guava's CharStreams here. It would be cleaner:
    
    ```java
    Reader in = new InputStreamReader(new BufferedInputStream(connection.getInputStream()));
    try
    {
        return CharStreams.toString(in);
    }
    finally
    {
        CloseableUtils.closeQuietly(in);
    }
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] curator pull request: initial BasicAuthExhibitorRestClient

Posted by madrob <gi...@git.apache.org>.
Github user madrob commented on a diff in the pull request:

    https://github.com/apache/curator/pull/10#discussion_r17336834
  
    --- Diff: curator-client/src/main/java/org/apache/curator/ensemble/exhibitor/BasicAuthExhibitorRestClient.java ---
    @@ -0,0 +1,65 @@
    +package org.apache.curator.ensemble.exhibitor;
    +
    +import com.google.common.io.CharStreams;
    +import org.apache.curator.utils.CloseableUtils;
    +import sun.misc.BASE64Encoder;
    +
    +import javax.net.ssl.*;
    +import java.io.BufferedInputStream;
    +import java.io.InputStream;
    +import java.io.InputStreamReader;
    +import java.io.Reader;
    +import java.net.HttpURLConnection;
    +import java.net.URI;
    +import java.security.cert.CertificateException;
    +import java.security.cert.X509Certificate;
    +
    +public class BasicAuthExhibitorRestClient  implements ExhibitorRestClient
    +{
    +    private final boolean useSsl;
    +    private final boolean validateSsl;
    +    private final String userInfo;
    +
    +    public BasicAuthExhibitorRestClient(boolean useSsl, boolean validateSsl, String userInfo)
    --- End diff --
    
    The Default client lets specifying SSL be optional (defaulting to false), it might be good to preserve that pattern here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] curator pull request: initial BasicAuthExhibitorRestClient

Posted by madrob <gi...@git.apache.org>.
Github user madrob commented on a diff in the pull request:

    https://github.com/apache/curator/pull/10#discussion_r17340486
  
    --- Diff: curator-client/src/main/java/org/apache/curator/ensemble/exhibitor/BasicAuthExhibitorRestClient.java ---
    @@ -0,0 +1,65 @@
    +package org.apache.curator.ensemble.exhibitor;
    +
    +import com.google.common.io.CharStreams;
    +import org.apache.curator.utils.CloseableUtils;
    +import sun.misc.BASE64Encoder;
    +
    +import javax.net.ssl.*;
    +import java.io.BufferedInputStream;
    +import java.io.InputStream;
    +import java.io.InputStreamReader;
    +import java.io.Reader;
    +import java.net.HttpURLConnection;
    +import java.net.URI;
    +import java.security.cert.CertificateException;
    +import java.security.cert.X509Certificate;
    +
    +public class BasicAuthExhibitorRestClient  implements ExhibitorRestClient
    +{
    +    private final boolean useSsl;
    +    private final boolean validateSsl;
    +    private final String userInfo;
    +
    +    public BasicAuthExhibitorRestClient(boolean useSsl, boolean validateSsl, String userInfo)
    +    {
    +        this.useSsl = useSsl;
    +        this.validateSsl = validateSsl;
    +        this.userInfo = userInfo;
    +    }
    +
    +    @Override
    +    public String getRaw(String hostname, int port, String uriPath, String mimeType) throws Exception
    +    {
    +        URI uri = new URI(useSsl ? "https" : "http", null, hostname, port, uriPath, null, null);
    +        HttpURLConnection connection = (HttpURLConnection)uri.toURL().openConnection();
    --- End diff --
    
    I'm not sure this cast is necessary, I think the connection can be a plain URLConnection for how we use it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] curator pull request: initial BasicAuthExhibitorRestClient

Posted by sclasen <gi...@git.apache.org>.
Github user sclasen commented on the pull request:

    https://github.com/apache/curator/pull/10#issuecomment-45111970
  
    This is for CURATOR-112


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] curator pull request: initial BasicAuthExhibitorRestClient

Posted by madrob <gi...@git.apache.org>.
Github user madrob commented on a diff in the pull request:

    https://github.com/apache/curator/pull/10#discussion_r17340570
  
    --- Diff: curator-client/src/main/java/org/apache/curator/ensemble/exhibitor/BasicAuthExhibitorRestClient.java ---
    @@ -0,0 +1,65 @@
    +package org.apache.curator.ensemble.exhibitor;
    +
    +import com.google.common.io.CharStreams;
    +import org.apache.curator.utils.CloseableUtils;
    +import sun.misc.BASE64Encoder;
    +
    +import javax.net.ssl.*;
    +import java.io.BufferedInputStream;
    +import java.io.InputStream;
    +import java.io.InputStreamReader;
    +import java.io.Reader;
    +import java.net.HttpURLConnection;
    +import java.net.URI;
    +import java.security.cert.CertificateException;
    +import java.security.cert.X509Certificate;
    +
    +public class BasicAuthExhibitorRestClient  implements ExhibitorRestClient
    +{
    +    private final boolean useSsl;
    +    private final boolean validateSsl;
    +    private final String userInfo;
    +
    +    public BasicAuthExhibitorRestClient(boolean useSsl, boolean validateSsl, String userInfo)
    +    {
    +        this.useSsl = useSsl;
    +        this.validateSsl = validateSsl;
    +        this.userInfo = userInfo;
    +    }
    +
    +    @Override
    +    public String getRaw(String hostname, int port, String uriPath, String mimeType) throws Exception
    +    {
    +        URI uri = new URI(useSsl ? "https" : "http", null, hostname, port, uriPath, null, null);
    +        HttpURLConnection connection = (HttpURLConnection)uri.toURL().openConnection();
    +        if (useSsl && !validateSsl) {
    +            X509TrustManager trustAllCert = new X509TrustManager() {
    --- End diff --
    
    This could be a statifc object instead of creating a new one on every single request. Same for the HostnameVerifier below.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---