Skip to content

Commit

Permalink
MultiAddressUrl client fails to send http request after https request (
Browse files Browse the repository at this point in the history
…#962)

Motivation:

If `MultiAddressUrl` was used to send an HTTPS request over secure
connection, all other requests to new non-secure servers will fail
with `DecoderException`. It happens because we incorrectly copy the
`HttpClientBuilder` template. We did not create a new instance of
`TcpClientConfig` and `HttpConfig`.

Modifications:

- Add copy ctor for `HttpConfig`;
- Use copy ctors for `TcpClientConfig` and `HttpConfig` in
`HttpClientConfig`;
- Add test to verify that `MultiAddressUrl` client can connect to
insecure server after connecting to a secure server and vice versa;

Result:

`HttpClientConfig` makes a full copy of `TcpClientConfig` and
`HttpConfig` that prevents the `HttpClientBuilder` template from
modifications. `MultiAddressUrl` client correctly copies the
builder template without internal state modifications.
  • Loading branch information
idelpivnitskiy authored Mar 10, 2020
1 parent cc466dc commit 8e4f570
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright © 2018-2019 Apple Inc. and the ServiceTalk project authors
* Copyright © 2018-2020 Apple Inc. and the ServiceTalk project authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -32,8 +32,8 @@ final class HttpClientConfig {
}

HttpClientConfig(final HttpClientConfig from) {
tcpConfig = from.tcpConfig();
protocolConfigs = from.protocolConfigs();
tcpConfig = new TcpClientConfig(from.tcpConfig());
protocolConfigs = new HttpConfig(from.protocolConfigs());
connectAddress = from.connectAddress;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright © 2019 Apple Inc. and the ServiceTalk project authors
* Copyright © 2019-2020 Apple Inc. and the ServiceTalk project authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -29,10 +29,22 @@

final class HttpConfig {
@Nullable
private H1ProtocolConfig h1Config = h1Default();
private H1ProtocolConfig h1Config;
@Nullable
private H2ProtocolConfig h2Config;
private List<String> supportedAlpnProtocols = emptyList();
private List<String> supportedAlpnProtocols;

HttpConfig() {
h1Config = h1Default();
h2Config = null;
supportedAlpnProtocols = emptyList();
}

HttpConfig(final HttpConfig from) {
this.h1Config = from.h1Config;
this.h2Config = from.h2Config;
this.supportedAlpnProtocols = from.supportedAlpnProtocols;
}

@Nullable
H1ProtocolConfig h1Config() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright © 2018-2019 Apple Inc. and the ServiceTalk project authors
* Copyright © 2018-2020 Apple Inc. and the ServiceTalk project authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -202,6 +202,30 @@ public void multiAddressClientWithSslToSecureServer() throws Exception {
}
}

@Test
public void multiAddressClientToSecureServerThenToNonSecureServer() throws Exception {
try (BlockingHttpClient client = HttpClients.forMultiAddressUrl()
.secure((hap, config) -> config.disableHostnameVerification()
.trustManager(DefaultTestCerts::loadMutualAuthCaPem))
.buildBlocking()) {
testRequestResponse(client, secureRequestTarget, true);
resetMocks();
testRequestResponse(client, requestTarget, false);
}
}

@Test
public void multiAddressClientToNonSecureServerThenToSecureServer() throws Exception {
try (BlockingHttpClient client = HttpClients.forMultiAddressUrl()
.secure((hap, config) -> config.disableHostnameVerification()
.trustManager(DefaultTestCerts::loadMutualAuthCaPem))
.buildBlocking()) {
testRequestResponse(client, requestTarget, false);
resetMocks();
testRequestResponse(client, secureRequestTarget, true);
}
}

private static void testRequestResponse(final BlockingHttpClient client, final String requestTarget,
final boolean secure) throws Exception {
final HttpResponse response = client.request(client.get(requestTarget));
Expand Down

0 comments on commit 8e4f570

Please sign in to comment.