Skip to content

Commit 0a3c034

Browse files
authored
s2a: Correct type of exception thrown (#11588)
* throw IllegalArgumentException in ProtoUtil. * throw exception in TrustManager in more standard way. * handle IllegalArgumentException in SslContextFactory. * Don't throw error on unknown TLS version.
1 parent 2aae68e commit 0a3c034

File tree

5 files changed

+24
-19
lines changed

5 files changed

+24
-19
lines changed

s2a/src/main/java/io/grpc/s2a/internal/handshaker/ProtoUtil.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ final class ProtoUtil {
2727
*
2828
* @param tlsVersion the {@link TLSVersion} object to be converted.
2929
* @return a {@link String} representation of the TLS version.
30-
* @throws AssertionError if the {@code tlsVersion} is not one of the supported TLS versions.
30+
* @throws IllegalArgumentException if the {@code tlsVersion} is not one of
31+
* the supported TLS versions.
3132
*/
3233
@VisibleForTesting
3334
static String convertTlsProtocolVersion(TLSVersion tlsVersion) {
@@ -41,7 +42,7 @@ static String convertTlsProtocolVersion(TLSVersion tlsVersion) {
4142
case TLS_VERSION_1_0:
4243
return "TLSv1";
4344
default:
44-
throw new AssertionError(
45+
throw new IllegalArgumentException(
4546
String.format("TLS version %d is not supported.", tlsVersion.getNumber()));
4647
}
4748
}
@@ -62,7 +63,11 @@ static ImmutableSet<String> buildTlsProtocolVersionSet(
6263
}
6364
if (versionNumber >= minTlsVersion.getNumber()
6465
&& versionNumber <= maxTlsVersion.getNumber()) {
65-
tlsVersions.add(convertTlsProtocolVersion(tlsVersion));
66+
try {
67+
tlsVersions.add(convertTlsProtocolVersion(tlsVersion));
68+
} catch (IllegalArgumentException e) {
69+
continue;
70+
}
6671
}
6772
}
6873
return tlsVersions.build();

s2a/src/main/java/io/grpc/s2a/internal/handshaker/S2ATrustManager.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,10 +120,10 @@ private void checkPeerTrusted(X509Certificate[] chain, boolean isCheckingClientC
120120
SessionResp resp;
121121
try {
122122
resp = stub.send(reqBuilder.build());
123-
} catch (IOException | InterruptedException e) {
124-
if (e instanceof InterruptedException) {
125-
Thread.currentThread().interrupt();
126-
}
123+
} catch (IOException e) {
124+
throw new CertificateException("Failed to send request to S2A.", e);
125+
} catch (InterruptedException e) {
126+
Thread.currentThread().interrupt();
127127
throw new CertificateException("Failed to send request to S2A.", e);
128128
}
129129
if (resp.hasStatus() && resp.getStatus().getCode() != 0) {

s2a/src/main/java/io/grpc/s2a/internal/handshaker/SslContextFactory.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,11 +138,13 @@ private static void configureSslContextWithClientTlsConfiguration(
138138
NoSuchAlgorithmException,
139139
UnrecoverableKeyException {
140140
sslContextBuilder.keyManager(createKeylessManager(clientTlsConfiguration));
141-
ImmutableSet<String> tlsVersions =
141+
ImmutableSet<String> tlsVersions;
142+
tlsVersions =
142143
ProtoUtil.buildTlsProtocolVersionSet(
143144
clientTlsConfiguration.getMinTlsVersion(), clientTlsConfiguration.getMaxTlsVersion());
144145
if (tlsVersions.isEmpty()) {
145-
throw new S2AConnectionException("Set of TLS versions received from S2A server is empty.");
146+
throw new S2AConnectionException("Set of TLS versions received from S2A server is"
147+
+ " empty or not supported.");
146148
}
147149
sslContextBuilder.protocols(tlsVersions);
148150
}

s2a/src/test/java/io/grpc/s2a/internal/handshaker/ProtoUtilTest.java

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,9 @@ public void convertTlsProtocolVersion_success() {
4646

4747
@Test
4848
public void convertTlsProtocolVersion_withUnknownTlsVersion_fails() {
49-
AssertionError expected =
49+
IllegalArgumentException expected =
5050
assertThrows(
51-
AssertionError.class,
51+
IllegalArgumentException.class,
5252
() -> ProtoUtil.convertTlsProtocolVersion(TLSVersion.TLS_VERSION_UNSPECIFIED));
5353
expect.that(expected).hasMessageThat().isEqualTo("TLS version 0 is not supported.");
5454
}
@@ -79,12 +79,10 @@ public void buildTlsProtocolVersionSet_success() {
7979

8080
@Test
8181
public void buildTlsProtocolVersionSet_failure() {
82-
AssertionError expected =
83-
assertThrows(
84-
AssertionError.class,
85-
() ->
86-
ProtoUtil.buildTlsProtocolVersionSet(
87-
TLSVersion.TLS_VERSION_UNSPECIFIED, TLSVersion.TLS_VERSION_1_3));
88-
expect.that(expected).hasMessageThat().isEqualTo("TLS version 0 is not supported.");
82+
expect
83+
.that(
84+
ProtoUtil.buildTlsProtocolVersionSet(
85+
TLSVersion.TLS_VERSION_UNSPECIFIED, TLSVersion.TLS_VERSION_1_3))
86+
.isEqualTo(ImmutableSet.of("TLSv1", "TLSv1.1", "TLSv1.2", "TLSv1.3"));
8987
}
9088
}

s2a/src/test/java/io/grpc/s2a/internal/handshaker/SslContextFactoryTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ public void createForClient_getsBadTlsVersionsFromServer_throwsError() throws Ex
142142

143143
assertThat(expected)
144144
.hasMessageThat()
145-
.contains("Set of TLS versions received from S2A server is empty.");
145+
.contains("Set of TLS versions received from S2A server is empty or not supported.");
146146
}
147147

148148
@Test

0 commit comments

Comments
 (0)