Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/23097 )
Change subject: IMPALA-13235: [Patch 2 of 5] - Add OpenSSL Utility Function to Validate PEM Bundles ...................................................................... Patch Set 9: -Code-Review (3 comments) http://gerrit.cloudera.org:8080/#/c/23097/9/be/src/util/openssl-util-test.cc File be/src/util/openssl-util-test.cc: http://gerrit.cloudera.org:8080/#/c/23097/9/be/src/util/openssl-util-test.cc@300 PS9, Line 300: : /// Asserts a bundle containing a single expired certificate is invalid. : TEST_F(OpenSSLUtilTest, ValidatePemBundleExpired) { : ASSERT_ERROR_MSG(ValidatePemBundle(read_cert(EXPIRED_CERT)), MSG_INVALID); : } I'm guessing that expired cert will be most likely issues met. Can you distinguish expired cert message differently from other invalid cert message? Or, change MSG_INVALID to: "PEM bundle contains at least 1 invalid/expired certificate". http://gerrit.cloudera.org:8080/#/c/23097/9/be/src/util/openssl-util-test.cc@308 PS9, Line 308: "-----BEGIN CERTIFICATE-----\nnot a cert" Make this a variable and reuse it. http://gerrit.cloudera.org:8080/#/c/23097/9/be/src/util/openssl-util-test.cc@353 PS9, Line 353: // Invalid cert is in the middle of the bundle. : EXPECT_STR_CONTAINS(ValidatePemBundle(strings::Substitute("$0$1$2", : read_cert(VALID_CERT_1), read_cert(VALID_CERT_2), : "-----BEGIN CERTIFICATE-----\nnot a cert")).GetDetail(), MSG_UNKNOWN); : : // Invalid cert is last in the bundle. : EXPECT_STR_CONTAINS(ValidatePemBundle(strings::Substitute("$0$1$2", : read_cert(VALID_CERT_1), read_cert(VALID_CERT_2), : "-----BEGIN CERTIFICATE-----\nnot a cert")).GetDetail(), MSG_UNKNOWN); Duplicate tests? Both invalid cert is at last. -- To view, visit http://gerrit.cloudera.org:8080/23097 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1e9f12e854b679ccf3e94a38c2688a7431460a7a Gerrit-Change-Number: 23097 Gerrit-PatchSet: 9 Gerrit-Owner: Jason Fehr <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: gaurav singh <[email protected]> Gerrit-Comment-Date: Mon, 30 Jun 2025 15:25:47 +0000 Gerrit-HasComments: Yes
