Skip to content

Commit

Permalink
dnsname: clang-tidy fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@chbruyand
chbruyand committed Jan 18, 2024
1 parent 9de3bfd commit d83edfb
Show file tree
Hide file tree
Showing 2 changed files with 119 additions and 66 deletions.
181 changes: 117 additions & 64 deletions pdns/dnsname.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ const DNSName g_rootdnsname("."), g_wildcarddnsname("*");
www.powerdns.com = 3www8powerdns3com0
*/

std::ostream& operator<<(std::ostream& os, const DNSName& d)
std::ostream& operator<<(std::ostream& output, const DNSName& name)
{
return os << d.toLogString();
return output << name.toLogString();
}

void DNSName::throwSafeRangeError(const std::string& msg, const char* buf, size_t length)
Expand All @@ -58,36 +58,43 @@ DNSName::DNSName(const std::string_view sw)
const char* p = sw.data();
size_t length = sw.length();

if (length == 0 || (length == 1 && p[0] == '.')) {
if (length == 0 || (length == 1 && sw[0] == '.')) {
d_storage.assign(1, '\0');
}
else {
if (!std::memchr(p, '\\', length)) {
if (std::memchr(p, '\\', length) == nullptr) {
unsigned char lenpos = 0;
unsigned char labellen = 0;
const char *const pbegin = p, *pend = p + length;
const char* const pbegin = p;
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
const char* const pend = p + length;

d_storage.reserve(length + 1);
for (auto iter = pbegin; iter != pend;) {
for (const auto* iter = pbegin; iter != pend;) {
lenpos = d_storage.size();
if (*iter == '.')
if (*iter == '.') {
throwSafeRangeError("Found . in wrong position in DNSName: ", p, length);
}
d_storage.append(1, '\0');
labellen = 0;
auto begiter = iter;
const auto* begiter = iter;
for (; iter != pend && *iter != '.'; ++iter) {
labellen++;
}
d_storage.append(begiter, iter);
if (iter != pend)
if (iter != pend) {
++iter;
if (labellen > 63)
}
if (labellen > 63) {
throwSafeRangeError("label too long to append: ", p, length);
}

if (iter - pbegin > static_cast<ptrdiff_t>(s_maxDNSNameLength - 1)) // reserve two bytes, one for length and one for the root label
if (iter - pbegin > static_cast<ptrdiff_t>(s_maxDNSNameLength - 1)) { // reserve two bytes, one for length and one for the root label
throwSafeRangeError("name too long to append: ", p, length);
}

d_storage[lenpos] = labellen;
// NOLINTNEXTLINE(cppcoreguidelines-narrowing-conversions): labellen is checked above to be <= 63
d_storage[lenpos] = static_cast<char>(labellen);
}
d_storage.append(1, '\0');
}
Expand All @@ -106,74 +113,89 @@ DNSName::DNSName(const char* pos, size_t len, size_t offset, bool uncompress, ui
throw std::range_error("Trying to read past the end of the buffer (" + std::to_string(offset) + " >= " + std::to_string(len) + ")");

if (!uncompress) {
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
if (const void* fnd = memchr(pos + offset, 0, len - offset)) {
d_storage.reserve(2 + (const char*)fnd - (pos + offset));
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
d_storage.reserve(2 + static_cast<const char*>(fnd) - (pos + offset));
}
}

packetParser(pos, len, offset, uncompress, qtype, qclass, consumed, 0, minOffset);
}

// this should be the __only__ dns name parser in PowerDNS.
void DNSName::packetParser(const char* qpos, size_t len, size_t offset, bool uncompress, uint16_t* qtype, uint16_t* qclass, unsigned int* consumed, int depth, uint16_t minOffset)
void DNSName::packetParser(const char* qpos, size_t length, size_t offset, bool uncompress, uint16_t* qtype, uint16_t* qclass, unsigned int* consumed, int depth, uint16_t minOffset)
{
const unsigned char* pos = (const unsigned char*)qpos;
auto pos = reinterpret_cast<const unsigned char*>(qpos);

Check warning on line 129 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, auth) 

'auto pos' can be declared as 'const auto *pos' (readability-qualified-auto - Level=Warning)

Check warning on line 129 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, auth) 

do not use reinterpret_cast (cppcoreguidelines-pro-type-reinterpret-cast - Level=Warning)

Check warning on line 129 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, rec) 

'auto pos' can be declared as 'const auto *pos' (readability-qualified-auto - Level=Warning)

Check warning on line 129 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, rec) 

do not use reinterpret_cast (cppcoreguidelines-pro-type-reinterpret-cast - Level=Warning)

Check warning on line 129 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, dnsdist) 

