diff --git a/plugins/experimental/jax_fingerprint/plugin.cc b/plugins/experimental/jax_fingerprint/plugin.cc index 374d66faecb..0bd8c327948 100644 --- a/plugins/experimental/jax_fingerprint/plugin.cc +++ b/plugins/experimental/jax_fingerprint/plugin.cc @@ -373,16 +373,16 @@ TSPluginInit(int argc, char const **argv) return; } - PluginConfig *config = new PluginConfig(); - config->plugin_type = PluginType::GLOBAL; + auto owned_config = std::make_unique(); + owned_config->plugin_type = PluginType::GLOBAL; - if (!read_config_option(argc, argv, *config)) { + if (!read_config_option(argc, argv, *owned_config)) { TSError("[%s] Failed to parse options.", PLUGIN_NAME); return; } - if (!config->log_filename.empty()) { - if (!create_log_file(config->log_filename, config->log_handle)) { + if (!owned_config->log_filename.empty()) { + if (!create_log_file(owned_config->log_filename, owned_config->log_handle)) { TSError("[%s] Failed to create log.", PLUGIN_NAME); return; } else { @@ -390,6 +390,17 @@ TSPluginInit(int argc, char const **argv) } } + // Reserve the index before registering the log field, so that every failure exit happens while the + // configuration is still owned here and nothing has taken a reference to it yet. + if (reserve_user_arg(*owned_config) == TS_ERROR) { + TSError("[%s] Failed to reserve user arg index.", PLUGIN_NAME); + return; + } + + // A global plugin's configuration lives for the life of the process: the log field callback and the + // continuation below both keep a reference to it, so release it from the unique_ptr here. + PluginConfig *config = owned_config.release(); + if (!config->log_symbol.empty()) { std::string name = "jax_fingerprint-"; name += config->method.name; @@ -412,11 +423,6 @@ TSPluginInit(int argc, char const **argv) TSLogIntUnmarshal); } - if (reserve_user_arg(*config) == TS_ERROR) { - TSError("[%s] Failed to reserve user arg index.", PLUGIN_NAME); - return; - } - TSCont cont = TSContCreate(main_handler, nullptr); TSContDataSet(cont, config); if (config->method.on_client_hello) { @@ -445,19 +451,17 @@ TSReturnCode TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSED */, int /* errbuf_size ATS_UNUSED */) { Dbg(dbg_ctl, "New instance for client matching %s to %s", argv[0], argv[1]); - auto config = new PluginConfig(); + auto config = std::make_unique(); config->plugin_type = PluginType::REMAP; // Parse parameters if (!read_config_option(argc - 1, const_cast(argv + 1), *config)) { - delete config; Dbg(dbg_ctl, "Bad arguments"); return TS_ERROR; } if (!config->log_symbol.empty()) { TSError("[%s] --log-field is not supported in remap.config. Use it in plugin.config instead.", PLUGIN_NAME); - delete config; return TS_ERROR; } @@ -476,22 +480,25 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSE return TS_ERROR; } + // Past here the instance handle owns the configuration and TSRemapDeleteInstance releases it. + PluginConfig *instance = config.release(); + // Create continuation - if (config->standalone) { + if (instance->standalone) { Dbg(dbg_ctl, "Standalone mode. Adding hooks."); - config->handler = TSContCreate(main_handler, nullptr); - if (config->method.on_client_hello) { - TSHttpHookAdd(TS_SSL_CLIENT_HELLO_HOOK, config->handler); + instance->handler = TSContCreate(main_handler, nullptr); + if (instance->method.on_client_hello) { + TSHttpHookAdd(TS_SSL_CLIENT_HELLO_HOOK, instance->handler); } - if (config->method.type == Method::Type::CONNECTION_BASED) { - TSHttpHookAdd(TS_VCONN_CLOSE_HOOK, config->handler); + if (instance->method.type == Method::Type::CONNECTION_BASED) { + TSHttpHookAdd(TS_VCONN_CLOSE_HOOK, instance->handler); } else { - TSHttpHookAdd(TS_HTTP_TXN_CLOSE_HOOK, config->handler); + TSHttpHookAdd(TS_HTTP_TXN_CLOSE_HOOK, instance->handler); } - TSContDataSet(config->handler, config); + TSContDataSet(instance->handler, instance); } - *ih = static_cast(config); + *ih = static_cast(instance); return TS_SUCCESS; } diff --git a/plugins/experimental/maxmind_acl/mmdb.cc b/plugins/experimental/maxmind_acl/mmdb.cc index 26aa8070472..4b41c459caa 100644 --- a/plugins/experimental/maxmind_acl/mmdb.cc +++ b/plugins/experimental/maxmind_acl/mmdb.cc @@ -79,10 +79,12 @@ Acl::init(char const *filename) } // Associate our config file with remap.config or .yaml if possible to be able to initiate reloads - TSMgmtString result; + TSMgmtString result = nullptr; const char *var_name = "proxy.config.url_remap_yaml.filename"; if (TS_SUCCESS != TSMgmtStringGet(var_name, &result) || TS_SUCCESS != TSMgmtConfigFileAdd(result, configloc.c_str())) { // Fall back to remap.config + TSfree(result); + result = nullptr; var_name = "proxy.config.url_remap.filename"; if (TS_SUCCESS != TSMgmtStringGet(var_name, &result)) { TSWarning("[%s] Could not retrieve remap filename", PLUGIN_NAME); @@ -90,6 +92,7 @@ Acl::init(char const *filename) TSWarning("[%s] Error adding mgmt config file", PLUGIN_NAME); } } + TSfree(result); // Find our database name and convert to full path as needed status = loaddb(maxmind["database"]); diff --git a/plugins/experimental/stale_response/stale_response.cc b/plugins/experimental/stale_response/stale_response.cc index 80b27082e61..519ab8b8a74 100644 --- a/plugins/experimental/stale_response/stale_response.cc +++ b/plugins/experimental/stale_response/stale_response.cc @@ -1070,6 +1070,10 @@ parse_args(int argc, char const *argv[]) plugin_config->log_info.stale_if_error = true; break; case 'd': + // The option may be repeated; release the previously duplicated name first. + if (plugin_config->log_info.filename != PLUGIN_TAG) { + free(const_cast(plugin_config->log_info.filename)); + } plugin_config->log_info.filename = strdup(optarg); break; diff --git a/plugins/experimental/uri_signing/config.cc b/plugins/experimental/uri_signing/config.cc index 36435b9db5f..27372790d56 100644 --- a/plugins/experimental/uri_signing/config.cc +++ b/plugins/experimental/uri_signing/config.cc @@ -282,6 +282,8 @@ read_config_from_json(json_t *const issuer_json) if (id_json) { id = json_string_value(id_json); if (id) { + /* An earlier issuer may have set an id; free it so it is not leaked. Last issuer wins. */ + free(cfg->id); cfg->id = static_cast(malloc(strlen(id) + 1)); strcpy(cfg->id, id); PluginDebug("Found Id in the config: %s", cfg->id); diff --git a/plugins/regex_revalidate/regex_revalidate.cc b/plugins/regex_revalidate/regex_revalidate.cc index 5e8d8cf2f7b..e3605b70005 100644 --- a/plugins/regex_revalidate/regex_revalidate.cc +++ b/plugins/regex_revalidate/regex_revalidate.cc @@ -778,6 +778,7 @@ TSPluginInit(int argc, const char *argv[]) while ((c = getopt_long(argc, (char *const *)argv, "c:l:f:m:", longopts, nullptr)) != -1) { switch (c) { case 'c': + TSfree(pstate->config_path); // An option can be repeated, so the earlier value is not leaked pstate->config_path = TSstrdup(optarg); break; case 'l': @@ -790,9 +791,11 @@ TSPluginInit(int argc, const char *argv[]) disable_timed_reload = true; break; case 'f': + TSfree(pstate->state_path); pstate->state_path = make_state_path(optarg); break; case 'm': + TSfree(pstate->match_header); pstate->match_header = TSstrdup(optarg); break; default: diff --git a/plugins/remap_purge/remap_purge.cc b/plugins/remap_purge/remap_purge.cc index fd0b8198fbe..eba0fce5ece 100644 --- a/plugins/remap_purge/remap_purge.cc +++ b/plugins/remap_purge/remap_purge.cc @@ -287,17 +287,21 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSE purge->allow_get = true; break; case 'h': + TSfree(purge->header); // An option can be repeated, so the earlier value is not leaked purge->header = TSstrdup(optarg); purge->header_len = strlen(purge->header); break; case 'i': + TSfree(purge->id); purge->id = TSstrdup(optarg); break; case 's': + TSfree(purge->secret); purge->secret = TSstrdup(optarg); purge->secret_len = strlen(purge->secret); break; case 'f': + TSfree(purge->state_file); purge->state_file = make_state_path(optarg); break; } diff --git a/plugins/xdebug/xdebug.cc b/plugins/xdebug/xdebug.cc index 32cd8631372..d228b89e4da 100644 --- a/plugins/xdebug/xdebug.cc +++ b/plugins/xdebug/xdebug.cc @@ -947,6 +947,7 @@ TSPluginInit(int argc, const char *argv[]) switch (opt) { case 'h': Dbg(dbg_ctl, "Setting header: %s", optarg); + TSfree(const_cast(xDebugHeader.str)); // The option can be repeated, so the earlier value is not leaked xDebugHeader.str = TSstrdup(optarg); break; case 'e': diff --git a/src/api/InkAPITest.cc b/src/api/InkAPITest.cc index eac60ccd3fc..881f0d91ebb 100644 --- a/src/api/InkAPITest.cc +++ b/src/api/InkAPITest.cc @@ -6656,6 +6656,9 @@ REGRESSION_TEST(SDK_API_TSMgmtGet)(RegressionTest *test, int /* atype ATS_UNUSED SDK_RPRINT(test, "TSMgmtStringGet", "TestCase1.4", TC_PASS, "ok"); } + // TSMgmtStringGet() hands back a copy the caller owns. + TSfree(svalue); + { TSRecordDataType result; auto ret = TSMgmtDataTypeGet(CONFIG_PARAM_STRING_NAME, &result); diff --git a/src/proxy/http/remap/RemapYamlConfig.cc b/src/proxy/http/remap/RemapYamlConfig.cc index 7a441434463..c515a6970d4 100644 --- a/src/proxy/http/remap/RemapYamlConfig.cc +++ b/src/proxy/http/remap/RemapYamlConfig.cc @@ -35,6 +35,7 @@ #include #include "tscore/Diags.h" +#include "tscore/ink_memory.h" #include "tscore/ink_string.h" #include "tsutil/ts_errata.h" #include "tsutil/PostScript.h" @@ -388,7 +389,12 @@ parse_map_referer(const YAML::Node &node, url_mapping *url_mapping) !strcasecmp(url.c_str(), "") || !strcasecmp(url.c_str(), "default_redirect_url")) { url_mapping->default_redirect_url = true; } - url_mapping->redir_chunk_list = redirect_tag_str::parse_format_redirect_url(ats_strdup(url.c_str())); + // parse_format_redirect_url() nul terminates each chunk in place before copying it out, and for a + // url with no format specifier that write lands on the terminating nul, which std::string does not + // allow a caller to assign. Give it a buffer we own instead, and release it once it returns; the + // chunk list holds copies. + ats_scoped_str redirect_url(ats_strdup(url.c_str())); + url_mapping->redir_chunk_list = redirect_tag_str::parse_format_redirect_url(redirect_url.get()); if (!node["regex"] || !node["regex"].IsSequence()) { return swoc::Errata("'regex' field must be sequence"); diff --git a/src/traffic_cache_tool/CacheTool.cc b/src/traffic_cache_tool/CacheTool.cc index c28c2111c42..8da841c4d2b 100644 --- a/src/traffic_cache_tool/CacheTool.cc +++ b/src/traffic_cache_tool/CacheTool.cc @@ -208,7 +208,7 @@ struct Cache { std::map _volumes; std::vector globalVec_stripe; std::unordered_set URLset; - unsigned short *stripes_hash_table; + unsigned short *stripes_hash_table = nullptr; }; Errata @@ -685,7 +685,14 @@ Cache::calcTotalSpanPhysicalSize() } #endif -Cache::~Cache() {} +Cache::~Cache() +{ + // The URL set and the stripe hash table are owned solely by this instance. + for (auto *url : URLset) { + delete url; + } + ats_free(stripes_hash_table); +} Errata Span::load()