diff --git a/iocore/cache/Cache.cc b/iocore/cache/Cache.cc index 384ee354a03..6eb15f6e709 100644 --- a/iocore/cache/Cache.cc +++ b/iocore/cache/Cache.cc @@ -38,6 +38,8 @@ #include "tscore/hugepages.h" #include +#include +#include constexpr ts::VersionNumber CACHE_DB_VERSION(CACHE_DB_MAJOR_VERSION, CACHE_DB_MINOR_VERSION); @@ -2873,10 +2875,8 @@ cplist_reconfigure() // else the size is greater... /* search the cp_list */ - int *sorted_vols = new int[gndisks]; - for (int i = 0; i < gndisks; i++) { - sorted_vols[i] = i; - } + std::vector sorted_vols(gndisks); + std::iota(sorted_vols.begin(), sorted_vols.end(), 0); for (int i = 0; i < gndisks - 1; i++) { int smallest = sorted_vols[i]; int smallest_ndx = i; @@ -2936,8 +2936,6 @@ cplist_reconfigure() size_to_alloc = size_in_blocks - cp->size; } - delete[] sorted_vols; - if (size_to_alloc) { if (create_volume(volume_number, size_to_alloc, cp->scheme, cp)) { return -1; diff --git a/iocore/net/QUICNetVConnection.cc b/iocore/net/QUICNetVConnection.cc index 17b7d9ad0cf..3d62e15e179 100644 --- a/iocore/net/QUICNetVConnection.cc +++ b/iocore/net/QUICNetVConnection.cc @@ -344,7 +344,7 @@ QUICNetVConnection::acceptEvent(int event, Event *e) MUTEX_TRY_LOCK(lock, h->mutex, t); if (!lock.is_locked()) { - if (event == EVENT_NONE) { + if (event == EVENT_NONE || e == nullptr) { t->schedule_in(this, HRTIME_MSECONDS(net_retry_delay)); return EVENT_DONE; } else { diff --git a/iocore/net/UnixNet.cc b/iocore/net/UnixNet.cc index b5688d53569..f0dc77cf18e 100644 --- a/iocore/net/UnixNet.cc +++ b/iocore/net/UnixNet.cc @@ -23,6 +23,10 @@ #include "P_Net.h" +#if HAVE_EVENTFD +#include +#endif + using namespace std::literals; std::atomic NetHandler::additional_accepts{0}; @@ -223,8 +227,8 @@ static void net_signal_hook_callback(EThread *thread) { #if HAVE_EVENTFD - uint64_t counter; - ATS_UNUSED_RETURN(read(thread->evfd, &counter, sizeof(uint64_t))); + eventfd_t counter; + ATS_UNUSED_RETURN(eventfd_read(thread->evfd, &counter)); #elif TS_USE_PORT /* Nothing to drain or do */ #else diff --git a/iocore/net/UnixNetVConnection.cc b/iocore/net/UnixNetVConnection.cc index aa513550a95..15e041aa2ae 100644 --- a/iocore/net/UnixNetVConnection.cc +++ b/iocore/net/UnixNetVConnection.cc @@ -386,7 +386,7 @@ write_to_net_io(NetHandler *nh, UnixNetVConnection *vc, EThread *thread) nh->write_ready_list.remove(vc); } - int err, ret; + int err{0}, ret{0}; if (vc->get_context() == NET_VCONNECTION_OUT) { ret = vc->sslStartHandShake(SSL_EVENT_CLIENT, err); diff --git a/mgmt/ProcessManager.cc b/mgmt/ProcessManager.cc index b8661eebbb2..e9676240562 100644 --- a/mgmt/ProcessManager.cc +++ b/mgmt/ProcessManager.cc @@ -344,6 +344,7 @@ ProcessManager::initLMConnection() if ((local_manager_sockfd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) { Fatal("Unable to create socket '%s': %s", sockpath.c_str(), strerror(errno)); + return; } if (fcntl(local_manager_sockfd, F_SETFD, FD_CLOEXEC) < 0) { diff --git a/plugins/experimental/memcache/tsmemcache.cc b/plugins/experimental/memcache/tsmemcache.cc index fcc6668679a..b4f0d657731 100644 --- a/plugins/experimental/memcache/tsmemcache.cc +++ b/plugins/experimental/memcache/tsmemcache.cc @@ -238,7 +238,7 @@ MC::add_binary_header(uint16_t err, uint8_t hdr_len, uint16_t key_len, uint32_t int MC::write_binary_error(protocol_binary_response_status err, int swallow) { - const char *errstr = "Unknown error"; + const char *errstr{nullptr}; switch (err) { case PROTOCOL_BINARY_RESPONSE_ENOMEM: errstr = "Out of memory"; diff --git a/plugins/experimental/ssl_session_reuse/src/Config.h b/plugins/experimental/ssl_session_reuse/src/Config.h index e4ac1ffd454..2128229150c 100644 --- a/plugins/experimental/ssl_session_reuse/src/Config.h +++ b/plugins/experimental/ssl_session_reuse/src/Config.h @@ -88,6 +88,7 @@ class Config virtual ~Config(); bool loadConfigOnChange(); + bool checkConfigChange(); static const int cCheckDivisor = 5; @@ -100,6 +101,7 @@ class Config std::mutex m_yconfigLock; bool m_noConfig; bool m_alreadyLoaded; + bool m_loading = false; time_t m_lastCheck; time_t m_lastmtime; }; diff --git a/plugins/experimental/ssl_session_reuse/src/config.cc b/plugins/experimental/ssl_session_reuse/src/config.cc index ae0cc317905..eb03ae1b8e0 100644 --- a/plugins/experimental/ssl_session_reuse/src/config.cc +++ b/plugins/experimental/ssl_session_reuse/src/config.cc @@ -23,6 +23,7 @@ */ #include +#include #include #include #include @@ -46,58 +47,86 @@ Config::Config() Config::~Config() = default; +namespace +{ bool -Config::loadConfig(const std::string &filename) +readConfig(const std::string &filename, std::map &config) { - if (m_alreadyLoaded) { - return true; + int fd = open(filename.c_str(), O_RDONLY); + if (fd < 0) { + return false; } - - bool success = false; - - m_filename = filename; - - int fd = (this->m_filename.length() > 0 ? open(m_filename.c_str(), O_RDONLY) : ts::NO_FD); struct stat info; - if (fd > 0 && 0 == fstat(fd, &info)) { - size_t n = info.st_size; - std::string config_data; - config_data.resize(n); - if (read(fd, const_cast(config_data.data()), n) != static_cast(n)) { + if (fstat(fd, &info) != 0 || info.st_size < 0) { + close(fd); + return false; + } + + std::string config_data(static_cast(info.st_size), '\0'); + size_t offset = 0; + while (offset < config_data.size()) { + ssize_t n = read(fd, config_data.data() + offset, config_data.size() - offset); + if (n < 0 && errno == EINTR) { + continue; + } + if (n <= 0) { close(fd); - return success; + return false; } + offset += static_cast(n); + } + close(fd); - ts::TextView content(config_data); - while (content) { - ts::TextView line = content.take_prefix_at('\n'); - if (line.empty() || '#' == *line) { - continue; - } - line.ltrim_if(&isspace); - ts::TextView field = line.take_prefix_at('='); - std::string field_name; - std::string value; - if (!field.empty()) { - field_name.assign(field.data(), field.size()); - } - if (!line.empty()) { - value.assign(line.data(), line.size()); - } - TSDebug(PLUGIN, "%.*s=%.*s", static_cast(field_name.size()), field_name.c_str(), static_cast(value.size()), - value.c_str()); - if (!field_name.empty()) { - m_config[field_name] = value; - } + ts::TextView content(config_data); + while (content) { + ts::TextView line = content.take_prefix_at('\n'); + if (line.empty() || '#' == *line) { + continue; + } + line.ltrim_if(&isspace); + ts::TextView field = line.take_prefix_at('='); + std::string field_name; + std::string value; + if (!field.empty()) { + field_name.assign(field.data(), field.size()); } + if (!line.empty()) { + value.assign(line.data(), line.size()); + } + TSDebug(PLUGIN, "%.*s=%.*s", static_cast(field_name.size()), field_name.c_str(), static_cast(value.size()), + value.c_str()); + if (!field_name.empty()) { + config[field_name] = value; + } + } + return true; +} +} // namespace - close(fd); +bool +Config::loadConfig(const std::string &filename) +{ + { + std::lock_guard lock(m_yconfigLock); + if (m_alreadyLoaded || m_loading) { + return m_alreadyLoaded; + } + m_filename = filename; + m_loading = true; + } + + // Parse without the lock, then publish the complete map under the lock. + // Readers see the previous map (empty during the first load), never a partial one. + std::map config; + bool success = readConfig(filename, config); + std::lock_guard lock(m_yconfigLock); + m_loading = false; + if (success) { + m_config.swap(config); m_noConfig = false; - success = true; m_alreadyLoaded = true; } - return success; } @@ -108,7 +137,9 @@ Config::setLastConfigChange() time_t oldLastmtime = m_lastmtime; memset(&s, 0, sizeof(s)); - stat(m_filename.c_str(), &s); + if (stat(m_filename.c_str(), &s) != 0) { + return false; + } m_lastmtime = s.st_mtime; @@ -120,10 +151,17 @@ Config::setLastConfigChange() bool Config::configHasChanged() +{ + std::lock_guard lock(m_yconfigLock); + return !m_loading && checkConfigChange(); +} + +bool +Config::checkConfigChange() { time_t checkTime = time(nullptr) / cCheckDivisor; - if (0 == m_lastmtime || m_lastCheck != checkTime) { + if (m_lastCheck != checkTime) { m_lastCheck = checkTime; return setLastConfigChange(); } @@ -133,33 +171,32 @@ Config::configHasChanged() bool Config::loadConfigOnChange() { - if (configHasChanged()) { - // loadConfig will check this, and if it hasn't been set it'll just bail. + std::string filename; + { + std::lock_guard lock(m_yconfigLock); + if (m_loading || m_filename.empty()) { + return true; + } + if (!checkConfigChange()) { + return true; + } m_alreadyLoaded = false; - return loadConfig(m_filename); + filename = m_filename; } - - return true; + return loadConfig(filename); } bool Config::getValue(const std::string &category, const std::string &key, std::string &value) { - if (!m_noConfig) { - m_yconfigLock.lock(); - - if (loadConfigOnChange()) { - // convert to category.key= value. - std::string keyname = category + "." + key; - std::map::iterator it = m_config.find(keyname); - - // we have to use find so we don't overwrite defaults when we don't find anything. - if (m_config.end() != it) { + if (loadConfigOnChange()) { + std::lock_guard lock(m_yconfigLock); + if (!m_noConfig) { + auto it = m_config.find(category + "." + key); + if (it != m_config.end()) { value = it->second; } } - m_yconfigLock.unlock(); } - return !value.empty(); } diff --git a/plugins/experimental/ssl_session_reuse/src/ssl_init.cc b/plugins/experimental/ssl_session_reuse/src/ssl_init.cc index afc88fffeb0..1af69a9709c 100644 --- a/plugins/experimental/ssl_session_reuse/src/ssl_init.cc +++ b/plugins/experimental/ssl_session_reuse/src/ssl_init.cc @@ -106,7 +106,7 @@ get_redis_auth_key(char *retKeyBuff, int buffSize) if (ssl_param.redis_auth_key_file.length()) { int fd = open(ssl_param.redis_auth_key_file.c_str(), O_RDONLY); struct stat info; - if (0 == fstat(fd, &info)) { + if (fd >= 0 && 0 == fstat(fd, &info)) { size_t n = info.st_size; std::string key_data; key_data.resize(n); @@ -115,9 +115,14 @@ get_redis_auth_key(char *retKeyBuff, int buffSize) while (read_len > 1 && key_data[read_len - 1] == '\n') { --read_len; } - memset(retKeyBuff, 0, buffSize); - strncpy(retKeyBuff, key_data.c_str(), read_len); - retval = key_data.length(); + if (read_len > 0 && read_len < buffSize && static_cast(read_len) <= key_data.length()) { + memset(retKeyBuff, 0, buffSize); + strncpy(retKeyBuff, key_data.c_str(), read_len); + retval = read_len; + } + } + if (fd >= 0) { + close(fd); } } else { TSError("Can not get redis auth key."); diff --git a/plugins/experimental/system_stats/system_stats.c b/plugins/experimental/system_stats/system_stats.c index f15cc8751a1..bc014a26a1a 100644 --- a/plugins/experimental/system_stats/system_stats.c +++ b/plugins/experimental/system_stats/system_stats.c @@ -178,6 +178,10 @@ setBondingStat(TSMutex stat_creation_mutex, const char *interface) snprintf(&infdir[0], sizeof(infdir), "%s/%s", NET_STATS_DIR, interface); DIR *localdir = opendir(infdir); + if (localdir == NULL) { + TSError("%s: Unable to open %s", DEBUG_TAG, infdir); + return; + } while ((dent = readdir(localdir)) != NULL) { if (((strncmp(SLAVE, dent->d_name, strlen(SLAVE)) == 0) || (strncmp(LOWER, dent->d_name, strlen(LOWER)) == 0)) && diff --git a/plugins/header_rewrite/header_rewrite.cc b/plugins/header_rewrite/header_rewrite.cc index bb609dbfbe5..37939b2d30f 100644 --- a/plugins/header_rewrite/header_rewrite.cc +++ b/plugins/header_rewrite/header_rewrite.cc @@ -17,6 +17,7 @@ */ #include +#include #include #include #include @@ -128,7 +129,7 @@ RulesConfig::add_rule(RuleSet *rule) bool RulesConfig::parse_config(const std::string &fname, TSHttpHookID default_hook) { - RuleSet *rule = nullptr; + std::unique_ptr rule; std::string filename; std::ifstream f; int lineno = 0; @@ -178,15 +179,15 @@ RulesConfig::parse_config(const std::string &fname, TSHttpHookID default_hook) } // If we are at the beginning of a new condition, save away the previous rule (but only if it has operators). - if (p.is_cond() && add_rule(rule)) { - rule = nullptr; + if (p.is_cond() && add_rule(rule.get())) { + rule.release(); } TSHttpHookID hook = default_hook; bool is_hook = p.cond_is_hook(hook); // This updates the hook if explicitly set, if not leaves at default if (nullptr == rule) { - rule = new RuleSet(); + rule = std::make_unique(); rule->set_hook(hook); if (is_hook) { @@ -194,7 +195,6 @@ RulesConfig::parse_config(const std::string &fname, TSHttpHookID default_hook) if ((default_hook == TS_REMAP_PSEUDO_HOOK) && ((TS_HTTP_READ_REQUEST_HDR_HOOK == hook) || (TS_HTTP_PRE_REMAP_HOOK == hook))) { TSError("[%s] you can not use cond %%{%s} in a remap rule", PLUGIN_NAME, p.get_op().c_str()); - delete rule; return false; } continue; @@ -222,13 +222,14 @@ RulesConfig::parse_config(const std::string &fname, TSHttpHookID default_hook) } } catch (std::runtime_error &e) { TSError("[%s] header_rewrite configuration exception: %s in file: %s", PLUGIN_NAME, e.what(), fname.c_str()); - delete rule; return false; } } // Add the last rule (possibly the only rule) - add_rule(rule); + if (add_rule(rule.get())) { + rule.release(); + } // Collect all resource IDs that we need for (int i = TS_HTTP_READ_REQUEST_HDR_HOOK; i < TS_HTTP_LAST_HOOK; ++i) { diff --git a/plugins/healthchecks/healthchecks.c b/plugins/healthchecks/healthchecks.c index 93174e76f20..5a1342c7112 100644 --- a/plugins/healthchecks/healthchecks.c +++ b/plugins/healthchecks/healthchecks.c @@ -110,9 +110,10 @@ reload_status_file(HCFileInfo *info, HCFileData *data) memset(data, 0, sizeof(HCFileData)); if (NULL != (fd = fopen(info->fname, "r"))) { data->exists = 1; - do { - data->b_len = fread(data->body, 1, MAX_BODY_LEN, fd); - } while (!feof(fd)); /* Only save the last 16KB of the file ... */ + data->b_len = fread(data->body, 1, MAX_BODY_LEN, fd); + if (ferror(fd)) { + data->b_len = 0; + } fclose(fd); } } @@ -308,68 +309,66 @@ parse_configs(const char *fname) return NULL; } - while (!feof(fd)) { + while (fgets(buf, sizeof(buf) - 1, fd) != NULL) { char *str, *save; char *ok = NULL, *miss = NULL, *mime = NULL; finfo = TSmalloc(sizeof(HCFileInfo)); memset(finfo, 0, sizeof(HCFileInfo)); - if (fgets(buf, sizeof(buf) - 1, fd)) { - str = strtok_r(buf, SEPARATORS, &save); - int state = 0; - while (NULL != str) { - if (strlen(str) > 0) { - switch (state) { - case 0: - if ('/' == *str) { - ++str; - } - strncpy(finfo->path, str, PATH_NAME_MAX - 1); - finfo->p_len = strlen(finfo->path); - break; - case 1: - strncpy(finfo->fname, str, MAX_PATH_LEN - 1); - finfo->basename = strrchr(finfo->fname, '/'); - if (finfo->basename) { - ++(finfo->basename); - } - break; - case 2: - mime = str; - break; - case 3: - ok = str; - break; - case 4: - miss = str; - break; + str = strtok_r(buf, SEPARATORS, &save); + int state = 0; + while (NULL != str) { + if (strlen(str) > 0) { + switch (state) { + case 0: + if ('/' == *str) { + ++str; + } + strncpy(finfo->path, str, PATH_NAME_MAX - 1); + finfo->p_len = strlen(finfo->path); + break; + case 1: + strncpy(finfo->fname, str, MAX_PATH_LEN - 1); + finfo->basename = strrchr(finfo->fname, '/'); + if (finfo->basename) { + ++(finfo->basename); } - ++state; + break; + case 2: + mime = str; + break; + case 3: + ok = str; + break; + case 4: + miss = str; + break; } - str = strtok_r(NULL, SEPARATORS, &save); + ++state; } + str = strtok_r(NULL, SEPARATORS, &save); + } - /* Fill in the info if everything was ok */ - if (state > 4) { - TSDebug(PLUGIN_NAME, "Parsed: %s %s %s %s %s", finfo->path, finfo->fname, mime, ok, miss); - finfo->ok = gen_header(ok, mime, &finfo->o_len); - finfo->miss = gen_header(miss, mime, &finfo->m_len); - finfo->data = TSmalloc(sizeof(HCFileData)); - memset(finfo->data, 0, sizeof(HCFileData)); - reload_status_file(finfo, finfo->data); - - /* Add it the linked list */ - TSDebug(PLUGIN_NAME, "Adding path=%s to linked list", finfo->path); - if (NULL == head_finfo) { - head_finfo = finfo; - } else { - prev_finfo->_next = finfo; - } - prev_finfo = finfo; + /* Fill in the info if everything was ok */ + if (state > 4) { + TSDebug(PLUGIN_NAME, "Parsed: %s %s %s %s %s", finfo->path, finfo->fname, mime, ok, miss); + finfo->ok = gen_header(ok, mime, &finfo->o_len); + finfo->miss = gen_header(miss, mime, &finfo->m_len); + finfo->data = TSmalloc(sizeof(HCFileData)); + memset(finfo->data, 0, sizeof(HCFileData)); + reload_status_file(finfo, finfo->data); + + /* Add it the linked list */ + TSDebug(PLUGIN_NAME, "Adding path=%s to linked list", finfo->path); + if (NULL == head_finfo) { + head_finfo = finfo; } else { - TSfree(finfo); + prev_finfo->_next = finfo; } + prev_finfo = finfo; + } else { + TSfree(finfo); } } fclose(fd); diff --git a/proxy/http/HttpBodyFactory.cc b/proxy/http/HttpBodyFactory.cc index c60680cc461..8ed6a610547 100644 --- a/proxy/http/HttpBodyFactory.cc +++ b/proxy/http/HttpBodyFactory.cc @@ -252,6 +252,7 @@ HttpBodyFactory::reconfigure() unlock(); return; } // callbacks not setup right + unlock(); //////////////////////////////////////////// // extract relevant records.config values // @@ -262,15 +263,15 @@ HttpBodyFactory::reconfigure() all_found = true; // enable_customizations if records.config set - rec_err = RecGetRecordInt("proxy.config.body_factory.enable_customizations", &e); - enable_customizations = ((rec_err == REC_ERR_OKAY) ? e : 0); - all_found = all_found && (rec_err == REC_ERR_OKAY); - Debug("body_factory", "enable_customizations = %d (found = %" PRId64 ")", enable_customizations, e); + rec_err = RecGetRecordInt("proxy.config.body_factory.enable_customizations", &e); + int new_enable_customizations = ((rec_err == REC_ERR_OKAY) ? e : 0); + all_found = all_found && (rec_err == REC_ERR_OKAY); + Debug("body_factory", "enable_customizations = %d (found = %" PRId64 ")", new_enable_customizations, e); - rec_err = RecGetRecordInt("proxy.config.body_factory.enable_logging", &e); - enable_logging = ((rec_err == REC_ERR_OKAY) ? (e ? true : false) : false); - all_found = all_found && (rec_err == REC_ERR_OKAY); - Debug("body_factory", "enable_logging = %d (found = %" PRId64 ")", enable_logging, e); + rec_err = RecGetRecordInt("proxy.config.body_factory.enable_logging", &e); + bool new_enable_logging = ((rec_err == REC_ERR_OKAY) ? (e ? true : false) : false); + all_found = all_found && (rec_err == REC_ERR_OKAY); + Debug("body_factory", "enable_logging = %d (found = %" PRId64 ")", new_enable_logging, e); ats_scoped_str directory_of_template_sets; @@ -296,21 +297,17 @@ HttpBodyFactory::reconfigure() Warning("config changed, but can't fetch all proxy.config.body_factory values"); } - ///////////////////////////////////////////// - // clear out previous template hash tables // - ///////////////////////////////////////////// - - nuke_template_tables(); - - ///////////////////////////////////////////////////////////// - // at this point, the body hash table is gone, so we start // - // building a new one, by scanning the template directory. // - ///////////////////////////////////////////////////////////// - + std::unique_ptr new_table_of_sets; if (directory_of_template_sets) { - table_of_sets = load_sets_from_directory(directory_of_template_sets); + new_table_of_sets = load_sets_from_directory(directory_of_template_sets); } + // Publish the staged settings and template table together under the lock. + lock(); + enable_customizations = new_enable_customizations; + enable_logging = new_enable_logging; + nuke_template_tables(); + table_of_sets = std::move(new_table_of_sets); unlock(); } @@ -718,7 +715,6 @@ HttpBodyFactory::nuke_template_tables() } } -// LOCKING: must be called with lock taken std::unique_ptr HttpBodyFactory::load_sets_from_directory(char *set_dir) { @@ -788,7 +784,6 @@ HttpBodyFactory::load_sets_from_directory(char *set_dir) return new_table_of_sets; } -// LOCKING: must be called with lock taken HttpBodySet * HttpBodyFactory::load_body_set_from_directory(char *set_name, char *tmpl_dir) { diff --git a/proxy/http/HttpProxyServerMain.cc b/proxy/http/HttpProxyServerMain.cc index 68f68102c96..92cba509c89 100644 --- a/proxy/http/HttpProxyServerMain.cc +++ b/proxy/http/HttpProxyServerMain.cc @@ -208,18 +208,22 @@ MakeHttpProxyAcceptor(HttpProxyAcceptor &acceptor, HttpProxyPort &port, unsigned // XXX the protocol probe should be a configuration option. - ProtocolProbeSessionAccept *probe = new ProtocolProbeSessionAccept(); - HttpSessionAccept *http = nullptr; // don't allocate this unless it will be used. - probe->proxyPort = &port; - probe->proxy_protocol_ipmap = &HttpConfig::m_master.config_proxy_protocol_ipmap; - - if (port.m_session_protocol_preference.intersects(HTTP_PROTOCOL_SET)) { - http = new HttpSessionAccept(accept_opt); - probe->registerEndpoint(ProtocolProbeSessionAccept::PROTO_HTTP, http); - } + ProtocolProbeSessionAccept *probe = nullptr; + HttpSessionAccept *http = nullptr; + + if (!port.isQUIC()) { + probe = new ProtocolProbeSessionAccept(); + probe->proxyPort = &port; + probe->proxy_protocol_ipmap = &HttpConfig::m_master.config_proxy_protocol_ipmap; - if (port.m_session_protocol_preference.intersects(HTTP2_PROTOCOL_SET)) { - probe->registerEndpoint(ProtocolProbeSessionAccept::PROTO_HTTP2, new Http2SessionAccept(accept_opt)); + if (port.m_session_protocol_preference.intersects(HTTP_PROTOCOL_SET)) { + http = new HttpSessionAccept(accept_opt); + probe->registerEndpoint(ProtocolProbeSessionAccept::PROTO_HTTP, http); + } + + if (port.m_session_protocol_preference.intersects(HTTP2_PROTOCOL_SET)) { + probe->registerEndpoint(ProtocolProbeSessionAccept::PROTO_HTTP2, new Http2SessionAccept(accept_opt)); + } } if (port.isSSL()) { diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index 527c139d418..c9492942701 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -7818,7 +7818,7 @@ void HttpTransact::handle_server_died(State *s) { const char *reason = nullptr; - const char *body_type = "UNKNOWN"; + const char *body_type = nullptr; HTTPStatus status = HTTP_STATUS_BAD_GATEWAY; //////////////////////////////////////////////////////// diff --git a/proxy/http/remap/unit-tests/plugin_testing_common.cc b/proxy/http/remap/unit-tests/plugin_testing_common.cc index 07a44e15f29..aa688ddd971 100644 --- a/proxy/http/remap/unit-tests/plugin_testing_common.cc +++ b/proxy/http/remap/unit-tests/plugin_testing_common.cc @@ -29,6 +29,8 @@ #include "plugin_testing_common.h" +#include + void PrintToStdErr(const char *fmt, ...) { @@ -48,7 +50,12 @@ getTemporaryDir() char dirNameTemplate[tmpDir.string().length() + 1]; sprintf(dirNameTemplate, "%s", tmpDir.c_str()); - return fs::path(mkdtemp(dirNameTemplate)); + char *directory = mkdtemp(dirNameTemplate); + + if (directory == nullptr) { + throw std::system_error(errno, std::system_category(), "mkdtemp"); + } + return fs::path(directory); } // implement functions to support unit-testing of option to enable/disable dynamic reload of plugins diff --git a/proxy/http3/Http3Frame.cc b/proxy/http3/Http3Frame.cc index 6bb195e1a61..3125b751dcc 100644 --- a/proxy/http3/Http3Frame.cc +++ b/proxy/http3/Http3Frame.cc @@ -480,8 +480,7 @@ Http3FrameFactory::create_headers_frame(IOBufferReader *header_block_reader, siz { ats_unique_buf buf = ats_unique_malloc(header_block_len); - int64_t nread; - while ((nread = header_block_reader->read(buf.get(), header_block_len)) > 0) { + while (header_block_reader->read(buf.get(), header_block_len) > 0) { ; } diff --git a/proxy/http3/QPACK.cc b/proxy/http3/QPACK.cc index a6932ea0d1d..2625280ae23 100644 --- a/proxy/http3/QPACK.cc +++ b/proxy/http3/QPACK.cc @@ -268,11 +268,13 @@ QPACK::decode(uint64_t stream_id, const uint8_t *header_block, size_t header_blo if (this->_dynamic_table.largest_index() < largest_reference) { // Blocked - if (this->_add_to_blocked_list( - new DecodeRequest(largest_reference, thread, cont, stream_id, header_block, header_block_len, hdr))) { + auto *decode_request = new DecodeRequest(largest_reference, thread, cont, stream_id, header_block, header_block_len, hdr); + + if (this->_add_to_blocked_list(decode_request)) { return 1; } else { // Number of blocked streams exceed the limit + delete decode_request; return -2; } } diff --git a/src/traffic_server/InkAPI.cc b/src/traffic_server/InkAPI.cc index 41e31467b88..b90bc098ef3 100644 --- a/src/traffic_server/InkAPI.cc +++ b/src/traffic_server/InkAPI.cc @@ -7769,8 +7769,8 @@ TSCacheScan(TSCont contp, TSCacheKey key, int KB_per_second) int TSStatCreate(const char *the_name, TSRecordDataType the_type, TSStatPersistence persist, TSStatSync sync) { - int id = ink_atomic_increment(&api_rsb_index, 1); - RecRawStatSyncCb syncer = RecRawStatSyncCount; + int id = ink_atomic_increment(&api_rsb_index, 1); + RecRawStatSyncCb syncer; // TODO: This only supports "int" data types at this point, since the "Raw" stats // interfaces only supports integers. Going forward, we could extend either the "Raw" diff --git a/src/tscore/ink_queue.cc b/src/tscore/ink_queue.cc index 9dd4c7d9778..6ff929f0e50 100644 --- a/src/tscore/ink_queue.cc +++ b/src/tscore/ink_queue.cc @@ -145,6 +145,7 @@ ink_freelist_init(InkFreeList **fl, const char *name, uint32_t type_size, uint32 // Make sure we align *all* the objects in the allocation, not just the first one f->type_size = INK_ALIGN(type_size, f->alignment); Debug(DEBUG_TAG "_init", "<%s> Type Size request/actual (%" PRIu32 "/%" PRIu32 ")", name, type_size, f->type_size); + ink_assert(f->type_size != 0); if (ats_hugepage_enabled()) { f->chunk_size = INK_ALIGN(chunk_size * f->type_size, ats_hugepage_size()) / f->type_size; } else { diff --git a/src/tscore/ts_file.cc b/src/tscore/ts_file.cc index f77faaefe58..0b1e25a181e 100644 --- a/src/tscore/ts_file.cc +++ b/src/tscore/ts_file.cc @@ -195,19 +195,27 @@ namespace file while (true) { size_t in = fread(buf, 1, bufsize, src); - if (0 == in) { + if (ferror(src)) { + ec = std::error_code(errno ? errno : EIO, std::system_category()); break; } - size_t out = fwrite(buf, 1, in, dst); - if (0 == out) { + if (in > 0 && fwrite(buf, 1, in, dst) != in) { + ec = std::error_code(errno ? errno : EIO, std::system_category()); + break; + } + if (in < static_cast(bufsize)) { break; } } - fclose(src); - fclose(dst); + if (fclose(src) != 0 && !ec) { + ec = std::error_code(errno ? errno : EIO, std::system_category()); + } + if (fclose(dst) != 0 && !ec) { + ec = std::error_code(errno ? errno : EIO, std::system_category()); + } - return true; + return !ec; } static bool diff --git a/src/tscore/unit_tests/test_ts_file.cc b/src/tscore/unit_tests/test_ts_file.cc index 2c599446844..6191fc8f7f3 100644 --- a/src/tscore/unit_tests/test_ts_file.cc +++ b/src/tscore/unit_tests/test_ts_file.cc @@ -22,6 +22,7 @@ */ #include +#include #include /* ofstream */ #include "tscore/ts_file.h" @@ -276,3 +277,53 @@ TEST_CASE("ts_file::path::copy", "[libts][fs_file]") CHECK(ts::file::remove(testdir1, ec)); CHECK_FALSE(ts::file::exists(testdir1)); } + +TEST_CASE("ts_file::copy", "[libts][ts_file_copy]") +{ + std::string directory = (ts::file::temp_directory_path() / "ts-file-copy-XXXXXX").string(); + REQUIRE(mkdtemp(directory.data()) != nullptr); + path testdir(directory); + path source = testdir / "source"; + path destination = testdir / "destination"; + std::error_code ec; + + for (size_t size : {0, 1, 65535, 65536, 65537, 131073}) { + INFO("Copy size: " << size); + std::string content(size, '\0'); + for (size_t i = 0; i < size; ++i) { + content[i] = static_cast(i % 256); + } + std::ofstream output(source.string(), std::ios::binary); + output.write(content.data(), content.size()); + output.close(); + REQUIRE(output.good()); + + CHECK(ts::file::copy(source, destination, ec)); + CHECK_FALSE(ec); + CHECK(ts::file::load(destination, ec) == content); + CHECK_FALSE(ec); + } + +#ifdef __linux__ + CHECK_FALSE(ts::file::copy(testdir, destination, ec)); + CHECK(ec.value() == EISDIR); + + if (access("/dev/full", W_OK) == 0) { + for (size_t size : {1, 65536}) { + INFO("Write failure size: " << size); + std::ofstream output(source.string(), std::ios::binary); + output << std::string(size, 'x'); + output.close(); + REQUIRE(output.good()); + + CHECK_FALSE(ts::file::copy(source, path("/dev/full"), ec)); + CHECK(ec.value() == ENOSPC); + } + } else { + WARN("Skipping write-failure checks: /dev/full is not writable"); + } +#endif + + CHECK(ts::file::remove(testdir, ec)); + CHECK_FALSE(ec); +} diff --git a/tests/gold_tests/timeout/ssl-delay-server.cc b/tests/gold_tests/timeout/ssl-delay-server.cc index 72390c4688e..a8e33d95f54 100644 --- a/tests/gold_tests/timeout/ssl-delay-server.cc +++ b/tests/gold_tests/timeout/ssl-delay-server.cc @@ -166,6 +166,10 @@ main(int argc, char *argv[]) fprintf(stderr, "Listen on %d connect delay=%d ttfb delay=%d\n", listen_port, connect_delay, ttfb_delay); int listenfd = socket(AF_INET, SOCK_STREAM, 0); + if (listenfd < 0) { + perror("socket"); + return EXIT_FAILURE; + } struct sockaddr_in serv_addr; memset(&serv_addr, '0', sizeof(serv_addr)); @@ -174,7 +178,11 @@ main(int argc, char *argv[]) serv_addr.sin_addr.s_addr = htonl(INADDR_ANY); serv_addr.sin_port = htons(listen_port); - bind(listenfd, reinterpret_cast(&serv_addr), sizeof(serv_addr)); + if (bind(listenfd, reinterpret_cast(&serv_addr), sizeof(serv_addr)) != 0) { + perror("bind"); + close(listenfd); + return EXIT_FAILURE; + } SSL_load_error_strings(); SSL_library_init(); diff --git a/tests/gold_tests/tls/tls_client_versions.test.py b/tests/gold_tests/tls/tls_client_versions.test.py index e8c13226d36..418938d1e19 100644 --- a/tests/gold_tests/tls/tls_client_versions.test.py +++ b/tests/gold_tests/tls/tls_client_versions.test.py @@ -16,6 +16,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import os + Test.Summary = ''' Test TLS protocol offering based on SNI ''' @@ -25,6 +27,10 @@ Test.SkipUnless(Condition.HasOpenSSLVersion("1.1.1")) +# Let the test control protocol availability instead of the system OpenSSL +# policy, which can impose a TLS 1.2 minimum despite the ATS settings below. +Test.Env['OPENSSL_CONF'] = os.devnull + # Define default ATS ts = Test.MakeATSProcess("ts", select_ports=True, enable_tls=True) server = Test.MakeOriginServer("server", ssl=True) @@ -81,16 +87,18 @@ tr.ReturnCode = 35 tr.StillRunningAfter = ts +# TLS 1.0 ECDHE handshakes require SHA-1 signatures, which Fedora disables +# independently of the security level. Use RSA key exchange for the TLS 1.0 checks. # Target foo.com for TLSv1. Should succeed tr = Test.AddTestRun("foo.com TLSv1") -tr.Processes.Default.Command = "curl -v --ciphers DEFAULT@SECLEVEL=0 --tls-max 1.0 --tlsv1 --resolve 'foo.com:{0}:127.0.0.1' -k https://foo.com:{0}".format( +tr.Processes.Default.Command = "curl -v --ciphers AES128-SHA:@SECLEVEL=0 --tls-max 1.0 --tlsv1 --resolve 'foo.com:{0}:127.0.0.1' -k https://foo.com:{0}".format( ts.Variables.ssl_port) tr.ReturnCode = 0 tr.StillRunningAfter = ts # Target bar.com for TLSv1. Should fail tr = Test.AddTestRun("bar.com TLSv1") -tr.Processes.Default.Command = "curl -v --ciphers DEFAULT@SECLEVEL=0 --tls-max 1.0 --tlsv1 --resolve 'bar.com:{0}:127.0.0.1' -k https://bar.com:{0}".format( +tr.Processes.Default.Command = "curl -v --ciphers AES128-SHA:@SECLEVEL=0 --tls-max 1.0 --tlsv1 --resolve 'bar.com:{0}:127.0.0.1' -k https://bar.com:{0}".format( ts.Variables.ssl_port) tr.ReturnCode = 35 tr.StillRunningAfter = ts diff --git a/tests/tools/plugins/async_engine.c b/tests/tools/plugins/async_engine.c index b596b418050..aa0eaf18c32 100644 --- a/tests/tools/plugins/async_engine.c +++ b/tests/tools/plugins/async_engine.c @@ -73,7 +73,10 @@ EVP_PKEY * async_load_privkey(ENGINE *e, const char *s_key_id, UI_METHOD *ui_method, void *callback_data) { fprintf(stderr, "Loading key %s\n", s_key_id); - FILE *f = fopen(s_key_id, "r"); + FILE *f = fopen(s_key_id, "r"); + if (f == NULL) { + return NULL; + } EVP_PKEY *key = PEM_read_PrivateKey(f, NULL, NULL, NULL); fclose(f); return key;