Copilot commented on code in PR #13108: URL: https://github.com/apache/trafficserver/pull/13108#discussion_r4096746689
########## src/proxy/VirtualHost.cc: ########## @@ -0,0 +1,645 @@ +/** @file + + Virtual Host configuration implementation + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include <algorithm> +#include <cctype> +#include <cerrno> +#include <atomic> +#include <memory> +#include <mutex> +#include <set> +#include <string> +#include <string_view> +#include <strings.h> +#include <sys/stat.h> +#include <yaml-cpp/yaml.h> + +#include "proxy/VirtualHost.h" +#include "proxy/ReverseProxy.h" +#include "mgmt/config/ConfigContextDiags.h" +#include "mgmt/config/ConfigRegistry.h" +#include "records/RecCore.h" +#include "tscore/Filenames.h" +#include "tsutil/Convert.h" + +namespace +{ +DbgCtl dbg_ctl_virtualhost("virtualhost"); + +/** Serializes both reload paths against each other. + + Each reload must hold this from the moment it reads the file until it publishes. Otherwise two + reloads that parse concurrently can publish in the opposite order to their reads, and the older + read wins: a full reload resurrects entries a newer one had dropped, and a single-entry reload + parsed before a full reload that removed its id re-adds that entry on top of it. The single-entry + reload additionally does a read-copy-modify-publish against the live config, which the same + critical section keeps atomic. It also keeps the remap plugin reload notifications of one rebuild + from interleaving with another's. Reload paths are scheduled on ET_TASK with unrelated mutexes + (a fresh one per @c ConfigRegistry::schedule_reload and one per trigger record), so nothing else + serializes them. Neither path is on the request path, so serializing the parse costs nothing. + */ +std::mutex vhost_reconfigure_mutex; +} // namespace + +std::atomic<int> VirtualHost::_configid{0}; + +std::string +VirtualHostConfig::Entry::get_id() const +{ + return id; +} + +namespace +{ +const std::set<std::string> valid_vhost_keys = {"id", "domains", "remap"}; + +/** Check that @a name is composed of non-empty hostname labels. + + Without this, a documented-as-unsupported form such as `foo[0-9]+.example.com` is accepted as + an exact domain that no Host header can ever equal: the entry loads clean and never fires. + */ +bool +is_hostname(std::string_view name) +{ + // A bracketed IPv6 literal is matched verbatim against the Host header. + if (name.size() > 2 && name.front() == '[' && name.back() == ']') { + return true; + } + + size_t label_len = 0; + + for (char c : name) { + if (c == '.') { + if (label_len == 0) { + return false; + } + label_len = 0; + continue; + } + if (!isalnum(static_cast<unsigned char>(c)) && c != '-' && c != '_') { + return false; + } + ++label_len; + } + return label_len > 0; +} + +/** Decode a single `virtualhost` sequence element. + + Diagnostics go through @a ctx so the operator who asked for the reload sees them in the reload + task log, not just in diags.log. + */ +bool +decode_virtualhost_entry(YAML::Node const &node, VirtualHostConfig::Entry &item, ConfigContext ctx) +{ + if (!node["id"]) { + CfgLoadLog(ctx, DL_Error, "Virtualhost entry at line %d must provide `id`", node.Mark().line + 1); + return false; + } + item.id = node["id"].as<std::string>(); + if (item.id.empty()) { + // A single-entry reload cannot name an empty id, so such an entry could never be reloaded on its own. + CfgLoadLog(ctx, DL_Error, "Virtualhost entry at line %d must provide a non-empty `id`", node.Mark().line + 1); + return false; + } + + for (const auto &elem : node) { + auto key = elem.first.as<std::string>(); + if (!valid_vhost_keys.contains(key)) { + CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' has unsupported key '%s' (line %d)", item.id.c_str(), key.c_str(), + elem.first.Mark().line + 1); + return false; + } + } + + auto domains = node["domains"]; + if (!domains || !domains.IsSequence() || domains.size() == 0) { + CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' must provide at least one domain in a `domains` sequence (line %d)", + item.id.c_str(), node.Mark().line + 1); + return false; + } + item.exact_domains.clear(); + item.wildcard_domains.clear(); + + for (const auto &it : domains) { + auto domain_entry = it.as<std::string>(); + if (domain_entry.empty()) { + CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' has an empty entry in `domains` (line %d)", item.id.c_str(), it.Mark().line + 1); + return false; + } + char domain[TS_MAX_HOST_NAME_LEN + 1]; + ts::transform_lower(domain_entry, domain); + + // Check if domain is wildcard, prefixed with * + if (domain[0] == '*') { + if (domain[1] != '.' || domain[2] == '\0' || domain[2] == '.' || strchr(domain + 2, '*') != nullptr) { + CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' wildcard '%s' must match '*.[domain]' format (line %d)", item.id.c_str(), + domain, it.Mark().line + 1); + return false; + } + if (!is_hostname(domain + 2)) { + CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' wildcard '%s' suffix is not a valid hostname; regex is not supported (line %d)", + item.id.c_str(), domain, it.Mark().line + 1); + return false; + } + item.wildcard_domains.emplace_back(domain + 2); + } else { + if (strchr(domain, '*') != nullptr) { + CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' domain '%s' may only use a wildcard in the leading '*.[domain]' form (line %d)", + item.id.c_str(), domain, it.Mark().line + 1); + return false; + } + if (!is_hostname(domain)) { + CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' domain '%s' is not a valid hostname; regex is not supported (line %d)", + item.id.c_str(), domain, it.Mark().line + 1); + return false; + } + item.exact_domains.emplace_back(domain); + } + } + + if (item.exact_domains.empty() && item.wildcard_domains.empty()) { + CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' must have at least one domain defined (line %d)", item.id.c_str(), + node.Mark().line + 1); + return false; + } + + return true; +} + +bool +build_virtualhost_entry(YAML::Node const &node, Ptr<VirtualHostConfig::Entry> &entry, ConfigContext ctx, + VirtualHostPluginReload *plugin_reload) +{ + entry.clear(); + Ptr<VirtualHostConfig::Entry> vhost = make_ptr(new VirtualHostConfig::Entry); + auto &conf = *vhost; + try { + if (!decode_virtualhost_entry(node, conf, ctx)) { + return false; + } + } catch (YAML::Exception const &ex) { + CfgLoadLog(ctx, DL_Error, "Failed to parse virtualhost entry at line %d: %s", node.Mark().line + 1, ex.what()); + return false; + } + + // Build UrlRewrite table for remap rules + auto remap_node = node["remap"]; + if (remap_node) { + if (plugin_reload) { + plugin_reload->begin(); + } + auto table = std::make_unique<UrlRewrite>(); + table->set_remap_yaml(true); + if (!table->load_table({}, &remap_node, ctx)) { + CfgLoadLog(ctx, DL_Error, "Failed to load remap rules for virtualhost '%s' at line %d", conf.id.c_str(), + remap_node.Mark().line + 1); + return false; + } + conf.remap_table = make_managed_url_rewrite(std::move(table)); + } + entry = std::move(vhost); + return true; +} +} // namespace + +namespace +{ +/// Number of entries in the currently published config, so a refused reload can say what it would +/// otherwise have dropped. +size_t +live_entry_count() +{ + VirtualHost::scoped_config config; + + return config ? config->entry_count() : 0; +} +} // namespace + +bool +VirtualHostConfig::load(ConfigContext ctx, bool initial_load, VirtualHostPluginReload *plugin_reload) +{ + _entries.clear(); + _exact_domains_to_id.clear(); + _wildcard_domains_to_id.clear(); + std::string config_path = RecConfigReadConfigPath("proxy.config.virtualhost.filename", ts::filename::VIRTUALHOST); + + struct stat sbuf; + if (stat(config_path.c_str(), &sbuf) == -1 && errno == ENOENT) { + if (!initial_load) { + CfgLoadLog(ctx, DL_Error, "Cannot reload virtualhost config: '%s' doesn't exist; keeping the %zu live entry(s)", + config_path.c_str(), live_entry_count()); + return false; + } + CfgLoadLog(ctx, DL_Warning, "Virtualhost configuration '%s' doesn't exist, no virtualhost entries loaded", config_path.c_str()); + return true; + } + + try { + YAML::Node config = YAML::LoadFile(config_path); + if (config.IsNull()) { + if (!initial_load) { + CfgLoadLog(ctx, DL_Error, "Cannot reload virtualhost config: '%s' is empty; keeping the %zu live entry(s)", + config_path.c_str(), live_entry_count()); + return false; + } + Dbg(dbg_ctl_virtualhost, "Empty virtualhost config: %s", config_path.c_str()); + return true; + } + + config = config["virtualhost"]; + if (config.IsNull() || !config.IsSequence()) { + CfgLoadLog(ctx, DL_Error, "%s: expected toplevel 'virtualhost' key to be a sequence", config_path.c_str()); Review Comment: The reload guard only treats an entirely null YAML document as empty. A valid `virtualhost: []` document reaches the loop and returns success, so a full reload publishes an empty `VirtualHostConfig` and drops every currently live entry, contrary to the documented reload safety guarantee that an empty config leaves the previous routing table in place. Reject an empty top-level sequence on reload (while retaining the startup behavior) before publishing it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
