Only log SSL Certificate verification being disabled warrning when it's actually disabled (#6766)

* Fix: Warn about SSL Certificate verification being disabled only when it's actually disabled

Only Boolean.TRUE disables SSL Certificate Validation, so both Boolean.FALSE and null should skip the warning message

* Add tests and changelog entry

* Address feedback

---------

Co-authored-by: Bruno Melo <bsilva.melo@gmail.com>

Author: zoewangg

.changes/next-release/bugfix-AWSCRTbasedS3Client-18099d7.json

@@ -0,0 +1,6 @@
+{
+    "type": "bugfix",
+    "category": "AWS CRT-based S3 Client",
+    "contributor": "bsmelo",
+    "description": "Only log `SSL Certificate verification is disabled` warning if trustAllCertificatesEnabled is set to true."
+}

services/s3/pom.xml

@@ -129,6 +129,12 @@
             <version>${awsjavasdk.version}</version>
         </dependency>
         <!-- Test Dependencies -->
+        <dependency>
+            <groupId>software.amazon.awssdk</groupId>
+            <artifactId>test-utils</artifactId>
+            <version>${awsjavasdk.version}</version>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <artifactId>commons-io</artifactId>
             <groupId>commons-io</groupId>

services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3NativeClientConfiguration.java

@@ -76,11 +76,14 @@ public S3NativeClientConfiguration(Builder builder) {
             TlsContextOptions.createDefaultClient()
                              .withCipherPreference(TlsCipherPreference.TLS_CIPHER_SYSTEM_DEFAULT);
 
-        if (builder.httpConfiguration != null
-            && builder.httpConfiguration.trustAllCertificatesEnabled() != null) {
-            log.warn(() -> "SSL Certificate verification is disabled. "
-                           + "This is not a safe setting and should only be used for testing.");
-            clientTlsContextOptions.withVerifyPeer(!builder.httpConfiguration.trustAllCertificatesEnabled());
+        if (builder.httpConfiguration != null &&
+            builder.httpConfiguration.trustAllCertificatesEnabled() != null) {
+            Boolean trustAllCertificatesEnabled = builder.httpConfiguration.trustAllCertificatesEnabled();
+            if (Boolean.TRUE.equals(trustAllCertificatesEnabled)) {
+                log.warn(() -> "SSL Certificate verification is disabled. "
+                               + "This is not a safe setting and should only be used for testing.");
+            }
+            clientTlsContextOptions.withVerifyPeer(!trustAllCertificatesEnabled);
         }
         this.tlsContext = new TlsContext(clientTlsContextOptions);
         this.credentialProviderAdapter = new CrtCredentialsProviderAdapter(

services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crt/S3NativeClientConfigurationTest.java

@@ -0,0 +1,76 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License").
+ * You may not use this file except in compliance with the License.
+ * A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ * or in the "license" file accompanying this file. This file is distributed
+ * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ * express or implied. See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+
+package software.amazon.awssdk.services.s3.internal.crt;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.stream.Stream;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
+import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider;
+import software.amazon.awssdk.services.s3.crt.S3CrtHttpConfiguration;
+import software.amazon.awssdk.testutils.LogCaptor;
+
+class S3NativeClientConfigurationTest {
+
+    private static final String SSL_WARNING = "SSL Certificate verification is disabled.";
+
+    @Test
+    void build_whenTrustAllCertificatesTrue_shouldLogWarning() {
+        try (LogCaptor logCaptor = LogCaptor.create();
+             S3NativeClientConfiguration config = buildConfig(
+                 S3CrtHttpConfiguration.builder().trustAllCertificatesEnabled(true).build())) {
+
+            assertThat(logCaptor.loggedEvents())
+                .anySatisfy(event -> assertThat(event.getMessage().getFormattedMessage()).contains(SSL_WARNING));
+        }
+    }
+
+    @ParameterizedTest
+    @MethodSource("noWarningConfigurations")
+    void build_whenTrustAllCertificatesNotTrue_shouldNotLogWarning(S3CrtHttpConfiguration httpConfig) {
+        try (LogCaptor logCaptor = LogCaptor.create();
+             S3NativeClientConfiguration config = buildConfig(httpConfig)) {
+
+            assertThat(logCaptor.loggedEvents())
+                .noneSatisfy(event -> assertThat(event.getMessage().getFormattedMessage()).contains(SSL_WARNING));
+        }
+    }
+
+    private static Stream<Arguments> noWarningConfigurations() {
+        return Stream.of(
+            Arguments.of(S3CrtHttpConfiguration.builder().trustAllCertificatesEnabled(false).build()),
+            Arguments.of(S3CrtHttpConfiguration.builder().build()),
+            Arguments.of((S3CrtHttpConfiguration) null)
+        );
+    }
+
+    private S3NativeClientConfiguration buildConfig(S3CrtHttpConfiguration httpConfig) {
+        S3NativeClientConfiguration.Builder builder =
+            S3NativeClientConfiguration.builder()
+                                       .signingRegion("us-east-1")
+                                       .credentialsProvider(
+                                           StaticCredentialsProvider.create(
+                                               AwsBasicCredentials.create("foo", "bar")));
+        if (httpConfig != null) {
+            builder.httpConfiguration(httpConfig);
+        }
+        return builder.build();
+    }
+}