From 133a39a81d605256f4ab9beef6a064f85e5de8a8 Mon Sep 17 00:00:00 2001 From: Jacob Su Date: Thu, 15 Aug 2024 11:12:02 +0800 Subject: [PATCH] Config: Add more utest for env config. v6.0.147 (#4142) 1. don't use static variable to store the result; 2. add more UT to handle the multi value and values with whitespaces; related to #4092 https://github.com/ossrs/srs/blob/16e569d82357757ddac6ef91d7a5fe7837319909/trunk/src/app/srs_app_config.cpp#L71-L82 `static SrsConfDirective* dir` removed, this static var here is to avoid the memory leak, I add the `SrsConfDirective` instance to the `env_dirs` directive container, which will destroy itself inside `SrsConfig` destructor. --------- Co-authored-by: winlin --- trunk/doc/CHANGELOG.md | 1 + trunk/src/app/srs_app_config.cpp | 12 ++- trunk/src/app/srs_app_config.hpp | 3 + trunk/src/core/srs_core_version6.hpp | 2 +- trunk/src/utest/srs_utest_config.cpp | 139 +++++++++++++++++++++++++-- 5 files changed, 148 insertions(+), 9 deletions(-) diff --git a/trunk/doc/CHANGELOG.md b/trunk/doc/CHANGELOG.md index 6225a1e4dd..9b9e80fb2a 100644 --- a/trunk/doc/CHANGELOG.md +++ b/trunk/doc/CHANGELOG.md @@ -7,6 +7,7 @@ The changelog for SRS. ## SRS 6.0 Changelog +* v6.0, 2024-08-15, Merge [#4142](https://github.com/ossrs/srs/pull/4142): Config: Add more utest for env config. v6.0.147 (#4142) * v6.0, 2024-08-14, Merge [#4141](https://github.com/ossrs/srs/pull/4141): Live: Crash for invalid live stream state when unmount HTTP. v6.0.146 (#4141) * v6.0, 2024-08-13, Merge [#4092](https://github.com/ossrs/srs/pull/4092): Config: Improve env config to support multi values. v6.0.146 (#4092) * v6.0, 2024-07-27, Merge [#4127](https://github.com/ossrs/srs/pull/4127): Transcode: Support video codec such as h264_qsv and libx265. v6.0.145 (#4127) diff --git a/trunk/src/app/srs_app_config.cpp b/trunk/src/app/srs_app_config.cpp index 06174bddbb..0731f3cd2b 100644 --- a/trunk/src/app/srs_app_config.cpp +++ b/trunk/src/app/srs_app_config.cpp @@ -69,14 +69,18 @@ const char* _srs_version = "XCORE-" RTMP_SIG_SRS_SERVER; #define SRS_OVERWRITE_BY_ENV_FLOAT_SECONDS(key) if (!srs_getenv(key).empty()) return srs_utime_t(::atof(srs_getenv(key).c_str()) * SRS_UTIME_SECONDS) #define SRS_OVERWRITE_BY_ENV_FLOAT_MILLISECONDS(key) if (!srs_getenv(key).empty()) return srs_utime_t(::atof(srs_getenv(key).c_str()) * SRS_UTIME_MILLISECONDS) #define SRS_OVERWRITE_BY_ENV_DIRECTIVE(key) { \ - static SrsConfDirective* dir = NULL; \ + SrsConfDirective* dir = env_cache_->get(key); \ if (!dir && !srs_getenv(key).empty()) { \ std::vector vec = srs_string_split(srs_getenv(key), " "); \ dir = new SrsConfDirective(); \ dir->name = key; \ for (size_t i = 0; i < vec.size(); ++i) { \ - dir->args.push_back(vec[i]); \ + std::string value = vec[i]; \ + if (!value.empty()) { \ + dir->args.push_back(value); \ + } \ } \ + env_cache_->directives.push_back(dir); \ } \ if (dir) return dir; \ } @@ -1345,11 +1349,15 @@ SrsConfig::SrsConfig() root = new SrsConfDirective(); root->conf_line = 0; root->name = "root"; + + env_cache_ = new SrsConfDirective(); + env_cache_->name = "env_cache_"; } SrsConfig::~SrsConfig() { srs_freep(root); + srs_freep(env_cache_); } void SrsConfig::subscribe(ISrsReloadHandler* handler) diff --git a/trunk/src/app/srs_app_config.hpp b/trunk/src/app/srs_app_config.hpp index afa700a0c8..28aec179db 100644 --- a/trunk/src/app/srs_app_config.hpp +++ b/trunk/src/app/srs_app_config.hpp @@ -305,6 +305,9 @@ class SrsConfig protected: // The directive root. SrsConfDirective* root; +private: + // The cache for parsing the config from environment variables. + SrsConfDirective* env_cache_; // Reload section private: // The reload subscribers, when reload, callback all handlers. diff --git a/trunk/src/core/srs_core_version6.hpp b/trunk/src/core/srs_core_version6.hpp index c44a46ff03..235267d83c 100644 --- a/trunk/src/core/srs_core_version6.hpp +++ b/trunk/src/core/srs_core_version6.hpp @@ -9,6 +9,6 @@ #define VERSION_MAJOR 6 #define VERSION_MINOR 0 -#define VERSION_REVISION 146 +#define VERSION_REVISION 147 #endif diff --git a/trunk/src/utest/srs_utest_config.cpp b/trunk/src/utest/srs_utest_config.cpp index c56e42efeb..8999dabfeb 100644 --- a/trunk/src/utest/srs_utest_config.cpp +++ b/trunk/src/utest/srs_utest_config.cpp @@ -5059,12 +5059,11 @@ VOID TEST(ConfigEnvTest, CheckEnvValuesHooks) } if (true) { - SrsSetEnvConfig(hooks, "SRS_VHOST_HTTP_HOOKS_ON_CONNECT", "http://server/api/connect https://server2/api/connect2"); + SrsSetEnvConfig(hooks, "SRS_VHOST_HTTP_HOOKS_ON_CONNECT", "http://server/api/connect"); SrsConfDirective* dir = conf.get_vhost_on_connect("__defaultVhost__"); ASSERT_TRUE(dir != NULL); - ASSERT_EQ((int)dir->args.size(), 2); + ASSERT_EQ((int)dir->args.size(), 1); ASSERT_STREQ("http://server/api/connect", dir->arg0().c_str()); - ASSERT_STREQ("https://server2/api/connect2", dir->arg1().c_str()); } if (true) { @@ -5076,12 +5075,11 @@ VOID TEST(ConfigEnvTest, CheckEnvValuesHooks) } if (true) { - SrsSetEnvConfig(hooks, "SRS_VHOST_HTTP_HOOKS_ON_PUBLISH", "http://server/api/publish http://server/api/publish2"); + SrsSetEnvConfig(hooks, "SRS_VHOST_HTTP_HOOKS_ON_PUBLISH", "http://server/api/publish"); SrsConfDirective* dir = conf.get_vhost_on_publish("__defaultVhost__"); ASSERT_TRUE(dir != NULL); - ASSERT_EQ((int)dir->args.size(), 2); + ASSERT_EQ((int)dir->args.size(), 1); ASSERT_STREQ("http://server/api/publish", dir->arg0().c_str()); - ASSERT_STREQ("http://server/api/publish2", dir->arg1().c_str()); } if (true) { @@ -5132,3 +5130,132 @@ VOID TEST(ConfigEnvTest, CheckEnvValuesHooks) ASSERT_STREQ("http://server/api/hls_notify", dir->arg0().c_str()); } } + +VOID TEST(ConfigEnvTest, CheckEnvValuesHooksMultiValues) +{ + MockSrsConfig conf; + + if (true) { + SrsSetEnvConfig(hooks, "SRS_VHOST_HTTP_HOOKS_ON_CONNECT", "http://server/api/connect https://server2/api/connect2"); + + SrsConfDirective* dir = conf.get_vhost_on_connect("__defaultVhost__"); + ASSERT_TRUE(dir != NULL); + ASSERT_EQ((int)dir->args.size(), 2); + ASSERT_STREQ("http://server/api/connect", dir->arg0().c_str()); + ASSERT_STREQ("https://server2/api/connect2", dir->arg1().c_str()); + } + + if (true) { + SrsSetEnvConfig(hooks, "SRS_VHOST_HTTP_HOOKS_ON_CLOSE", "http://server/api/close close2 close3"); + SrsConfDirective* dir = conf.get_vhost_on_close("__defaultVhost__"); + ASSERT_TRUE(dir != NULL); + ASSERT_TRUE((int)dir->args.size() == 3); + ASSERT_STREQ("http://server/api/close", dir->arg0().c_str()); + ASSERT_STREQ("close2", dir->arg1().c_str()); + ASSERT_STREQ("close3", dir->arg2().c_str()); + } + + if (true) { + SrsSetEnvConfig(hooks, "SRS_VHOST_HTTP_HOOKS_ON_PUBLISH", "http://server/api/publish http://server/api/publish2"); + SrsConfDirective* dir = conf.get_vhost_on_publish("__defaultVhost__"); + ASSERT_TRUE(dir != NULL); + ASSERT_EQ((int)dir->args.size(), 2); + ASSERT_STREQ("http://server/api/publish", dir->arg0().c_str()); + ASSERT_STREQ("http://server/api/publish2", dir->arg1().c_str()); + } + + if (true) { + SrsSetEnvConfig(hooks, "SRS_VHOST_HTTP_HOOKS_ON_UNPUBLISH", "http://server/api/unpublish 2"); + SrsConfDirective* dir = conf.get_vhost_on_unpublish("__defaultVhost__"); + ASSERT_TRUE(dir != NULL); + ASSERT_TRUE((int)dir->args.size() == 2); + ASSERT_STREQ("http://server/api/unpublish", dir->arg0().c_str()); + ASSERT_STREQ("2", dir->arg1().c_str()); + } + + if (true) { + SrsSetEnvConfig(hooks, "SRS_VHOST_HTTP_HOOKS_ON_PLAY", "http://server/api/play http://server/api/play2"); + SrsConfDirective* dir = conf.get_vhost_on_play("__defaultVhost__"); + ASSERT_TRUE(dir != NULL); + ASSERT_TRUE((int)dir->args.size() == 2); + ASSERT_STREQ("http://server/api/play", dir->arg0().c_str()); + ASSERT_STREQ("http://server/api/play2", dir->arg1().c_str()); + } + + if (true) { + SrsSetEnvConfig(hooks, "SRS_VHOST_HTTP_HOOKS_ON_STOP", "http://server/api/stop http://server/api/stop2"); + SrsConfDirective* dir = conf.get_vhost_on_stop("__defaultVhost__"); + ASSERT_TRUE(dir != NULL); + ASSERT_TRUE((int)dir->args.size() == 2); + ASSERT_STREQ("http://server/api/stop", dir->arg0().c_str()); + ASSERT_STREQ("http://server/api/stop2", dir->arg1().c_str()); + } + + if (true) { + SrsSetEnvConfig(hooks, "SRS_VHOST_HTTP_HOOKS_ON_DVR", "http://server/api/dvr http://server/api/dvr2"); + SrsConfDirective* dir = conf.get_vhost_on_dvr("__defaultVhost__"); + ASSERT_TRUE(dir != NULL); + ASSERT_TRUE((int)dir->args.size() == 2); + ASSERT_STREQ("http://server/api/dvr", dir->arg0().c_str()); + ASSERT_STREQ("http://server/api/dvr2", dir->arg1().c_str()); + } + + if (true) { + SrsSetEnvConfig(hooks, "SRS_VHOST_HTTP_HOOKS_ON_HLS", "http://server/api/hls http://server/api/hls2"); + SrsConfDirective* dir = conf.get_vhost_on_hls("__defaultVhost__"); + ASSERT_TRUE(dir != NULL); + ASSERT_TRUE((int)dir->args.size() == 2); + ASSERT_STREQ("http://server/api/hls", dir->arg0().c_str()); + ASSERT_STREQ("http://server/api/hls2", dir->arg1().c_str()); + } + + if (true) { + SrsSetEnvConfig(hooks, "SRS_VHOST_HTTP_HOOKS_ON_HLS_NOTIFY", "http://server/api/hls_notify http://server/api/hls_notify2"); + SrsConfDirective* dir = conf.get_vhost_on_hls_notify("__defaultVhost__"); + ASSERT_TRUE(dir != NULL); + ASSERT_TRUE((int)dir->args.size() == 2); + ASSERT_STREQ("http://server/api/hls_notify", dir->arg0().c_str()); + ASSERT_STREQ("http://server/api/hls_notify2", dir->arg1().c_str()); + } +} + +VOID TEST(ConfigEnvTest, CheckEnvValuesHooksWithWhitespaces) +{ + MockSrsConfig conf; + + if (true) { + SrsSetEnvConfig(hooks, "SRS_VHOST_HTTP_HOOKS_ON_PUBLISH", "http://server/api/publish http://server/api/publish2"); + SrsConfDirective* dir = conf.get_vhost_on_publish("__defaultVhost__"); + ASSERT_TRUE(dir != NULL); + ASSERT_EQ((int)dir->args.size(), 2); + ASSERT_STREQ("http://server/api/publish", dir->arg0().c_str()); + ASSERT_STREQ("http://server/api/publish2", dir->arg1().c_str()); + } + + if (true) { + SrsSetEnvConfig(hooks, "SRS_VHOST_HTTP_HOOKS_ON_UNPUBLISH", "http://server/api/unpublish "); + SrsConfDirective* dir = conf.get_vhost_on_unpublish("__defaultVhost__"); + ASSERT_TRUE(dir != NULL); + ASSERT_TRUE((int)dir->args.size() == 1); + ASSERT_STREQ("http://server/api/unpublish", dir->arg0().c_str()); + } + + if (true) { + SrsSetEnvConfig(hooks, "SRS_VHOST_HTTP_HOOKS_ON_PLAY", " http://server/api/play play2 play3 "); + SrsConfDirective* dir = conf.get_vhost_on_play("__defaultVhost__"); + ASSERT_TRUE(dir != NULL); + ASSERT_TRUE((int)dir->args.size() == 3); + ASSERT_STREQ("http://server/api/play", dir->arg0().c_str()); + ASSERT_STREQ("play2", dir->arg1().c_str()); + ASSERT_STREQ("play3", dir->arg2().c_str()); + } + + if (true) { + SrsSetEnvConfig(hooks, "SRS_VHOST_HTTP_HOOKS_ON_DVR", " dvr"); + SrsConfDirective* dir = conf.get_vhost_on_dvr("__defaultVhost__"); + ASSERT_TRUE(dir != NULL); + ASSERT_TRUE((int)dir->args.size() == 1); + ASSERT_STREQ("dvr", dir->arg0().c_str()); + } + +}