Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add https support #47

Merged
merged 16 commits into from
Jun 11, 2020
Merged

add https support #47

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
fd7c0df
add https support
shzcore Jun 8, 2020
e055386
coding Standards
shzcore Jun 8, 2020
4c2ee03
fix coding standards
shzcore Jun 8, 2020
82d4c9d
fix icoding standards
shzcore Jun 8, 2020
9fc4cc4
coding standards
shzcore Jun 8, 2020
c294456
coding standards
shzcore Jun 8, 2020
7ed3f36
coding standards
shzcore Jun 8, 2020
87ed884
coding standards
shzcore Jun 9, 2020
b4d1134
coding standards
shzcore Jun 9, 2020
2a38b12
coding standards
shzcore Jun 9, 2020
648cefb
add test
shzcore Jun 11, 2020
4e57191
coding standards
shzcore Jun 11, 2020
83551d8
coding standards
shzcore Jun 11, 2020
bf90c31
add test HostNameVerifier
shzcore Jun 11, 2020
0e14e94
add test HostNameVerifier
shzcore Jun 11, 2020
ca326a6
add test HostNameVerifier
shzcore Jun 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

<groupId>com.baidu.hugegraph</groupId>
<artifactId>hugegraph-common</artifactId>
<version>1.7.5</version>
<version>1.7.6</version>

<name>hugegraph-common</name>
<url>https://github.com/hugegraph/hugegraph-common</url>
Expand Down Expand Up @@ -260,7 +260,7 @@
<manifestEntries>
<!-- Must be on one line, otherwise the automatic
upgrade script cannot replace the version number -->
<Implementation-Version>1.7.5.0</Implementation-Version>
<Implementation-Version>1.7.6.0</Implementation-Version>
</manifestEntries>
</archive>
</configuration>
Expand Down
128 changes: 106 additions & 22 deletions src/main/java/com/baidu/hugegraph/rest/RestClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,27 +19,12 @@

package com.baidu.hugegraph.rest;

import static org.glassfish.jersey.apache.connector.ApacheClientProperties.CONNECTION_MANAGER;

import java.util.Collection;
import java.util.Map;
import java.util.concurrent.Callable;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;

import javax.ws.rs.client.Client;
import javax.ws.rs.client.ClientBuilder;
import javax.ws.rs.client.Entity;
import javax.ws.rs.client.Invocation.Builder;
import javax.ws.rs.client.WebTarget;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Variant;

import com.baidu.hugegraph.util.ExecutorUtil;
import com.google.common.collect.ImmutableMap;
import org.apache.commons.lang3.tuple.Pair;
import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
import org.apache.http.pool.PoolStats;
import org.glassfish.jersey.SslConfigurator;
import org.glassfish.jersey.apache.connector.ApacheConnectorProvider;
import org.glassfish.jersey.client.ClientConfig;
import org.glassfish.jersey.client.ClientProperties;
Expand All @@ -49,8 +34,26 @@
import org.glassfish.jersey.message.GZipEncoder;
import org.glassfish.jersey.uri.UriComponent;

import com.baidu.hugegraph.util.ExecutorUtil;
import com.google.common.collect.ImmutableMap;
import javax.net.ssl.*;
import javax.ws.rs.client.Client;
import javax.ws.rs.client.ClientBuilder;
import javax.ws.rs.client.Entity;
import javax.ws.rs.client.Invocation.Builder;
import javax.ws.rs.client.WebTarget;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Variant;
import java.security.KeyManagementException;
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
import java.util.Collection;
import java.util.Map;
import java.util.concurrent.Callable;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;

import static org.glassfish.jersey.apache.connector.ApacheClientProperties.CONNECTION_MANAGER;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep origin order

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

address it


public abstract class RestClient {

Expand Down Expand Up @@ -90,9 +93,63 @@ public RestClient(String url, String user, String password, int timeout,
.config(maxTotal, maxPerRoute)
.build());
}


public RestClient(String url, String user, String password, int timeout,
int maxTotal, int maxPerRoute, String protocol,
String trustStoreFile, String trustStorePassword) {
this(url, new ConfigBuilder().config(timeout)
.config(user, password)
.config(maxTotal, maxPerRoute)
.config(protocol, trustStoreFile, trustStorePassword)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line 109 seems exceed 80 chars, wrap line like

.config(protocol, trustStoreFile, 
        trustStorePassword)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also pay attention to other places

.build());
}