'auto pos' can be declared as 'const auto *pos' (readability-qualified-auto - Level=Warning)

Check warning on line 129 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, dnsdist) 

do not use reinterpret_cast (cppcoreguidelines-pro-type-reinterpret-cast - Level=Warning)
unsigned char labellen;
const unsigned char* opos = pos;

if (offset >= len)
throw std::range_error("Trying to read past the end of the buffer (" + std::to_string(offset) + " >= " + std::to_string(len) + ")");
if (offset < (int)minOffset)
if (offset >= length) {
throw std::range_error("Trying to read past the end of the buffer (" + std::to_string(offset) + " >= " + std::to_string(length) + ")");
}
if (offset < static_cast<size_t>(minOffset)) {
throw std::range_error("Trying to read before the beginning of the buffer (" + std::to_string(offset) + " < " + std::to_string(minOffset) + ")");
}

const unsigned char* end = pos + len;
const unsigned char* end = pos + length;

Check warning on line 140 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, auth) 

do not use pointer arithmetic (cppcoreguidelines-pro-bounds-pointer-arithmetic - Level=Warning)

Check warning on line 140 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, rec) 

do not use pointer arithmetic (cppcoreguidelines-pro-bounds-pointer-arithmetic - Level=Warning)

Check warning on line 140 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, dnsdist) 

do not use pointer arithmetic (cppcoreguidelines-pro-bounds-pointer-arithmetic - Level=Warning)
pos += offset;
while ((labellen = *pos++) && pos < end) { // "scan and copy"
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
while ((labellen = *pos++) != 0u && pos < end) { // "scan and copy"

Check warning on line 143 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, auth) 

integer literal has suffix 'u', which is not uppercase (readability-uppercase-literal-suffix - Level=Warning)

Check warning on line 143 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, rec) 

integer literal has suffix 'u', which is not uppercase (readability-uppercase-literal-suffix - Level=Warning)

Check warning on line 143 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, dnsdist) 

integer literal has suffix 'u', which is not uppercase (readability-uppercase-literal-suffix - Level=Warning)
if (labellen >= 0xc0) {
if (!uncompress)
if (!uncompress) {
throw std::range_error("Found compressed label, instructed not to follow");
}

labellen &= (~0xc0);
int newpos = (labellen << 8) + *(const unsigned char*)pos;
size_t newpos = (labellen << 8) + *pos;

if (newpos < offset) {
if (newpos < (int)minOffset)
if (newpos < minOffset) {
throw std::range_error("Invalid label position during decompression (" + std::to_string(newpos) + " < " + std::to_string(minOffset) + ")");
if (++depth > 100)
}
if (++depth > 100) {
throw std::range_error("Abort label decompression after 100 redirects");
packetParser((const char*)opos, len, newpos, true, nullptr, nullptr, nullptr, depth, minOffset);
}
packetParser((const char*)opos, length, newpos, true, nullptr, nullptr, nullptr, depth, minOffset);

Check warning on line 159 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, auth) 

do not use C-style cast to convert between unrelated types (cppcoreguidelines-pro-type-cstyle-cast - Level=Warning)

Check warning on line 159 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, rec) 

do not use C-style cast to convert between unrelated types (cppcoreguidelines-pro-type-cstyle-cast - Level=Warning)

Check warning on line 159 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, dnsdist) 

