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

Jump to bottom

Add support for SSL Context per connection #180

Closed
wants to merge 8 commits into from
Closed

Add support for SSL Context per connection #180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
bf06202
Add support for SSL Context per connection
May 25, 2019
7aad9d5
Move raw url connection creation back to send method
joris-lambrechts Oct 17, 2019
c693112
Merge branch 'master' of https://github.com/jcabi/jcabi-http
joris-lambrechts Oct 17, 2019
951affb
Merge remote-tracking branch 'upstream/master'
Jun 20, 2020
d1e98fd
Fix merge conflicts
Jun 20, 2020
761a330
Fix checkstyle errors
Jun 26, 2020
ee59676
Remove context resolution in TrustedWire to restore default behaviour
Jun 27, 2020
442f304
Add test cases to verify that the provided SSLContext is used
Jun 27, 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
9 changes: 9 additions & 0 deletions src/main/java/com/jcabi/http/Request.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import com.jcabi.aspects.Immutable;
import java.io.IOException;
import java.io.InputStream;
import javax.net.ssl.SSLContext;

/**
* RESTful request.
Expand Down Expand Up @@ -165,6 +166,14 @@ public interface Request {
*/
Request timeout(int connect, int read);

/**
* Use this SSL Context.
*
* @param context The SSL Context to use
* @return New alternated request
*/
Request sslcontext(SSLContext context);

/**
* Execute it with a specified HTTP method.
* @return Response
Expand Down
7 changes: 5 additions & 2 deletions src/main/java/com/jcabi/http/Wire.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import java.io.InputStream;
import java.util.Collection;
import java.util.Map;
import javax.net.ssl.SSLContext;

/**
* Wire.
Expand Down Expand Up @@ -69,12 +70,14 @@ public interface Wire {
* @param content HTTP body
* @param connect The connect timeout
* @param read The read timeout
* @param sslcontext SSL Context
* @return Response obtained
* @throws IOException if fails
*/
Response send(Request req, String home, String method,
Collection<Map.Entry<String, String>> headers, InputStream content,
int connect, int read)
Collection<Map.Entry<String, String>> headers,
InputStream content, int connect, int read,
SSLContext sslcontext)
throws IOException;

}
38 changes: 23 additions & 15 deletions src/main/java/com/jcabi/http/request/ApacheRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import java.util.Collection;
import java.util.LinkedList;
import java.util.Map;
import javax.net.ssl.SSLContext;
import lombok.EqualsAndHashCode;
import lombok.ToString;
import org.apache.http.Header;
Expand All @@ -54,7 +55,8 @@
import org.apache.http.client.methods.HttpEntityEnclosingRequestBase;
import org.apache.http.entity.BufferedHttpEntity;
import org.apache.http.entity.InputStreamEntity;
import org.apache.http.impl.client.HttpClients;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.util.EntityUtils;

/**
Expand Down Expand Up @@ -87,24 +89,25 @@ public Response send(final Request req, final String home,
final Collection<Map.Entry<String, String>> headers,
final InputStream content,
final int connect,
final int read) throws IOException {
final CloseableHttpResponse response =
HttpClients.createSystem().execute(
final int read,
final SSLContext sslcontext) throws IOException {
final CloseableHttpClient client = HttpClientBuilder.create()
.useSystemProperties()
.setSSLContext(sslcontext)
.build();
try (CloseableHttpResponse response = client.execute(
this.httpRequest(
home, method, headers, content,
connect, read
home, method, headers, content,
connect, read
)
);
try {
)) {
return new DefaultResponse(
req,
response.getStatusLine().getStatusCode(),
response.getStatusLine().getReasonPhrase(),
this.headers(response.getAllHeaders()),
this.consume(response.getEntity())
req,
response.getStatusLine().getStatusCode(),
response.getStatusLine().getReasonPhrase(),
this.headers(response.getAllHeaders()),
this.consume(response.getEntity())
);
} finally {
response.close();
}
}
/**
Expand Down Expand Up @@ -245,6 +248,11 @@ public Request method(final String method) {
return this.base.method(method);
}

@Override
public Request sslcontext(final SSLContext context) {
return this.base.sslcontext(context);
}

@Override
public Request timeout(final int connect, final int read) {
return this.base.timeout(connect, read);
Expand Down
43 changes: 42 additions & 1 deletion src/main/java/com/jcabi/http/request/BaseRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import java.util.Map;
import javax.json.Json;
import javax.json.JsonStructure;
import javax.net.ssl.SSLContext;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.UriBuilder;
import lombok.EqualsAndHashCode;
Expand Down Expand Up @@ -138,6 +139,11 @@ public final class BaseRequest implements Request {
@Immutable.Array
private final transient byte[] content;

/**
* SSL Context.
*/
private final transient SSLContext sslctx;

