diff --git a/dbms/src/Common/TiFlashSecurity.h b/dbms/src/Common/TiFlashSecurity.h index c151a308f29..ad1805b5444 100644 --- a/dbms/src/Common/TiFlashSecurity.h +++ b/dbms/src/Common/TiFlashSecurity.h @@ -56,18 +56,18 @@ struct TiFlashSecurityConfig bool miss_key_path = true; if (config.has("security.ca_path")) { - ca_path = config.getString("security.ca_path"); - miss_ca_path = false; + ca_path = Poco::trim(config.getString("security.ca_path")); + miss_ca_path = ca_path.empty(); } if (config.has("security.cert_path")) { - cert_path = config.getString("security.cert_path"); - miss_cert_path = false; + cert_path = Poco::trim(config.getString("security.cert_path")); + miss_cert_path = cert_path.empty(); } if (config.has("security.key_path")) { - key_path = config.getString("security.key_path"); - miss_key_path = false; + key_path = Poco::trim(config.getString("security.key_path")); + miss_key_path = key_path.empty(); } if (miss_ca_path && miss_cert_path && miss_key_path) { diff --git a/dbms/src/Common/tests/gtest_tiflash_security.cpp b/dbms/src/Common/tests/gtest_tiflash_security.cpp index 5e7cdc2ceca..d5a0421acd9 100644 --- a/dbms/src/Common/tests/gtest_tiflash_security.cpp +++ b/dbms/src/Common/tests/gtest_tiflash_security.cpp @@ -13,17 +13,14 @@ // limitations under the License. #include +#include +#include #include #include -namespace DB +namespace DB::tests { -namespace tests -{ -class TestTiFlashSecurity : public ext::Singleton -{ -}; TEST(TestTiFlashSecurity, Config) { @@ -50,5 +47,77 @@ TEST(TestTiFlashSecurity, Config) ASSERT_EQ((int)config.allowed_common_names.count("abc"), 1); ASSERT_EQ((int)config.allowed_common_names.count("efg"), 1); } -} // namespace tests -} // namespace DB + +TEST(TiFlashSecurityTest, EmptyConfig) +try +{ + const auto log = Logger::get(); + + for (const auto & c : Strings{ + // empty strings + R"([security] +ca_path="" +cert_path="" +key_path="")", + // non-empty strings with space only + R"([security] +ca_path=" " +cert_path="" +key_path="")", + }) + { + SCOPED_TRACE(fmt::format("case: {}", c)); + auto new_config = loadConfigFromString(c); + TiFlashSecurityConfig tiflash_config(*new_config, log); + ASSERT_FALSE(tiflash_config.has_tls_config); + } +} +CATCH + +TEST(TiFlashSecurityTest, InvalidConfig) +try +{ + const auto log = Logger::get(); + + for (const auto & c : Strings{ + // only a part of ssl path is set + R"([security] +ca_path="security/ca.pem" +cert_path="" +key_path="")", + R"([security] +ca_path="" +cert_path="security/cert.pem" +key_path="")", + R"([security] +ca_path="" +cert_path="" +key_path="security/key.pem")", + R"([security] +ca_path="" +cert_path="security/cert.pem" +key_path="security/key.pem")", + // comment out + R"([security] +ca_path="security/ca.pem" +#cert_path="security/cert.pem" +key_path="security/key.pem")", + }) + { + SCOPED_TRACE(fmt::format("case: {}", c)); + auto new_config = loadConfigFromString(c); + try + { + TiFlashSecurityConfig tiflash_config(*new_config, log); + ASSERT_FALSE(true) << "should raise exception"; + } + catch (Exception & e) + { + // the error code must be INVALID_CONFIG_PARAMETER + ASSERT_EQ(e.code(), ErrorCodes::INVALID_CONFIG_PARAMETER); + } + } +} +CATCH + +} // namespace DB::tests