do not use C-style cast to convert between unrelated types (cppcoreguidelines-pro-type-cstyle-cast - Level=Warning)
}
else
else {
throw std::range_error("Found a forward reference during label decompression");
}
pos++;
break;
}
else if (labellen & 0xc0) {
if ((labellen & 0xc0) != 0u) {

Check warning on line 167 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, auth) 

integer literal has suffix 'u', which is not uppercase (readability-uppercase-literal-suffix - Level=Warning)

Check warning on line 167 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, rec) 

integer literal has suffix 'u', which is not uppercase (readability-uppercase-literal-suffix - Level=Warning)

Check warning on line 167 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, dnsdist) 

integer literal has suffix 'u', which is not uppercase (readability-uppercase-literal-suffix - Level=Warning)
throw std::range_error("Found an invalid label length in qname (only one of the first two bits is set)");
}
if (pos + labellen < end) {
appendRawLabel((const char*)pos, labellen);
}
else
else {
throw std::range_error("Found an invalid label length in qname");
}
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
pos += labellen;
}
if (d_storage.empty())
if (d_storage.empty()) {
d_storage.append(1, (char)0); // we just parsed the root
if (consumed)
}
if (consumed != nullptr) {
*consumed = pos - opos - offset;
if (qtype) {
}
if (qtype != nullptr) {
if (pos + 2 > end) {
throw std::range_error("Trying to read qtype past the end of the buffer (" + std::to_string((pos - opos) + 2) + " > " + std::to_string(len) + ")");
throw std::range_error("Trying to read qtype past the end of the buffer (" + std::to_string((pos - opos) + 2) + " > " + std::to_string(length) + ")");
}
*qtype = (*(const unsigned char*)pos) * 256 + *((const unsigned char*)pos + 1);
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
*qtype = *pos * 256 + *(pos + 1);
}
pos += 2;
if (qclass) {
if (qclass != nullptr) {
if (pos + 2 > end) {
throw std::range_error("Trying to read qclass past the end of the buffer (" + std::to_string((pos - opos) + 2) + " > " + std::to_string(len) + ")");
throw std::range_error("Trying to read qclass past the end of the buffer (" + std::to_string((pos - opos) + 2) + " > " + std::to_string(length) + ")");
}
*qclass = (*(const unsigned char*)pos) * 256 + *((const unsigned char*)pos + 1);
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
*qclass = *pos * 256 + *(pos + 1);
}
}

Expand Down Expand Up @@ -253,23 +275,27 @@ size_t DNSName::wirelength() const
// Are WE part of parent
bool DNSName::isPartOf(const DNSName& parent) const
{
if (parent.empty() || empty())
if (parent.empty() || empty()) {
throw std::out_of_range("empty dnsnames aren't part of anything");
}

if (parent.d_storage.size() > d_storage.size())
if (parent.d_storage.size() > d_storage.size()) {
return false;
}

// this is slightly complicated since we can't start from the end, since we can't see where a label begins/ends then
for (auto us = d_storage.cbegin(); us < d_storage.cend(); us += *us + 1) {
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
for (const auto* us = d_storage.cbegin(); us < d_storage.cend(); us += *us + 1) {
auto distance = std::distance(us, d_storage.cend());
if (distance < 0 || static_cast<size_t>(distance) < parent.d_storage.size()) {
break;
}
if (static_cast<size_t>(distance) == parent.d_storage.size()) {
auto p = parent.d_storage.cbegin();
for (; us != d_storage.cend(); ++us, ++p) {
if (dns_tolower(*p) != dns_tolower(*us))
if (dns_tolower(*p) != dns_tolower(*us)) {
return false;
}
}
return true;
}
Expand Down Expand Up @@ -326,14 +352,15 @@ DNSName DNSName::labelReverse() const
{
DNSName ret;

if (isRoot())
if (isRoot()) {
return *this; // we don't create the root automatically below
}

if (!empty()) {
vector<string> l = getRawLabels();
while (!l.empty()) {
ret.appendRawLabel(l.back());
l.pop_back();
vector<string> labels = getRawLabels();
while (!labels.empty()) {
ret.appendRawLabel(labels.back());
labels.pop_back();
}
}
return ret;
Expand All @@ -346,12 +373,15 @@ void DNSName::appendRawLabel(const std::string& label)

void DNSName::appendRawLabel(const char* start, unsigned int length)
{
if (length == 0)
if (length == 0) {
throw std::range_error("no such thing as an empty label to append");
if (length > 63)
}
if (length > 63) {
throw std::range_error("label too long to append");
if (d_storage.size() + length > s_maxDNSNameLength - 1) // reserve one byte for the label length
}
if (d_storage.size() + length > s_maxDNSNameLength - 1) { // reserve one byte for the label length
throw std::range_error("name too long to append");
}

if (d_storage.empty()) {
d_storage.append(1, (char)length);
Expand All @@ -365,15 +395,19 @@ void DNSName::appendRawLabel(const char* start, unsigned int length)

void DNSName::prependRawLabel(const std::string& label)
{
if (label.empty())
if (label.empty()) {
throw std::range_error("no such thing as an empty label to prepend");
if (label.size() > 63)
}
if (label.size() > 63) {
throw std::range_error("label too long to prepend");
if (d_storage.size() + label.size() > s_maxDNSNameLength - 1) // reserve one byte for the label length
}
if (d_storage.size() + label.size() > s_maxDNSNameLength - 1) { // reserve one byte for the label length
throw std::range_error("name too long to prepend");
}

if (d_storage.empty())
if (d_storage.empty()) {
d_storage.append(1, (char)0);
}

string_t prep(1, (char)label.size());
prep.append(label.c_str(), label.size());
Expand All @@ -382,7 +416,8 @@ void DNSName::prependRawLabel(const std::string& label)

bool DNSName::slowCanonCompare(const DNSName& rhs) const
{
auto ours = getRawLabels(), rhsLabels = rhs.getRawLabels();
auto ours = getRawLabels();
auto rhsLabels = rhs.getRawLabels();
return std::lexicographical_compare(ours.rbegin(), ours.rend(), rhsLabels.rbegin(), rhsLabels.rend(), CIStringCompare());
}

Expand All @@ -391,19 +426,30 @@ vector<std::string> DNSName::getRawLabels() const
vector<std::string> ret;
ret.reserve(countLabels());
// 3www4ds9a2nl0
for (const unsigned char* p = (const unsigned char*)d_storage.c_str(); p < ((const unsigned char*)d_storage.c_str()) + d_storage.size() && *p; p += *p + 1) {
ret.push_back({(const char*)p + 1, (size_t)*p}); // XXX FIXME
const auto* pointer = reinterpret_cast<const unsigned char*>(d_storage.c_str());

Check warning on line 429 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, auth) 

do not use reinterpret_cast (cppcoreguidelines-pro-type-reinterpret-cast - Level=Warning)

Check warning on line 429 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, rec) 

do not use reinterpret_cast (cppcoreguidelines-pro-type-reinterpret-cast - Level=Warning)

Check warning on line 429 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, dnsdist) 

do not use reinterpret_cast (cppcoreguidelines-pro-type-reinterpret-cast - Level=Warning)
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
while (pointer < (reinterpret_cast<const unsigned char*>(d_storage.c_str()) + d_storage.size()) && *pointer != 0u) {

Check warning on line 431 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, auth) 

do not use reinterpret_cast (cppcoreguidelines-pro-type-reinterpret-cast - Level=Warning)

Check warning on line 431 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, auth) 

integer literal has suffix 'u', which is not uppercase (readability-uppercase-literal-suffix - Level=Warning)

Check warning on line 431 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, rec) 

do not use reinterpret_cast (cppcoreguidelines-pro-type-reinterpret-cast - Level=Warning)

Check warning on line 431 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, rec) 

integer literal has suffix 'u', which is not uppercase (readability-uppercase-literal-suffix - Level=Warning)

Check warning on line 431 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, dnsdist) 

do not use reinterpret_cast (cppcoreguidelines-pro-type-reinterpret-cast - Level=Warning)

Check warning on line 431 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, dnsdist) 

integer literal has suffix 'u', which is not uppercase (readability-uppercase-literal-suffix - Level=Warning)
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
ret.emplace_back(reinterpret_cast<const char*>(pointer) + 1, static_cast<size_t>(*pointer));

Check warning on line 433 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, auth) 

do not use reinterpret_cast (cppcoreguidelines-pro-type-reinterpret-cast - Level=Warning)

Check warning on line 433 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, rec) 

