From 7dcdce8cafeb1b1fe0c795bc5a8c69909ddd168b Mon Sep 17 00:00:00 2001 From: Otto Moerbeek Date: Tue, 9 May 2023 13:15:34 +0200 Subject: [PATCH] More delinting A set of random files made clean. During this process .clang-tidy.full was also amended a bit. --- pdns/recursordist/rec-taskqueue.cc | 60 +++---- pdns/recursordist/rec-taskqueue.hh | 4 +- pdns/recursordist/taskqueue.cc | 8 +- pdns/recursordist/taskqueue.hh | 36 ++--- pdns/recursordist/ws-recursor.cc | 250 ++++++++++++++++------------- pdns/recursordist/ws-recursor.hh | 10 +- pdns/secpoll.cc | 12 +- pdns/zonemd.cc | 57 ++++--- pdns/zonemd.hh | 32 ++-- 9 files changed, 253 insertions(+), 216 deletions(-) diff --git a/pdns/recursordist/rec-taskqueue.cc b/pdns/recursordist/rec-taskqueue.cc index f805ced72e4c..29cc6d75c889 100644 --- a/pdns/recursordist/rec-taskqueue.cc +++ b/pdns/recursordist/rec-taskqueue.cc @@ -19,6 +19,7 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ + #include "rec-taskqueue.hh" #include "taskqueue.hh" #include "lock.hh" @@ -30,8 +31,8 @@ class TimedSet { public: - TimedSet(time_t t) : - d_expiry_seconds(t) + TimedSet(time_t time) : + d_expiry_seconds(time) { } @@ -40,11 +41,11 @@ class TimedSet // This purge is relatively cheap, as we're walking an ordered index uint64_t erased = 0; auto& ind = d_set.template get(); - auto it = ind.begin(); - while (it != ind.end()) { - if (it->d_ttd < now) { + auto iter = ind.begin(); + while (iter != ind.end()) { + if (iter->d_ttd < now) { ++erased; - it = ind.erase(it); + iter = ind.erase(iter); } else { break; @@ -79,17 +80,18 @@ class TimedSet private: struct Entry { - Entry(const pdns::ResolveTask& task, time_t ttd) : - d_task(task), d_ttd(ttd) {} + Entry(pdns::ResolveTask task, time_t ttd) : + d_task(std::move(task)), d_ttd(ttd) {} pdns::ResolveTask d_task; time_t d_ttd; }; - typedef multi_index_container, member>, - ordered_non_unique, member>>> - timed_set_t; + using timed_set_t = multi_index_container< + Entry, + indexed_by, + member>, + ordered_non_unique, + member>>>; timed_set_t d_set; time_t d_expiry_seconds; unsigned int d_count{0}; @@ -116,15 +118,15 @@ static void resolve(const struct timeval& now, bool logErrors, const pdns::Resol { auto log = g_slog->withName("taskq")->withValues("name", Logging::Loggable(task.d_qname), "qtype", Logging::Loggable(QType(task.d_qtype).toString()), "netmask", Logging::Loggable(task.d_netmask.empty() ? "" : task.d_netmask.toString())); const string msg = "Exception while running a background ResolveTask"; - SyncRes sr(now); + SyncRes resolver(now); vector ret; - sr.setRefreshAlmostExpired(task.d_refreshMode); - sr.setQuerySource(task.d_netmask); - bool ex = true; + resolver.setRefreshAlmostExpired(task.d_refreshMode); + resolver.setQuerySource(task.d_netmask); + bool exceptionOccurred = true; try { log->info(Logr::Debug, "resolving", "refresh", Logging::Loggable(task.d_refreshMode)); - int res = sr.beginResolve(task.d_qname, QType(task.d_qtype), QClass::IN, ret); - ex = false; + int res = resolver.beginResolve(task.d_qname, QType(task.d_qtype), QClass::IN, ret); + exceptionOccurred = false; log->info(Logr::Debug, "done", "rcode", Logging::Loggable(res), "records", Logging::Loggable(ret.size())); } catch (const std::exception& e) { @@ -146,7 +148,7 @@ static void resolve(const struct timeval& now, bool logErrors, const pdns::Resol catch (...) { log->error(Logr::Error, msg, "Unexpectec exception"); } - if (ex) { + if (exceptionOccurred) { if (task.d_refreshMode) { ++s_almost_expired_tasks.exceptions; } @@ -168,15 +170,15 @@ static void tryDoT(const struct timeval& now, bool logErrors, const pdns::Resolv { auto log = g_slog->withName("taskq")->withValues("method", Logging::Loggable("tryDoT"), "name", Logging::Loggable(task.d_qname), "qtype", Logging::Loggable(QType(task.d_qtype).toString()), "ip", Logging::Loggable(task.d_ip)); const string msg = "Exception while running a background tryDoT task"; - SyncRes sr(now); + SyncRes resolver(now); vector ret; - sr.setRefreshAlmostExpired(false); - bool ex = true; + resolver.setRefreshAlmostExpired(false); + bool exceptionOccurred = true; try { log->info(Logr::Debug, "trying DoT"); - bool ok = sr.tryDoT(task.d_qname, QType(task.d_qtype), task.d_nsname, task.d_ip, now.tv_sec); - ex = false; - log->info(Logr::Debug, "done", "ok", Logging::Loggable(ok)); + bool tryOK = resolver.tryDoT(task.d_qname, QType(task.d_qtype), task.d_nsname, task.d_ip, now.tv_sec); + exceptionOccurred = false; + log->info(Logr::Debug, "done", "ok", Logging::Loggable(tryOK)); } catch (const std::exception& e) { log->error(Logr::Error, msg, e.what()); @@ -197,7 +199,7 @@ static void tryDoT(const struct timeval& now, bool logErrors, const pdns::Resolv catch (...) { log->error(Logr::Error, msg, "Unexpected exception"); } - if (ex) { + if (exceptionOccurred) { ++s_resolve_tasks.exceptions; } else { @@ -262,7 +264,7 @@ void pushResolveTask(const DNSName& qname, uint16_t qtype, time_t now, time_t de } } -bool pushTryDoTTask(const DNSName& qname, uint16_t qtype, const ComboAddress& ip, time_t deadline, const DNSName& nsname) +bool pushTryDoTTask(const DNSName& qname, uint16_t qtype, const ComboAddress& ipAddress, time_t deadline, const DNSName& nsname) { if (SyncRes::isUnsupported(qtype)) { auto log = g_slog->withName("taskq")->withValues("name", Logging::Loggable(qname), "qtype", Logging::Loggable(QType(qtype).toString())); @@ -270,7 +272,7 @@ bool pushTryDoTTask(const DNSName& qname, uint16_t qtype, const ComboAddress& ip return false; } - pdns::ResolveTask task{qname, qtype, deadline, false, tryDoT, ip, nsname, {}}; + pdns::ResolveTask task{qname, qtype, deadline, false, tryDoT, ipAddress, nsname, {}}; bool pushed = s_taskQueue.lock()->queue.push(std::move(task)); if (pushed) { ++s_almost_expired_tasks.pushed; diff --git a/pdns/recursordist/rec-taskqueue.hh b/pdns/recursordist/rec-taskqueue.hh index 67bc6e0285a6..425cb55ac05f 100644 --- a/pdns/recursordist/rec-taskqueue.hh +++ b/pdns/recursordist/rec-taskqueue.hh @@ -22,7 +22,7 @@ #pragma once #include -#include +#include class DNSName; union ComboAddress; @@ -36,7 +36,7 @@ void runTasks(size_t max, bool logErrors); bool runTaskOnce(bool logErrors); void pushAlmostExpiredTask(const DNSName& qname, uint16_t qtype, time_t deadline, const Netmask& netmask); void pushResolveTask(const DNSName& qname, uint16_t qtype, time_t now, time_t deadline); -bool pushTryDoTTask(const DNSName& qname, uint16_t qtype, const ComboAddress& ip, time_t deadline, const DNSName& nsname); +bool pushTryDoTTask(const DNSName& qname, uint16_t qtype, const ComboAddress& ipAddress, time_t deadline, const DNSName& nsname); void taskQueueClear(); pdns::ResolveTask taskQueuePop(); diff --git a/pdns/recursordist/taskqueue.cc b/pdns/recursordist/taskqueue.cc index 8382cc4f4bc0..d18a8cb8c9ca 100644 --- a/pdns/recursordist/taskqueue.cc +++ b/pdns/recursordist/taskqueue.cc @@ -45,14 +45,14 @@ ResolveTask TaskQueue::pop() return ret; } -bool ResolveTask::run(bool logErrors) +bool ResolveTask::run(bool logErrors) const { if (d_func == nullptr) { auto log = g_slog->withName("taskq")->withValues("name", Logging::Loggable(d_qname), "qtype", Logging::Loggable(QType(d_qtype).toString())); log->error(Logr::Debug, "null task"); return false; } - struct timeval now; + struct timeval now{}; Utility::gettimeofday(&now); if (d_deadline >= now.tv_sec) { d_func(now, logErrors, *this); @@ -70,8 +70,8 @@ bool ResolveTask::run(bool logErrors) namespace boost { -size_t hash_value(const ComboAddress& a) +size_t hash_value(const ComboAddress& address) { - return ComboAddress::addressOnlyHash()(a); + return ComboAddress::addressOnlyHash()(address); } } diff --git a/pdns/recursordist/taskqueue.hh b/pdns/recursordist/taskqueue.hh index ace34d8d8df8..6f174e05fec1 100644 --- a/pdns/recursordist/taskqueue.hh +++ b/pdns/recursordist/taskqueue.hh @@ -63,23 +63,23 @@ struct ResolveTask DNSName d_nsname; Netmask d_netmask; - bool operator<(const ResolveTask& a) const + bool operator<(const ResolveTask& task) const { - return std::tie(d_qname, d_qtype, d_refreshMode, d_func, d_ip, d_netmask) < std::tie(a.d_qname, a.d_qtype, a.d_refreshMode, a.d_func, a.d_ip, a.d_netmask); + return std::tie(d_qname, d_qtype, d_refreshMode, d_func, d_ip, d_netmask) < std::tie(task.d_qname, task.d_qtype, task.d_refreshMode, task.d_func, task.d_ip, task.d_netmask); } - bool run(bool logErrors); + [[nodiscard]] bool run(bool logErrors) const; }; class TaskQueue { public: - bool empty() const + [[nodiscard]] bool empty() const { return d_queue.empty(); } - size_t size() const + [[nodiscard]] size_t size() const { return d_queue.size(); } @@ -87,12 +87,12 @@ public: bool push(ResolveTask&& task); ResolveTask pop(); - uint64_t getPushes() + [[nodiscard]] uint64_t getPushes() const { return d_pushes; } - uint64_t getExpired() + [[nodiscard]] uint64_t getExpired() const { return d_expired; } @@ -116,19 +116,17 @@ private: { }; - typedef multi_index_container< + using queue_t = multi_index_container< ResolveTask, - indexed_by< - ordered_unique, - composite_key, - member, - member, - member, - member, - member>>, - sequenced>>> - queue_t; + indexed_by, + composite_key, + member, + member, + member, + member, + member>>, + sequenced>>>; queue_t d_queue; uint64_t d_pushes{0}; diff --git a/pdns/recursordist/ws-recursor.cc b/pdns/recursordist/ws-recursor.cc index c25236ed5ab5..324928b88894 100644 --- a/pdns/recursordist/ws-recursor.cc +++ b/pdns/recursordist/ws-recursor.cc @@ -99,16 +99,16 @@ static void apiServerConfigACL(const std::string& aclType, HttpRequest* req, Htt } } - ostringstream ss; + ostringstream strStream; // Clear -from-file if set, so our changes take effect - ss << aclType << "-file=" << endl; + strStream << aclType << "-file=" << endl; // Clear ACL setting, and provide a "parent" value - ss << aclType << "=" << endl; - ss << aclType << "+=" << nmg.toString() << endl; + strStream << aclType << "=" << endl; + strStream << aclType << "+=" << nmg.toString() << endl; - apiWriteConfigFile(aclType, ss.str()); + apiWriteConfigFile(aclType, strStream.str()); parseACLs(); @@ -146,23 +146,24 @@ static void apiServerConfigAllowNotifyFrom(HttpRequest* req, HttpResponse* resp) static void fillZone(const DNSName& zonename, HttpResponse* resp) { auto iter = SyncRes::t_sstorage.domainmap->find(zonename); - if (iter == SyncRes::t_sstorage.domainmap->end()) + if (iter == SyncRes::t_sstorage.domainmap->end()) { throw ApiException("Could not find domain '" + zonename.toLogString() + "'"); + } const SyncRes::AuthDomain& zone = iter->second; Json::array servers; for (const ComboAddress& server : zone.d_servers) { - servers.push_back(server.toStringWithPort()); + servers.push_back(server.toStringWithPort()); // NOLINT: union } Json::array records; - for (const SyncRes::AuthDomain::records_t::value_type& dr : zone.d_records) { + for (const SyncRes::AuthDomain::records_t::value_type& record : zone.d_records) { records.push_back(Json::object{ - {"name", dr.d_name.toString()}, - {"type", DNSRecordContent::NumberToType(dr.d_type)}, - {"ttl", (double)dr.d_ttl}, - {"content", dr.getContent()->getZoneRepresentation()}}); + {"name", record.d_name.toString()}, + {"type", DNSRecordContent::NumberToType(record.d_type)}, + {"ttl", (double)record.d_ttl}, + {"content", record.getContent()->getZoneRepresentation()}}); } // id is the canonical lookup key, which doesn't actually match the name (in some cases) @@ -179,7 +180,7 @@ static void fillZone(const DNSName& zonename, HttpResponse* resp) resp->setJsonBody(doc); } -static void doCreateZone(const Json document) +static void doCreateZone(const Json& document) { if (::arg()["api-config-dir"].empty()) { throw ApiException("Config Option \"api-config-dir\" must be set"); @@ -191,18 +192,20 @@ static void doCreateZone(const Json document) string singleIPTarget = document["single_target_ip"].string_value(); string kind = toUpper(stringFromJson(document, "kind")); - bool rd = boolFromJson(document, "recursion_desired"); + bool rdFlag = boolFromJson(document, "recursion_desired"); string confbasename = "zone-" + apiZoneNameToId(zone); if (kind == "NATIVE") { - if (rd) + if (rdFlag) { throw ApiException("kind=Native and recursion_desired are mutually exclusive"); + } if (!singleIPTarget.empty()) { try { ComboAddress rem(singleIPTarget); - if (rem.sin4.sin_family != AF_INET) + if (rem.sin4.sin_family != AF_INET) { // NOLINT: union throw ApiException(""); - singleIPTarget = rem.toString(); + } + singleIPTarget = rem.toString(); // NOLINT: union } catch (...) { throw ApiException("Single IP target '" + singleIPTarget + "' is invalid"); @@ -226,25 +229,26 @@ static void doCreateZone(const Json document) else if (kind == "FORWARDED") { string serverlist; for (const auto& value : document["servers"].array_items()) { - string server = value.string_value(); - if (server == "") { + const string& server = value.string_value(); + if (server.empty()) { throw ApiException("Forwarded-to server must not be an empty string"); } try { - ComboAddress ca = parseIPAndPort(server, 53); + ComboAddress address = parseIPAndPort(server, 53); if (!serverlist.empty()) { serverlist += ";"; } - serverlist += ca.toStringWithPort(); + serverlist += address.toStringWithPort(); // NOLINT: union } catch (const PDNSException& e) { throw ApiException(e.reason); } } - if (serverlist == "") + if (serverlist.empty()) { throw ApiException("Need at least one upstream server when forwarding"); + } - if (rd) { + if (rdFlag) { apiWriteConfigFile(confbasename, "forward-zones-recurse+=" + zonename + "=" + serverlist); } else { @@ -289,8 +293,9 @@ static void apiServerZones(HttpRequest* req, HttpResponse* resp) DNSName zonename = apiNameToDNSName(stringFromJson(document, "name")); auto iter = SyncRes::t_sstorage.domainmap->find(zonename); - if (iter != SyncRes::t_sstorage.domainmap->end()) + if (iter != SyncRes::t_sstorage.domainmap->end()) { throw ApiException("Zone already exists"); + } doCreateZone(document); reloadZoneConfiguration(); @@ -299,15 +304,16 @@ static void apiServerZones(HttpRequest* req, HttpResponse* resp) return; } - if (req->method != "GET") + if (req->method != "GET") { throw HttpMethodNotAllowedException(); + } Json::array doc; for (const SyncRes::domainmap_t::value_type& val : *SyncRes::t_sstorage.domainmap) { const SyncRes::AuthDomain& zone = val.second; Json::array servers; for (const ComboAddress& server : zone.d_servers) { - servers.push_back(server.toStringWithPort()); + servers.push_back(server.toStringWithPort()); // NOLINT: union } // id is the canonical lookup key, which doesn't actually match the name (in some cases) string zoneId = apiZoneNameToId(val.first); @@ -326,9 +332,10 @@ static void apiServerZoneDetail(HttpRequest* req, HttpResponse* resp) { DNSName zonename = apiZoneIdToName(req->parameters["id"]); - SyncRes::domainmap_t::const_iterator iter = SyncRes::t_sstorage.domainmap->find(zonename); - if (iter == SyncRes::t_sstorage.domainmap->end()) + auto iter = SyncRes::t_sstorage.domainmap->find(zonename); + if (iter == SyncRes::t_sstorage.domainmap->end()) { throw ApiException("Could not find domain '" + zonename.toLogString() + "'"); + } if (req->method == "PUT") { Json document = req->json(); @@ -359,18 +366,20 @@ static void apiServerZoneDetail(HttpRequest* req, HttpResponse* resp) static void apiServerSearchData(HttpRequest* req, HttpResponse* resp) { - if (req->method != "GET") + if (req->method != "GET") { throw HttpMethodNotAllowedException(); + } - string q = req->getvars["q"]; - if (q.empty()) + string qVar = req->getvars["q"]; + if (qVar.empty()) { throw ApiException("Query q can't be blank"); + } Json::array doc; for (const SyncRes::domainmap_t::value_type& val : *SyncRes::t_sstorage.domainmap) { string zoneId = apiZoneNameToId(val.first); string zoneName = val.first.toString(); - if (pdns_ci_find(zoneName, q) != string::npos) { + if (pdns_ci_find(zoneName, qVar) != string::npos) { doc.push_back(Json::object{ {"type", "zone"}, {"zone_id", zoneId}, @@ -378,22 +387,23 @@ static void apiServerSearchData(HttpRequest* req, HttpResponse* resp) } // if zone name is an exact match, don't bother with returning all records/comments in it - if (val.first == DNSName(q)) { + if (val.first == DNSName(qVar)) { continue; } const SyncRes::AuthDomain& zone = val.second; - for (const SyncRes::AuthDomain::records_t::value_type& rr : zone.d_records) { - if (pdns_ci_find(rr.d_name.toString(), q) == string::npos && pdns_ci_find(rr.getContent()->getZoneRepresentation(), q) == string::npos) + for (const SyncRes::AuthDomain::records_t::value_type& resourceRec : zone.d_records) { + if (pdns_ci_find(resourceRec.d_name.toString(), qVar) == string::npos && pdns_ci_find(resourceRec.getContent()->getZoneRepresentation(), qVar) == string::npos) { continue; + } doc.push_back(Json::object{ {"type", "record"}, {"zone_id", zoneId}, {"zone_name", zoneName}, - {"name", rr.d_name.toString()}, - {"content", rr.getContent()->getZoneRepresentation()}}); + {"name", resourceRec.d_name.toString()}, + {"content", resourceRec.getContent()->getZoneRepresentation()}}); } } resp->setJsonBody(doc); @@ -401,13 +411,14 @@ static void apiServerSearchData(HttpRequest* req, HttpResponse* resp) static void apiServerCacheFlush(HttpRequest* req, HttpResponse* resp) { - if (req->method != "PUT") + if (req->method != "PUT") { throw HttpMethodNotAllowedException(); + } DNSName canon = apiNameToDNSName(req->getvars["domain"]); - bool subtree = (req->getvars.count("subtree") > 0 && req->getvars["subtree"].compare("true") == 0); + bool subtree = req->getvars.count("subtree") > 0 && req->getvars["subtree"] == "true"; uint16_t qtype = 0xffff; - if (req->getvars.count("type")) { + if (req->getvars.count("type") != 0) { qtype = QType::chartocode(req->getvars["type"].c_str()); } @@ -419,8 +430,9 @@ static void apiServerCacheFlush(HttpRequest* req, HttpResponse* resp) static void apiServerRPZStats(HttpRequest* req, HttpResponse* resp) { - if (req->method != "GET") + if (req->method != "GET") { throw HttpMethodNotAllowedException(); + } auto luaconf = g_luaconfs.getLocal(); auto numZones = luaconf->dfe.size(); @@ -429,12 +441,14 @@ static void apiServerRPZStats(HttpRequest* req, HttpResponse* resp) for (size_t i = 0; i < numZones; i++) { auto zone = luaconf->dfe.getZone(i); - if (zone == nullptr) + if (zone == nullptr) { continue; + } const auto& name = zone->getName(); auto stats = getRPZZoneStats(name); - if (stats == nullptr) + if (stats == nullptr) { continue; + } Json::object zoneInfo = { {"transfers_failed", (double)stats->d_failedTransfers}, {"transfers_success", (double)stats->d_successfulTransfers}, @@ -452,8 +466,9 @@ static void prometheusMetrics(HttpRequest* req, HttpResponse* resp) { static MetricDefinitionStorage s_metricDefinitions; - if (req->method != "GET") + if (req->method != "GET") { throw HttpMethodNotAllowedException(); + } std::ostringstream output; @@ -467,7 +482,7 @@ static void prometheusMetrics(HttpRequest* req, HttpResponse* resp) MetricDefinition metricDetails; if (s_metricDefinitions.getMetricDetails(metricName, metricDetails)) { - std::string prometheusTypeName = s_metricDefinitions.getPrometheusStringMetricType( + std::string prometheusTypeName = MetricDefinitionStorage::getPrometheusStringMetricType( metricDetails.d_prometheusType); if (prometheusTypeName.empty()) { @@ -508,18 +523,20 @@ static void serveStuff(HttpRequest* req, HttpResponse* resp) { resp->headers["Cache-Control"] = "max-age=86400"; - if (req->url.path == "/") + if (req->url.path == "/") { req->url.path = "/index.html"; +} const string charset = "; charset=utf-8"; - if (boost::ends_with(req->url.path, ".html")) + if (boost::ends_with(req->url.path, ".html")) { resp->headers["Content-Type"] = "text/html" + charset; - else if (boost::ends_with(req->url.path, ".css")) + } else if (boost::ends_with(req->url.path, ".css")) { resp->headers["Content-Type"] = "text/css" + charset; - else if (boost::ends_with(req->url.path, ".js")) + } else if (boost::ends_with(req->url.path, ".js")) { resp->headers["Content-Type"] = "application/javascript" + charset; - else if (boost::ends_with(req->url.path, ".png")) + } else if (boost::ends_with(req->url.path, ".png")) { resp->headers["Content-Type"] = "image/png"; + } resp->headers["X-Content-Type-Options"] = "nosniff"; resp->headers["X-Frame-Options"] = "deny"; @@ -528,8 +545,8 @@ static void serveStuff(HttpRequest* req, HttpResponse* resp) resp->headers["X-XSS-Protection"] = "1; mode=block"; // resp->headers["Content-Security-Policy"] = "default-src 'self'; style-src 'self' 'unsafe-inline'"; - if (!req->url.path.empty() && g_urlmap.count(req->url.path.c_str() + 1)) { - resp->body = g_urlmap.at(req->url.path.c_str() + 1); + if (!req->url.path.empty() && (g_urlmap.count(req->url.path.substr(1)) != 0)) { + resp->body = g_urlmap.at(req->url.path.substr(1)); resp->status = 200; } else { @@ -1172,9 +1189,8 @@ const std::map MetricDefinitionStorage::d_metrics "Number of remote logging events")}, }; -#define CHECK_PROMETHEUS_METRICS 0 +constexpr bool CHECK_PROMETHEUS_METRICS = false; -#if CHECK_PROMETHEUS_METRICS static void validatePrometheusMetrics() { MetricDefinitionStorage s_metricDefinitions; @@ -1196,13 +1212,12 @@ static void validatePrometheusMetrics() } } } -#endif RecursorWebServer::RecursorWebServer(FDMultiplexer* fdm) { -#if CHECK_PROMETHEUS_METRICS - validatePrometheusMetrics(); -#endif + if (CHECK_PROMETHEUS_METRICS) { + validatePrometheusMetrics(); + } d_ws = make_unique(fdm, arg()["webserver-address"], arg().asNum("webserver-port")); d_ws->setSLog(g_slog->withName("webserver")); @@ -1219,7 +1234,7 @@ RecursorWebServer::RecursorWebServer(FDMultiplexer* fdm) // legacy dispatch d_ws->registerApiHandler( - "/jsonstat", [this](HttpRequest* req, HttpResponse* resp) { jsonstat(req, resp); }, true); + "/jsonstat", [](HttpRequest* req, HttpResponse* resp) { jsonstat(req, resp); }, true); d_ws->registerApiHandler("/api/v1/servers/localhost/cache/flush", apiServerCacheFlush); d_ws->registerApiHandler("/api/v1/servers/localhost/config/allow-from", apiServerConfigAllowFrom); d_ws->registerApiHandler("/api/v1/servers/localhost/config/allow-notify-from", &apiServerConfigAllowNotifyFrom); @@ -1234,8 +1249,8 @@ RecursorWebServer::RecursorWebServer(FDMultiplexer* fdm) d_ws->registerApiHandler("/api/v1", apiDiscoveryV1); d_ws->registerApiHandler("/api", apiDiscovery); - for (const auto& u : g_urlmap) { - d_ws->registerWebHandler("/" + u.first, serveStuff); + for (const auto& url : g_urlmap) { + d_ws->registerWebHandler("/" + url.first, serveStuff); } d_ws->registerWebHandler("/", serveStuff); @@ -1247,7 +1262,7 @@ void RecursorWebServer::jsonstat(HttpRequest* req, HttpResponse* resp) { string command; - if (req->getvars.count("command")) { + if (req->getvars.count("command") != 0) { command = req->getvars["command"]; req->getvars.erase("command"); } @@ -1258,36 +1273,43 @@ void RecursorWebServer::jsonstat(HttpRequest* req, HttpResponse* resp) vector queries; bool filter = !req->getvars["public-filtered"].empty(); - if (req->getvars["name"] == "servfail-queries") + if (req->getvars["name"] == "servfail-queries") { queries = broadcastAccFunction>(pleaseGetServfailQueryRing); - else if (req->getvars["name"] == "bogus-queries") + } + else if (req->getvars["name"] == "bogus-queries") { queries = broadcastAccFunction>(pleaseGetBogusQueryRing); - else if (req->getvars["name"] == "queries") + } + else if (req->getvars["name"] == "queries") { queries = broadcastAccFunction>(pleaseGetQueryRing); + } typedef map counts_t; counts_t counts; - for (const query_t& q : queries) { - if (filter) - counts[pair(getRegisteredName(q.first), q.second)]++; - else - counts[pair(q.first, q.second)]++; + for (const query_t& count : queries) { + if (filter) { + counts[pair(getRegisteredName(count.first), count.second)]++; + } else { + counts[pair(count.first, count.second)]++; + } } typedef std::multimap rcounts_t; rcounts_t rcounts; - for (counts_t::const_iterator i = counts.begin(); i != counts.end(); ++i) - rcounts.emplace(-i->second, i->first); + for (const auto& count : counts) { + rcounts.emplace(-count.second, count.first); + } Json::array entries; - unsigned int tot = 0, totIncluded = 0; - for (const rcounts_t::value_type& q : rcounts) { - totIncluded -= q.first; + unsigned int tot = 0; + unsigned int totIncluded = 0; + for (const rcounts_t::value_type& count : rcounts) { + totIncluded -= count.first; entries.push_back(Json::array{ - -q.first, q.second.first.toLogString(), DNSRecordContent::NumberToType(q.second.second)}); - if (tot++ >= 100) + -count.first, count.second.first.toLogString(), DNSRecordContent::NumberToType(count.second.second)}); + if (tot++ >= 100) { break; +} } if (queries.size() != totIncluded) { entries.push_back(Json::array{ @@ -1296,39 +1318,42 @@ void RecursorWebServer::jsonstat(HttpRequest* req, HttpResponse* resp) resp->setJsonBody(Json::object{{"entries", entries}}); return; } - else if (command == "get-remote-ring") { + if (command == "get-remote-ring") { vector queries; - if (req->getvars["name"] == "remotes") + if (req->getvars["name"] == "remotes") { queries = broadcastAccFunction>(pleaseGetRemotes); - else if (req->getvars["name"] == "servfail-remotes") + } else if (req->getvars["name"] == "servfail-remotes") { queries = broadcastAccFunction>(pleaseGetServfailRemotes); - else if (req->getvars["name"] == "bogus-remotes") + } else if (req->getvars["name"] == "bogus-remotes") { queries = broadcastAccFunction>(pleaseGetBogusRemotes); - else if (req->getvars["name"] == "large-answer-remotes") + } else if (req->getvars["name"] == "large-answer-remotes") { queries = broadcastAccFunction>(pleaseGetLargeAnswerRemotes); - else if (req->getvars["name"] == "timeouts") + } else if (req->getvars["name"] == "timeouts") { queries = broadcastAccFunction>(pleaseGetTimeouts); - + } typedef map counts_t; counts_t counts; - for (const ComboAddress& q : queries) { - counts[q]++; + for (const ComboAddress& query : queries) { + counts[query]++; } typedef std::multimap rcounts_t; rcounts_t rcounts; - for (counts_t::const_iterator i = counts.begin(); i != counts.end(); ++i) - rcounts.emplace(-i->second, i->first); + for (const auto& count : counts) { + rcounts.emplace(-count.second, count.first); + } Json::array entries; - unsigned int tot = 0, totIncluded = 0; - for (const rcounts_t::value_type& q : rcounts) { - totIncluded -= q.first; + unsigned int tot = 0; + unsigned int totIncluded = 0; + for (const rcounts_t::value_type& count : rcounts) { + totIncluded -= count.first; entries.push_back(Json::array{ - -q.first, q.second.toString()}); - if (tot++ >= 100) + -count.first, count.second.toString()}); // NOLINT: union + if (tot++ >= 100) { break; + } } if (queries.size() != totIncluded) { entries.push_back(Json::array{ @@ -1338,14 +1363,12 @@ void RecursorWebServer::jsonstat(HttpRequest* req, HttpResponse* resp) resp->setJsonBody(Json::object{{"entries", entries}}); return; } - else { - resp->setErrorResult("Command '" + command + "' not found", 404); - } + resp->setErrorResult("Command '" + command + "' not found", 404); } -void AsyncServerNewConnectionMT(void* p) +void AsyncServerNewConnectionMT(void* arg) { - AsyncServer* server = (AsyncServer*)p; + auto* server = static_cast(arg); try { auto socket = server->accept(); // this is actually a shared_ptr @@ -1379,9 +1402,9 @@ void AsyncServer::newConnection() } // This is an entry point from FDM, so it needs to catch everything. -void AsyncWebServer::serveConnection(std::shared_ptr client) const +void AsyncWebServer::serveConnection(const std::shared_ptr& socket) const { - if (!client->acl(d_acl)) { + if (!socket->acl(d_acl)) { return; } @@ -1402,7 +1425,7 @@ void AsyncWebServer::serveConnection(std::shared_ptr client) const try { YaHTTP::AsyncRequestLoader yarl; yarl.initialize(&req); - client->setNonBlocking(); + socket->setNonBlocking(); const struct timeval timeout { @@ -1410,16 +1433,16 @@ void AsyncWebServer::serveConnection(std::shared_ptr client) const }; std::shared_ptr tlsCtx{nullptr}; if (d_loglevel > WebServer::LogLevel::None) { - client->getRemote(remote); + socket->getRemote(remote); } - auto handler = std::make_shared("", false, client->releaseHandle(), timeout, tlsCtx); + auto handler = std::make_shared("", false, socket->releaseHandle(), timeout, tlsCtx); PacketBuffer data; try { while (!req.complete) { auto ret = arecvtcp(data, 16384, handler, true); if (ret == LWResult::Result::Success) { - string str(reinterpret_cast(data.data()), data.size()); + string str(reinterpret_cast(data.data()), data.size()); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) req.complete = yarl.feed(str); } else { @@ -1438,10 +1461,10 @@ void AsyncWebServer::serveConnection(std::shared_ptr client) const logRequest(req, remote); WebServer::handleRequest(req, resp); - ostringstream ss; - resp.write(ss); - const string& s = ss.str(); - reply.insert(reply.end(), s.cbegin(), s.cend()); + ostringstream stringStream; + resp.write(stringStream); + const string& str = stringStream.str(); + reply.insert(reply.end(), str.cbegin(), str.cend()); logResponse(resp, remote, logprefix); @@ -1457,9 +1480,10 @@ void AsyncWebServer::serveConnection(std::shared_ptr client) const req.d_slog->error(Logr::Error, e.reason, "Exception handing request", "exception", Logging::Loggable("PDNSException"))); } catch (std::exception& e) { - if (strstr(e.what(), "timeout") == 0) + if (strstr(e.what(), "timeout") == nullptr) { SLOG(g_log << Logger::Error << logprefix << "STL Exception: " << e.what() << endl, req.d_slog->error(Logr::Error, e.what(), "Exception handing request", "exception", Logging::Loggable("std::exception"))); + } } catch (...) { SLOG(g_log << Logger::Error << logprefix << "Unknown exception" << endl, @@ -1476,10 +1500,12 @@ void AsyncWebServer::serveConnection(std::shared_ptr client) const void AsyncWebServer::go() { - if (!d_server) + if (!d_server) { return; + } auto server = std::dynamic_pointer_cast(d_server); - if (!server) + if (!server) { return; - server->asyncWaitForConnections(d_fdm, [this](const std::shared_ptr& c) { serveConnection(c); }); + } + server->asyncWaitForConnections(d_fdm, [this](const std::shared_ptr& socket) { serveConnection(socket); }); } diff --git a/pdns/recursordist/ws-recursor.hh b/pdns/recursordist/ws-recursor.hh index 1389b43f04e3..7821f10746ed 100644 --- a/pdns/recursordist/ws-recursor.hh +++ b/pdns/recursordist/ws-recursor.hh @@ -37,9 +37,9 @@ public: d_server_socket.setNonBlocking(); }; - friend void AsyncServerNewConnectionMT(void* p); + friend void AsyncServerNewConnectionMT(void* arg); - typedef std::function)> newconnectioncb_t; + using newconnectioncb_t = std::function&)>; void asyncWaitForConnections(FDMultiplexer* fdm, const newconnectioncb_t& callback); private: @@ -57,10 +57,10 @@ public: private: FDMultiplexer* d_fdm; - void serveConnection(std::shared_ptr socket) const; + void serveConnection(const std::shared_ptr& socket) const; protected: - virtual std::shared_ptr createServer() override + std::shared_ptr createServer() override { return std::make_shared(d_listenaddress, d_port); }; @@ -70,7 +70,7 @@ class RecursorWebServer : public boost::noncopyable { public: explicit RecursorWebServer(FDMultiplexer* fdm); - void jsonstat(HttpRequest* req, HttpResponse* resp); + static void jsonstat(HttpRequest* req, HttpResponse* resp); private: std::unique_ptr d_ws{nullptr}; diff --git a/pdns/secpoll.cc b/pdns/secpoll.cc index e798a3b61fe3..8ee4a59bbfbc 100644 --- a/pdns/secpoll.cc +++ b/pdns/secpoll.cc @@ -32,8 +32,9 @@ bool isReleaseVersion(const std::string &version) { } static void setSecPollToUnknownOnOK(int &secPollStatus) { - if(secPollStatus == 1) // it was ok, now it is unknown + if (secPollStatus == 1) { // it was ok, now it is unknown secPollStatus = 0; + } } void processSecPoll(const int res, const std::vector &ret, int &secPollStatus, std::string &secPollMessage) { @@ -44,15 +45,16 @@ void processSecPoll(const int res, const std::vector &ret, int &secPo } if (ret.empty()) { // empty NOERROR... wat? - if(secPollStatus == 1) // it was ok, now it is unknown + if (secPollStatus == 1) { // it was ok, now it is unknown secPollStatus = 0; + } throw PDNSException("Had empty answer on NOERROR RCODE"); } DNSRecord record; - for (auto const &r: ret) { - if (r.d_type == QType::TXT && r.d_place == DNSResourceRecord::Place::ANSWER) { - record = r; + for (auto const &records: ret) { + if (records.d_type == QType::TXT && records.d_place == DNSResourceRecord::Place::ANSWER) { + record = records; break; } } diff --git a/pdns/zonemd.cc b/pdns/zonemd.cc index 89cec85aa3b8..1e5973b8a861 100644 --- a/pdns/zonemd.cc +++ b/pdns/zonemd.cc @@ -36,20 +36,13 @@ void pdns::ZoneMD::readRecords(ZoneParserTNG& zpt) void pdns::ZoneMD::readRecords(const vector& records) { - for (auto& record : records) { + for (const auto& record : records) { readRecord(record); } } -void pdns::ZoneMD::readRecord(const DNSRecord& record) +void pdns::ZoneMD::processRecord(const DNSRecord& record) { - if (!record.d_name.isPartOf(d_zone) && record.d_name != d_zone) { - return; - } - if (record.d_class == QClass::IN && record.d_type == QType::SOA && d_soaRecordContent) { - return; - } - if (record.d_class == QClass::IN && record.d_name == d_zone) { switch (record.d_type) { case QType::SOA: { @@ -107,25 +100,38 @@ void pdns::ZoneMD::readRecord(const DNSRecord& record) if (param == nullptr) { throw PDNSException("Invalid NSEC3PARAM record"); } - if (g_maxNSEC3Iterations && param->d_iterations > g_maxNSEC3Iterations) { + if (g_maxNSEC3Iterations > 0 && param->d_iterations > g_maxNSEC3Iterations) { return; } d_nsec3params.emplace_back(param); d_nsec3label = d_zone; d_nsec3label.prependRawLabel(toBase32Hex(hashQNameWithSalt(param->d_salt, param->d_iterations, d_zone))); // Zap the NSEC3 at labels that we now know are not relevant - for (auto it = d_nsec3s.begin(); it != d_nsec3s.end();) { - if (it->first != d_nsec3label) { - it = d_nsec3s.erase(it); + for (auto item = d_nsec3s.begin(); item != d_nsec3s.end();) { + if (item->first != d_nsec3label) { + item = d_nsec3s.erase(item); } else { - ++it; + ++item; } } break; } } } +} + +void pdns::ZoneMD::readRecord(const DNSRecord& record) +{ + if (!record.d_name.isPartOf(d_zone) && record.d_name != d_zone) { + return; + } + if (record.d_class == QClass::IN && record.d_type == QType::SOA && d_soaRecordContent) { + return; + } + + processRecord(record); + // Until we have seen the NSEC3PARAM record, we save all of them, as we do not know the label for the zone yet if (record.d_class == QClass::IN && (d_nsec3label.empty() || record.d_name == d_nsec3label)) { switch (record.d_type) { @@ -154,7 +160,7 @@ void pdns::ZoneMD::readRecord(const DNSRecord& record) d_resourceRecordSetTTLs[key] = record.d_ttl; } -void pdns::ZoneMD::verify(bool& validationDone, bool& validationOK) +void pdns::ZoneMD::verify(bool& validationDone, bool& validationOK) // NOLINT { validationDone = false; validationOK = false; @@ -165,9 +171,10 @@ void pdns::ZoneMD::verify(bool& validationDone, bool& validationOK) // Get all records and remember RRSets and TTLs // Determine which digests to compute based on accepted zonemd records present - unique_ptr sha384digest{nullptr}, sha512digest{nullptr}; + unique_ptr sha384digest{nullptr}; + unique_ptr sha512digest{nullptr}; - for (const auto& it : d_zonemdRecords) { + for (const auto& item : d_zonemdRecords) { // The SOA Serial field MUST exactly match the ZONEMD Serial // field. If the fields do not match, digest verification MUST // NOT be considered successful with this ZONEMD RR. @@ -179,14 +186,14 @@ void pdns::ZoneMD::verify(bool& validationDone, bool& validationOK) // The Hash Algorithm field MUST be checked. If the verifier does // not support the given hash algorithm, verification MUST NOT be // considered successful with this ZONEMD RR. - const auto duplicate = it.second.duplicate; - const auto& r = it.second.record; - if (!duplicate && r->d_serial == d_soaRecordContent->d_st.serial && r->d_scheme == 1 && (r->d_hashalgo == 1 || r->d_hashalgo == 2)) { + const auto duplicate = item.second.duplicate; + const auto& record = item.second.record; + if (!duplicate && record->d_serial == d_soaRecordContent->d_st.serial && record->d_scheme == 1 && (record->d_hashalgo == 1 || record->d_hashalgo == 2)) { // A supported ZONEMD record - if (r->d_hashalgo == 1) { + if (record->d_hashalgo == 1) { sha384digest = make_unique(384); } - else if (r->d_hashalgo == 2) { + else if (record->d_hashalgo == 2) { sha512digest = make_unique(512); } } @@ -216,14 +223,14 @@ void pdns::ZoneMD::verify(bool& validationDone, bool& validationOK) } sortedRecords_t sorted; - for (auto& rr : rrset.second) { + for (auto& resourceRecord : rrset.second) { if (qtype == QType::RRSIG) { - const auto rrsig = std::dynamic_pointer_cast(rr); + const auto rrsig = std::dynamic_pointer_cast(resourceRecord); if (rrsig->d_type == QType::ZONEMD && qname == d_zone) { continue; } } - sorted.insert(rr); + sorted.insert(resourceRecord); } if (sorted.empty()) { diff --git a/pdns/zonemd.hh b/pdns/zonemd.hh index 3f20d52eaceb..e941a951f5bb 100644 --- a/pdns/zonemd.hh +++ b/pdns/zonemd.hh @@ -50,30 +50,32 @@ public: ValidationFailure }; - ZoneMD(const DNSName& zone) : - d_zone(zone) + ZoneMD(DNSName zone) : + d_zone(std::move(zone)) {} void readRecords(ZoneParserTNG& zpt); void readRecords(const std::vector& records); void readRecord(const DNSRecord& record); + void processRecord(const DNSRecord& record); void verify(bool& validationDone, bool& validationOK); // Return the zone's apex DNSKEYs - const std::set>& getDNSKEYs() const + [[nodiscard]] const std::set>& getDNSKEYs() const { return d_dnskeys; } // Return the zone's apex RRSIGs - const std::vector>& getRRSIGs() const + [[nodiscard]] const std::vector>& getRRSIGs() const { return d_rrsigs; } // Return the zone's apex ZONEMDs - std::vector> getZONEMDs() const + [[nodiscard]] std::vector> getZONEMDs() const { std::vector> ret; + ret.reserve(d_zonemdRecords.size()); for (const auto& zonemd : d_zonemdRecords) { ret.emplace_back(zonemd.second.record); } @@ -81,24 +83,24 @@ public: } // Return the zone's apex NSECs with signatures - const ContentSigPair& getNSECs() const + [[nodiscard]] const ContentSigPair& getNSECs() const { return d_nsecs; } // Return the zone's apex NSEC3s with signatures - const ContentSigPair& getNSEC3s() const + [[nodiscard]] const ContentSigPair& getNSEC3s() const { - const auto it = d_nsec3s.find(d_nsec3label); - return it == d_nsec3s.end() ? empty : d_nsec3s.at(d_nsec3label); + const auto item = d_nsec3s.find(d_nsec3label); + return item == d_nsec3s.end() ? empty : d_nsec3s.at(d_nsec3label); } - const DNSName& getNSEC3Label() const + [[nodiscard]] const DNSName& getNSEC3Label() const { return d_nsec3label; } - const std::vector>& getNSEC3Params() const + [[nodiscard]] const std::vector>& getNSEC3Params() const { return d_nsec3params; } @@ -109,16 +111,16 @@ private: struct CanonRRSetKeyCompare { - bool operator()(const RRSetKey_t& a, const RRSetKey_t& b) const + bool operator()(const RRSetKey_t& lhs, const RRSetKey_t& rhs) const { // FIXME surely we can be smarter here - if (a.first.canonCompare(b.first)) { + if (lhs.first.canonCompare(rhs.first)) { return true; } - if (b.first.canonCompare(a.first)) { + if (rhs.first.canonCompare(lhs.first)) { return false; } - return a.second < b.second; + return lhs.second < rhs.second; } };