/**
* Public ctor.
* @param wre Wire
Expand Down Expand Up @@ -181,6 +187,26 @@ public BaseRequest(final Wire wre, final String uri,
final Iterable<Map.Entry<String, String>> headers,
final String method, final byte[] body,
final int cnct, final int rdd) {
this(wre, uri, headers, method, body, cnct, rdd, null);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@joris-lambrechts Can we avoid null here by passing SSLContext.getDefault()?

Copy link
Author

@joris-lambrechts joris-lambrechts Jun 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@andreoss SSLContext.getDefault() throws a checked java.security.NoSuchAlgorithmException. I could use Lomboks lombok.SneakyThrows to overcome this with a @SneakyThrows(NoSuchAlgorithmException.class) if allowed

Copy link
Author

Choose a reason for hiding this comment

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

It's not possible when calling a sibling constructor. I tried to default it in the constructor with all the parameters but that causes test failures breaking the default behavior. I'll leave it as-is with the extra null check.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@joris-lambrechts You can extract this to a private static method, just like in TrustedWire

Copy link
Author

Choose a reason for hiding this comment

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

Is there a need to have this as the default behavior? Not all communication uses https, e.g. internal services might still use non-secure connections.

}

/**
* Public ctor.
* @param wre Wire
* @param uri The resource to work with
* @param headers Headers
* @param method HTTP method
* @param body HTTP request body
* @param cnct Connect timeout for http connection
* @param rdd Read timeout for http connection
* @param context The SSL context to use
* @checkstyle ParameterNumber (5 lines)
*/
public BaseRequest(final Wire wre, final String uri,
final Iterable<Map.Entry<String, String>> headers,
final String method, final byte[] body,
final int cnct, final int rdd,
final SSLContext context) {
this.wire = wre;
URI addr = URI.create(uri);
if (addr.getPath() != null && addr.getPath().isEmpty()) {
Expand All @@ -192,6 +218,21 @@ public BaseRequest(final Wire wre, final String uri,
this.content = body.clone();
this.connect = cnct;
this.read = rdd;
this.sslctx = context;
}

@Override
public Request sslcontext(final SSLContext context) {
return new BaseRequest(
this.wire,
this.home,
this.hdrs,
this.mtd,
this.content,
this.connect,
this.read,
context
);
}

@Override
Expand Down Expand Up @@ -364,7 +405,7 @@ private Response fetchResponse(final InputStream stream)
final Response response = this.wire.send(
this, this.home, this.mtd,
this.hdrs, stream, this.connect,
this.read
this.read, this.sslctx
);
final URI uri = URI.create(this.home);
Logger.info(
Expand Down
11 changes: 9 additions & 2 deletions src/main/java/com/jcabi/http/request/FakeRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.Map;
import javax.net.ssl.SSLContext;
import lombok.EqualsAndHashCode;

/**
Expand Down Expand Up @@ -131,13 +132,14 @@ public FakeRequest(final int status, final String reason,
this.base = new BaseRequest(
new Wire() {
@Override
// @checkstyle ParameterNumber (6 lines)
// @checkstyle ParameterNumber (7 lines)
public Response send(final Request req, final String home,
final String method,
final Collection<Map.Entry<String, String>> headers,
final InputStream text,
final int connect,
final int read) {
final int read,
final SSLContext sslcontext) {
return new DefaultResponse(
req,
FakeRequest.this.code,
Expand Down Expand Up @@ -191,6 +193,11 @@ public Request timeout(final int connect, final int read) {
return this.base.timeout(connect, read);
}

@Override
public Request sslcontext(final SSLContext context) {
return this.base.sslcontext(context);
}

@Override
public Response fetch() throws IOException {
return this.base.fetch();
Expand Down
55 changes: 41 additions & 14 deletions src/main/java/com/jcabi/http/request/JdkRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import javax.net.ssl.HttpsURLConnection;
import javax.net.ssl.SSLContext;
import lombok.EqualsAndHashCode;
import lombok.ToString;

Expand Down Expand Up @@ -80,33 +82,29 @@ public final class JdkRequest implements Request {
* @checkstyle AnonInnerLength (200 lines)
*/
private static final Wire WIRE = new Wire() {

// @checkstyle ParameterNumber (6 lines)
@Override
public Response send(final Request req, final String home,
final String method,
final Collection<Map.Entry<String, String>> headers,
final InputStream content,
final int connect,
final int read) throws IOException {
final int read,
final SSLContext sslcontext) throws IOException {
final URLConnection raw = new URL(home).openConnection();
if (!(raw instanceof HttpURLConnection)) {
throw new IOException(
String.format(
final String message = String.format(
"'%s' opens %s instead of expected HttpURLConnection",
home, raw.getClass().getName()
)
);
throw new IOException(message);
}
final HttpURLConnection conn = HttpURLConnection.class.cast(raw);
HttpURLConnection conn = null;
try {
conn.setConnectTimeout(connect);
conn.setReadTimeout(read);
conn.setRequestMethod(method);
conn.setUseCaches(false);
conn.setInstanceFollowRedirects(false);
for (final Map.Entry<String, String> header : headers) {
conn.addRequestProperty(header.getKey(), header.getValue());
}
conn = this.createHttpUrlConnection(
raw, connect, read, method, headers, sslcontext
);
if (method.equals(Request.POST) || method.equals(Request.PUT)
|| method.equals(Request.PATCH)) {
conn.setDoOutput(true);
Expand All @@ -131,9 +129,33 @@ public Response send(final Request req, final String home,
exp
);
} finally {
conn.disconnect();
if (conn != null) {
conn.disconnect();
}
}
}
// @checkstyle ParameterNumber (5 lines)
// @checkstyle LineLengthCheck (4 lines)
private HttpURLConnection createHttpUrlConnection(final URLConnection raw,
final int connect, final int read, final String method,
final Collection<Map.Entry<String, String>> headers,
final SSLContext sslcontext) throws IOException {
final HttpURLConnection conn =
HttpURLConnection.class.cast(raw);
conn.setConnectTimeout(connect);
conn.setReadTimeout(read);
conn.setRequestMethod(method);
conn.setUseCaches(false);
conn.setInstanceFollowRedirects(false);
for (final Map.Entry<String, String> header : headers) {
conn.addRequestProperty(header.getKey(), header.getValue());
}
if (sslcontext != null && conn instanceof HttpsURLConnection) {
final HttpsURLConnection sconn = (HttpsURLConnection) conn;
sconn.setSSLSocketFactory(sslcontext.getSocketFactory());
}
return conn;
}
/**
* Fully write the input stream contents to the output stream.
* @param content The content to write
Expand Down Expand Up @@ -268,6 +290,11 @@ public Request timeout(final int connect, final int read) {
return this.base.timeout(connect, read);
}

@Override
public Request sslcontext(final SSLContext context) {
return this.base.sslcontext(context);
}

@Override
public Response fetch() throws IOException {
return this.base.fetch();
Expand Down
Loading