From: Miod Vallat Date: Mon, 1 Dec 2025 15:41:02 +0000 (+0100) Subject: Split patchZone() in smaller bits. X-Git-Tag: auth-5.0.2^2~10 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2aa1c52240b022391b4c3399502ebf0d06e50d5e;p=thirdparty%2Fpdns.git Split patchZone() in smaller bits. This is preparation work for future changes which would likely to trigger the "excessive cognitive complexity" diagnostic from clang-tidy, so cut ahead. Signed-off-by: Miod Vallat (cherry picked from commit a8e57021e4a69164445acad7b56862a7c04b5788) --- diff --git a/pdns/ws-auth.cc b/pdns/ws-auth.cc index 138499100b..100a4f6d2d 100644 --- a/pdns/ws-auth.cc +++ b/pdns/ws-auth.cc @@ -2340,27 +2340,32 @@ static void apiServerZoneRectify(HttpRequest* req, HttpResponse* resp) resp->setSuccessResult("Rectified"); } +// The allowed values for the "changetype" field of a Json patch record. +enum changeType +{ + DELETE, // delete complete RRset + REPLACE, // replace complete RRset +}; + // Validate the "changetype" field of a Json patch record. -// Returns true if this is a replace operation, false if this is a delete -// operation. +// Returns the recognized operation. // Throws an exception if unrecognized. -static bool validateChangeType(const Json& rrset) +static changeType validateChangeType(const std::string& changetype) { - string changetype = toUpper(stringFromJson(rrset, "changetype")); if (changetype == "DELETE") { - return false; + return DELETE; } if (changetype == "REPLACE") { - return true; + return REPLACE; } - throw ApiException("Changetype not understood"); + throw ApiException("Changetype '" + changetype + "' is not a valid value"); } // Replace the rrset for `qname' in zone `zonename' with the contents of // `new_records', making sure to remove no longer needed ENT entries, and // also enforcing the exclusivity rules (at most one CNAME, DNAME and SOA, // etc). -static void replaceZoneRecords(DomainInfo& domainInfo, const ZoneName& zonename, vector& new_records, const DNSName& qname, const QType qtype) +static void replaceZoneRecords(const DomainInfo& domainInfo, const ZoneName& zonename, vector& new_records, const DNSName& qname, const QType qtype) { bool ent_present = false; bool dname_seen = qtype == QType::DNAME; @@ -2400,96 +2405,131 @@ static void replaceZoneRecords(DomainInfo& domainInfo, const ZoneName& zonename, } } +// Parse the record name and type from a Json patch record. +static void parseRecordNameAndType(const Json& rrset, DNSName& qname, QType& qtype) +{ + qname = apiNameToDNSName(stringFromJson(rrset, "name")); + apiCheckQNameAllowedCharacters(qname.toString()); + qtype = stringFromJson(rrset, "type"); + if (qtype.getCode() == QType::ENT) { + throw ApiException("RRset " + qname.toString() + " IN " + stringFromJson(rrset, "type") + ": unknown type given"); + } +} + +// Apply a DELETE changetype. +static void applyDelete(const DomainInfo& domainInfo, DNSName& qname, QType& qtype) +{ + // Delete all matching qname/qtype RRs (and implicitly, comments). + if (!domainInfo.backend->replaceRRSet(domainInfo.id, qname, qtype, {})) { + throw ApiException("Hosting backend does not support editing records."); + } +} + +// Struct gathering the SOA edition details, so as not to pass billions of +// parameters to applyReplace() below. +struct soaEditSettings +{ + bool edit_done{false}; + string edit_api_kind; + string edit_kind; +}; + +// Apply a REPLACE changetype. +static bool applyReplace(const DomainInfo& domainInfo, const ZoneName& zonename, const Json& rrset, DNSName& qname, QType& qtype, soaEditSettings& soa) +{ + bool replace_records = rrset["records"].is_array(); + bool replace_comments = rrset["comments"].is_array(); + + if (!replace_records && !replace_comments) { + throw ApiException("No change for RRset " + qname.toString() + " IN " + qtype.toString()); + } + + vector new_records; + vector new_comments; + + try { + if (replace_records) { + // ttl shouldn't be required if we don't get new records. + uint32_t ttl = uintFromJson(rrset, "ttl"); + gatherRecords(rrset, qname, qtype, ttl, new_records); + + for (DNSResourceRecord& resourceRecord : new_records) { + resourceRecord.domain_id = static_cast(domainInfo.id); + if (resourceRecord.qtype.getCode() == QType::SOA && resourceRecord.qname == zonename.operator const DNSName&()) { + soa.edit_done = increaseSOARecord(resourceRecord, soa.edit_api_kind, soa.edit_kind, zonename); + } + } + checkNewRecords(new_records, zonename); + } + + if (replace_comments) { + gatherComments(rrset, qname, qtype, new_comments); + + for (Comment& comment : new_comments) { + comment.domain_id = static_cast(domainInfo.id); + } + } + } + catch (const JsonException& e) { + throw ApiException("New RRsets are invalid: " + string(e.what())); + } + + if (replace_records) { + replaceZoneRecords(domainInfo, zonename, new_records, qname, qtype); + } + if (replace_comments) { + if (!domainInfo.backend->replaceComments(domainInfo.id, qname, qtype, new_comments)) { + throw ApiException("Hosting backend does not support editing comments."); + } + } + return true; +} + static void patchZone(UeberBackend& backend, const ZoneName& zonename, DomainInfo& domainInfo, const vector& rrsets, HttpResponse* resp) { domainInfo.backend->startTransaction(zonename); try { - string soa_edit_api_kind; - string soa_edit_kind; - domainInfo.backend->getDomainMetadataOne(zonename, "SOA-EDIT-API", soa_edit_api_kind); - domainInfo.backend->getDomainMetadataOne(zonename, "SOA-EDIT", soa_edit_kind); - bool soa_edit_done = false; - - vector new_records; - vector new_comments; - set> seen; + soaEditSettings soa; + domainInfo.backend->getDomainMetadataOne(zonename, "SOA-EDIT-API", soa.edit_api_kind); + domainInfo.backend->getDomainMetadataOne(zonename, "SOA-EDIT", soa.edit_kind); + set> seen; for (const auto& rrset : rrsets) { - bool isReplaceOperation = validateChangeType(rrset); - DNSName qname = apiNameToDNSName(stringFromJson(rrset, "name")); - apiCheckQNameAllowedCharacters(qname.toString()); + string changetype = toUpper(stringFromJson(rrset, "changetype")); + auto operationType = validateChangeType(changetype); + DNSName qname; QType qtype; - qtype = stringFromJson(rrset, "type"); - if (qtype.getCode() == 0) { - throw ApiException("RRset " + qname.toString() + " IN " + stringFromJson(rrset, "type") + ": unknown type given"); - } + parseRecordNameAndType(rrset, qname, qtype); - if (seen.count({qname, qtype, isReplaceOperation}) != 0) { - throw ApiException("Duplicate RRset " + qname.toString() + " IN " + qtype.toString() + " with changetype: " + (isReplaceOperation ? "replace" : "delete")); + if (seen.count({qname, qtype, operationType}) != 0) { + throw ApiException("Duplicate RRset " + qname.toString() + " IN " + qtype.toString() + " with changetype: " + changetype); } - seen.insert({qname, qtype, isReplaceOperation}); + seen.insert({qname, qtype, operationType}); - if (!isReplaceOperation) { - // delete all matching qname/qtype RRs (and, implicitly comments). - if (!domainInfo.backend->replaceRRSet(domainInfo.id, qname, qtype, vector())) { - throw ApiException("Hosting backend does not support editing records."); - } - } - else { + if (operationType != DELETE) { if (domainInfo.kind == DomainInfo::Consumer) { // Allow deleting all RRsets, just not modifying them. throw ApiException("Modifying RRsets in Consumer zones is unsupported"); } - // we only validate for REPLACE, as DELETE can be used to "fix" out of zone records. + + // We intentionally do not perform this check for DELETE, as it can be + // used as a poor man's way to "fix" out-of-zone records. if (!qname.isPartOf(zonename)) { throw ApiException("RRset " + qname.toString() + " IN " + qtype.toString() + ": Name is out of zone"); } + } - bool replace_records = rrset["records"].is_array(); - bool replace_comments = rrset["comments"].is_array(); - - if (!replace_records && !replace_comments) { - throw ApiException("No change for RRset " + qname.toString() + " IN " + qtype.toString()); - } - - new_records.clear(); - new_comments.clear(); - - try { - if (replace_records) { - // ttl shouldn't be required if we don't get new records. - uint32_t ttl = uintFromJson(rrset, "ttl"); - gatherRecords(rrset, qname, qtype, ttl, new_records); - - for (DNSResourceRecord& resourceRecord : new_records) { - resourceRecord.domain_id = static_cast(domainInfo.id); - if (resourceRecord.qtype.getCode() == QType::SOA && resourceRecord.qname == zonename.operator const DNSName&()) { - soa_edit_done = increaseSOARecord(resourceRecord, soa_edit_api_kind, soa_edit_kind, zonename); - } - } - checkNewRecords(new_records, zonename); - } - - if (replace_comments) { - gatherComments(rrset, qname, qtype, new_comments); - - for (Comment& comment : new_comments) { - comment.domain_id = static_cast(domainInfo.id); - } - } - } - catch (const JsonException& e) { - throw ApiException("New RRsets are invalid: " + string(e.what())); - } - - if (replace_records) { - replaceZoneRecords(domainInfo, zonename, new_records, qname, qtype); - } - if (replace_comments) { - if (!domainInfo.backend->replaceComments(domainInfo.id, qname, qtype, new_comments)) { - throw ApiException("Hosting backend does not support editing comments."); - } + switch (operationType) { + case DELETE: + applyDelete(domainInfo, qname, qtype); + break; + case REPLACE: + if (!applyReplace(domainInfo, zonename, rrset, qname, qtype, soa)) { + // Proper error response has been setup, no need to do anything further. + domainInfo.backend->abortTransaction(); + return; } + break; } } @@ -2497,11 +2537,11 @@ static void patchZone(UeberBackend& backend, const ZoneName& zonename, DomainInf bool zone_disabled = (!backend.getSOAUncached(zonename, soaData)); // edit SOA (if needed) - if (!zone_disabled && !soa_edit_api_kind.empty() && !soa_edit_done) { + if (!zone_disabled && !soa.edit_api_kind.empty() && !soa.edit_done) { // return old serial in headers, before changing it resp->headers["X-PDNS-Old-Serial"] = std::to_string(soaData.serial); - updateZoneSerial(domainInfo, soaData, soa_edit_api_kind, soa_edit_kind); + updateZoneSerial(domainInfo, soaData, soa.edit_api_kind, soa.edit_kind); // return new serial in headers resp->headers["X-PDNS-New-Serial"] = std::to_string(soaData.serial);