private TrustManager[] NoCheckTrustManager() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

method name must be started with lower case, rename to createNoCheckTrustManager

return new TrustManager[]{
new X509TrustManager()
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

put in one line

}

private static class X509TrustManager implements javax.net.ssl.X509TrustManager {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to NoCheckTrustManager

@Override
shzcore marked this conversation as resolved.
Show resolved Hide resolved
public void checkClientTrusted(X509Certificate[] chain, String authType)
throws CertificateException {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

align with X509Certificate

}

@Override
public void checkServerTrusted(X509Certificate[] chain, String authType)
throws CertificateException {
}

@Override
public X509Certificate[] getAcceptedIssuers() {
return null;
}
}

public RestClient(String url, ClientConfig config) {
this.client = ClientBuilder.newClient(config);
Client client = null;
Object protocol = config.getProperties().get("protocol");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

if (protocol != null && protocol.equals("https")) {
try {
SslConfigurator sslConfig = SslConfigurator.newInstance();
sslConfig.trustStoreFile(config.getProperties().get("trustStoreFile").toString())
.trustStorePassword(config.getProperties().get("trustStorePassword").toString());
sslConfig.securityProtocol("SSL");
SSLContext sc = sslConfig.createSSLContext();
TrustManager[] trustAllCerts = NoCheckTrustManager();
sc.init(null, trustAllCerts, new java.security.SecureRandom());
client = ClientBuilder.newBuilder()
.hostnameVerifier(new HostNameVerifier(url))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

align with .newBuilder

.sslContext(sc)
.build();
} catch (KeyManagementException e) {
throw new ClientException("security key management exception", e);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

improve message

}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a method

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

address it

} else {
client = ClientBuilder.newClient(config);
}
this.client = client;
this.client.register(GZipEncoder.class);
this.target = this.client.target(url);
this.pool = (PoolingHttpClientConnectionManager)
Expand Down Expand Up @@ -284,6 +341,26 @@ private static String encode(String raw) {
return UriComponent.encode(raw, UriComponent.Type.PATH_SEGMENT);
}

private static class HostNameVerifier implements HostnameVerifier {

private String url;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set final


private HostNameVerifier(String url) {
this.url = url;
}

@Override
public boolean verify(String hostname, SSLSession session) {
if (!this.url.equals("") && this.url.contains(hostname)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use this.url.isEmpty() instead of this.url.equals("")

return true;
} else {
HostnameVerifier hv = HttpsURLConnection.getDefaultHostnameVerifier();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename hv to verifier

return hv.verify(hostname, session);
}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove it

}

protected abstract void checkStatus(Response response,
Response.Status... statuses);

Expand Down Expand Up @@ -336,6 +413,13 @@ public ConfigBuilder config(int maxTotal, int maxPerRoute) {
return this;
}

public ConfigBuilder config(String protocol, String trustStoreFile, String trustStorePassword) {
this.config.property("protocol", protocol);
this.config.property("trustStoreFile", trustStoreFile);
this.config.property("trustStorePassword", trustStorePassword);
return this;
}

public ClientConfig build() {
return this.config;
}
Expand Down
11 changes: 11 additions & 0 deletions src/test/java/com/baidu/hugegraph/unit/rest/RestClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,17 @@ public RestClientImpl(String url, String user, String password,
this.content = "";
}

public RestClientImpl(String url, String user, String password,
int timeout, int maxTotal, int maxPerRoute,
String protocol, String trustStoreFile,
String trustStorePassword, int status) {
super(url, user, password, timeout, maxTotal, maxPerRoute, protocol,
trustStoreFile, trustStorePassword);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

align

this.status = status;
this.headers = ImmutableMultivaluedMap.empty();
this.content = "";
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just add a test case: testHttpsGet:

    public void testHttpsGet() {
        RestClient client = new RestClientImpl(...);
        RestResult restResult = client.get("path", "id1");
        Assert.assertEquals(200, restResult.status());
    }

public RestClientImpl(String url, int timeout, int status) {
this(url, timeout, status, ImmutableMultivaluedMap.empty(), "");
}
Expand Down