do not use reinterpret_cast (cppcoreguidelines-pro-type-reinterpret-cast - Level=Warning)

Check warning on line 433 in pdns/dnsname.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, dnsdist) 

do not use reinterpret_cast (cppcoreguidelines-pro-type-reinterpret-cast - Level=Warning)
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
pointer += *pointer + 1;
}
return ret;
}

std::string DNSName::getRawLabel(unsigned int pos) const
{
unsigned int currentPos = 0;
for (const unsigned char* p = (const unsigned char*)d_storage.c_str(); p < ((const unsigned char*)d_storage.c_str()) + d_storage.size() && *p; p += *p + 1, currentPos++) {
const auto* pointer = reinterpret_cast<const unsigned char*>(d_storage.c_str());
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
while (pointer < (reinterpret_cast<const unsigned char*>(d_storage.c_str()) + d_storage.size()) && *pointer != 0u) {
if (currentPos == pos) {
return std::string((const char*)p + 1, (size_t)*p);
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
return std::string(reinterpret_cast<const char*>(pointer) + 1, static_cast<size_t>(*pointer));
}
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
pointer += *pointer + 1;
++currentPos;
}

throw std::out_of_range("trying to get label at position " + std::to_string(pos) + " of a DNSName that only has " + std::to_string(currentPos) + " labels");
Expand All @@ -418,16 +464,18 @@ DNSName DNSName::getLastLabel() const

bool DNSName::chopOff()
{
if (d_storage.empty() || d_storage[0] == 0)
if (d_storage.empty() || d_storage[0] == 0) {
return false;
}
d_storage.erase(0, (unsigned int)d_storage[0] + 1);
return true;
}

bool DNSName::isWildcard() const
{
if (d_storage.size() < 2)
if (d_storage.size() < 2) {
return false;
}
auto p = d_storage.begin();
return (*p == 0x01 && *++p == '*');
}
Expand Down Expand Up @@ -457,8 +505,9 @@ unsigned int DNSName::countLabels() const

void DNSName::trimToLabels(unsigned int to)
{
while (countLabels() > to && chopOff())
while (countLabels() > to && chopOff()) {
;
}
}

size_t hash_value(DNSName const& d)
Expand All @@ -472,12 +521,15 @@ void DNSName::appendEscapedLabel(std::string& appendTo, const char* orig, size_t

while (pos < len) {
auto p = static_cast<uint8_t>(orig[pos]);
if (p == '.')
if (p == '.') {
appendTo += "\\.";
else if (p == '\\')
}
else if (p == '\\') {
appendTo += "\\\\";
else if (p > 0x20 && p < 0x7f)
}
else if (p > 0x20 && p < 0x7f) {
appendTo.append(1, (char)p);
}
else {
char buf[] = "000";
auto got = snprintf(buf, sizeof(buf), "%03" PRIu8, p);
Expand All @@ -500,8 +552,9 @@ bool DNSName::has8bitBytes() const
for (size_t idx = 0; idx < length; idx++) {
++pos;
char c = s.at(pos);
if (!((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '-' || c == '_' || c == '*' || c == '.' || c == '/' || c == '@' || c == ' ' || c == '\\' || c == ':'))
if (!((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '-' || c == '_' || c == '*' || c == '.' || c == '/' || c == '@' || c == ' ' || c == '\\' || c == ':')) {
return true;
}
}
++pos;
length = s.at(pos);
Expand Down
Loading

0 comments on commit d83edfb

Please sign in to comment.