From 01dfcfcecf454a7bdc92980403796f1d3a49d8a4 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Tue, 9 May 2023 11:07:18 -0700 Subject: [PATCH] QDnsLookup: merge some of the domain label expansion code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use qt_ACE_do directly from QtCore, to avoid going through Latin1 (US-ASCII) multiple times. Drive-by reduce the size of the buffers from PACKETSZ (512) to the maximum name a label can be (255 plus 1 for the null, just in case). Drive-by replace the last QString::fromWCharArray with QStringView, saving an unnecessary memory allocation before calling QtPrivate::convertToLatin1(). Change-Id: I3e3bfef633af4130a03afffd175d8be1feb5d74b Reviewed-by: Mårten Nordheim --- src/corelib/io/qurl_p.h | 4 +- src/network/kernel/qdnslookup.cpp | 12 ++++- src/network/kernel/qdnslookup_p.h | 14 +++++- src/network/kernel/qdnslookup_unix.cpp | 64 +++++++++++++------------- src/network/kernel/qdnslookup_win.cpp | 17 ++++--- 5 files changed, 66 insertions(+), 45 deletions(-) diff --git a/src/corelib/io/qurl_p.h b/src/corelib/io/qurl_p.h index 43dcdf82fb..9562d1993d 100644 --- a/src/corelib/io/qurl_p.h +++ b/src/corelib/io/qurl_p.h @@ -29,8 +29,8 @@ extern Q_AUTOTEST_EXPORT qsizetype qt_urlRecode(QString &appendTo, QStringView u // in qurlidna.cpp enum AceLeadingDot { AllowLeadingDot, ForbidLeadingDot }; enum AceOperation { ToAceOnly, NormalizeAce }; -extern QString qt_ACE_do(const QString &domain, AceOperation op, AceLeadingDot dot, - QUrl::AceProcessingOptions options); +QString Q_CORE_EXPORT qt_ACE_do(const QString &domain, AceOperation op, AceLeadingDot dot, + QUrl::AceProcessingOptions options = {}); extern Q_AUTOTEST_EXPORT void qt_punycodeEncoder(QStringView in, QString *output); extern Q_AUTOTEST_EXPORT QString qt_punycodeDecoder(const QString &pc); diff --git a/src/network/kernel/qdnslookup.cpp b/src/network/kernel/qdnslookup.cpp index 70fbb5bdd7..0356633735 100644 --- a/src/network/kernel/qdnslookup.cpp +++ b/src/network/kernel/qdnslookup.cpp @@ -1048,8 +1048,18 @@ QDnsTextRecord &QDnsTextRecord::operator=(const QDnsTextRecord &other) very fast and never fails. */ +static QDnsLookupRunnable::EncodedLabel encodeLabel(const QString &label) +{ + QString encodedLabel = qt_ACE_do(label, ToAceOnly, ForbidLeadingDot); +#ifdef Q_OS_WIN + return encodedLabel; +#else + return std::move(encodedLabel).toLatin1(); +#endif +} + inline QDnsLookupRunnable::QDnsLookupRunnable(const QDnsLookupPrivate *d) - : requestName(QUrl::toAce(d->name)), + : requestName(encodeLabel(d->name)), nameserver(d->nameserver), requestType(d->type), port(d->port) diff --git a/src/network/kernel/qdnslookup_p.h b/src/network/kernel/qdnslookup_p.h index 03767fd831..da4721411b 100644 --- a/src/network/kernel/qdnslookup_p.h +++ b/src/network/kernel/qdnslookup_p.h @@ -25,6 +25,7 @@ #include "QtNetwork/qdnslookup.h" #include "QtNetwork/qhostaddress.h" #include "private/qobject_p.h" +#include "private/qurl_p.h" QT_REQUIRE_CONFIG(dnslookup); @@ -174,6 +175,12 @@ class QDnsLookupRunnable : public QObject, public QRunnable Q_OBJECT public: +#ifdef Q_OS_WIN + using EncodedLabel = QString; +#else + using EncodedLabel = QByteArray; +#endif + QDnsLookupRunnable(const QDnsLookupPrivate *d); void run() override; @@ -181,8 +188,13 @@ signals: void finished(const QDnsLookupReply &reply); private: + template static QString decodeLabel(T encodedLabel) + { + return qt_ACE_do(encodedLabel.toString(), NormalizeAce, ForbidLeadingDot); + } void query(QDnsLookupReply *reply); - QByteArray requestName; + + EncodedLabel requestName; QHostAddress nameserver; QDnsLookup::Type requestType; quint16 port; diff --git a/src/network/kernel/qdnslookup_unix.cpp b/src/network/kernel/qdnslookup_unix.cpp index 296c703423..a7984f936c 100644 --- a/src/network/kernel/qdnslookup_unix.cpp +++ b/src/network/kernel/qdnslookup_unix.cpp @@ -219,18 +219,26 @@ void QDnsLookupRunnable::query(QDnsLookupReply *reply) if (header->rcode) return reply->makeDnsRcodeError(header->rcode); - char host[PACKETSZ], answer[PACKETSZ]; qptrdiff offset = sizeof(HEADER); unsigned char *response = buffer.data(); int status; + auto expandHost = [&](qptrdiff offset) { + char host[MAXCDNAME + 1]; + status = dn_expand(response, response + responseLength, response + offset, host, sizeof(host)); + if (status >= 0) + return decodeLabel(QLatin1StringView(host)); + + // failed + reply->makeInvalidReplyError(QDnsLookup::tr("Could not expand domain name")); + return QString(); + }; + if (ntohs(header->qdcount) == 1) { // Skip the query host, type (2 bytes) and class (2 bytes). - status = dn_expand(response, response + responseLength, response + offset, host, sizeof(host)); - if (status < 0) { - reply->makeInvalidReplyError(QDnsLookup::tr("Could not expand domain name")); + expandHost(offset); + if (status < 0) return; - } if (offset + status + 4 >= responseLength) header->qdcount = 0xffff; // invalid reply below else @@ -243,12 +251,9 @@ void QDnsLookupRunnable::query(QDnsLookupReply *reply) const int answerCount = ntohs(header->ancount); int answerIndex = 0; while ((offset < responseLength) && (answerIndex < answerCount)) { - status = dn_expand(response, response + responseLength, response + offset, host, sizeof(host)); - if (status < 0) { - reply->makeInvalidReplyError(QDnsLookup::tr("Could not expand domain name")); + const QString name = expandHost(offset); + if (status < 0) return; - } - const QString name = QUrl::fromAce(host); offset += status; if (offset + RRFIXEDSZ > responseLength) { @@ -283,57 +288,52 @@ void QDnsLookupRunnable::query(QDnsLookupReply *reply) record.d->value = QHostAddress(response + offset); reply->hostAddressRecords.append(record); } else if (type == QDnsLookup::CNAME) { - status = dn_expand(response, response + responseLength, response + offset, answer, sizeof(answer)); + QDnsDomainNameRecord record; + record.d->name = name; + record.d->timeToLive = ttl; + record.d->value = expandHost(offset); if (status < 0) return reply->makeInvalidReplyError(QDnsLookup::tr("Invalid canonical name record")); - QDnsDomainNameRecord record; - record.d->name = name; - record.d->timeToLive = ttl; - record.d->value = QUrl::fromAce(answer); reply->canonicalNameRecords.append(record); } else if (type == QDnsLookup::NS) { - status = dn_expand(response, response + responseLength, response + offset, answer, sizeof(answer)); + QDnsDomainNameRecord record; + record.d->name = name; + record.d->timeToLive = ttl; + record.d->value = expandHost(offset); if (status < 0) return reply->makeInvalidReplyError(QDnsLookup::tr("Invalid name server record")); - QDnsDomainNameRecord record; - record.d->name = name; - record.d->timeToLive = ttl; - record.d->value = QUrl::fromAce(answer); reply->nameServerRecords.append(record); } else if (type == QDnsLookup::PTR) { - status = dn_expand(response, response + responseLength, response + offset, answer, sizeof(answer)); - if (status < 0) - return reply->makeInvalidReplyError(QDnsLookup::tr("Invalid pointer record")); QDnsDomainNameRecord record; record.d->name = name; record.d->timeToLive = ttl; - record.d->value = QUrl::fromAce(answer); + record.d->value = expandHost(offset); + if (status < 0) + return reply->makeInvalidReplyError(QDnsLookup::tr("Invalid pointer record")); reply->pointerRecords.append(record); } else if (type == QDnsLookup::MX) { const quint16 preference = qFromBigEndian(response + offset); - status = dn_expand(response, response + responseLength, response + offset + 2, answer, sizeof(answer)); - if (status < 0) - return reply->makeInvalidReplyError(QDnsLookup::tr("Invalid mail exchange record")); QDnsMailExchangeRecord record; - record.d->exchange = QUrl::fromAce(answer); + record.d->exchange = expandHost(offset + 2); record.d->name = name; record.d->preference = preference; record.d->timeToLive = ttl; + if (status < 0) + return reply->makeInvalidReplyError(QDnsLookup::tr("Invalid mail exchange record")); reply->mailExchangeRecords.append(record); } else if (type == QDnsLookup::SRV) { const quint16 priority = qFromBigEndian(response + offset); const quint16 weight = qFromBigEndian(response + offset + 2); const quint16 port = qFromBigEndian(response + offset + 4); - status = dn_expand(response, response + responseLength, response + offset + 6, answer, sizeof(answer)); - if (status < 0) - return reply->makeInvalidReplyError(QDnsLookup::tr("Invalid service record")); QDnsServiceRecord record; record.d->name = name; - record.d->target = QUrl::fromAce(answer); + record.d->target = expandHost(offset + 6); record.d->port = port; record.d->priority = priority; record.d->timeToLive = ttl; record.d->weight = weight; + if (status < 0) + return reply->makeInvalidReplyError(QDnsLookup::tr("Invalid service record")); reply->serviceRecords.append(record); } else if (type == QDnsLookup::TXT) { QDnsTextRecord record; diff --git a/src/network/kernel/qdnslookup_win.cpp b/src/network/kernel/qdnslookup_win.cpp index 9ca8f25bbf..f6c47c2b74 100644 --- a/src/network/kernel/qdnslookup_win.cpp +++ b/src/network/kernel/qdnslookup_win.cpp @@ -66,11 +66,10 @@ QT_BEGIN_NAMESPACE void QDnsLookupRunnable::query(QDnsLookupReply *reply) { // Perform DNS query. - const QString requestNameUtf16 = QString::fromUtf8(requestName.data(), requestName.size()); alignas(DNS_ADDR_ARRAY) uchar dnsAddresses[sizeof(DNS_ADDR_ARRAY) + sizeof(DNS_ADDR)]; DNS_QUERY_REQUEST request = {}; request.Version = 1; - request.QueryName = reinterpret_cast(requestNameUtf16.constData()); + request.QueryName = reinterpret_cast(requestName.constData()); request.QueryType = requestType; request.QueryOptions = DNS_QUERY_STANDARD | DNS_QUERY_TREAT_AS_FQDN; @@ -98,7 +97,7 @@ void QDnsLookupRunnable::query(QDnsLookupReply *reply) // Extract results. for (PDNS_RECORD ptr = results.pQueryRecords; ptr != NULL; ptr = ptr->pNext) { - const QString name = QUrl::fromAce( QString::fromWCharArray( ptr->pName ).toLatin1() ); + const QString name = decodeLabel(ptr->pName); if (ptr->wType == QDnsLookup::A) { QDnsHostAddressRecord record; record.d->name = name; @@ -118,12 +117,12 @@ void QDnsLookupRunnable::query(QDnsLookupReply *reply) QDnsDomainNameRecord record; record.d->name = name; record.d->timeToLive = ptr->dwTtl; - record.d->value = QUrl::fromAce(QString::fromWCharArray(ptr->Data.Cname.pNameHost).toLatin1()); + record.d->value = decodeLabel(ptr->Data.Cname.pNameHost); reply->canonicalNameRecords.append(record); } else if (ptr->wType == QDnsLookup::MX) { QDnsMailExchangeRecord record; record.d->name = name; - record.d->exchange = QUrl::fromAce(QString::fromWCharArray(ptr->Data.Mx.pNameExchange).toLatin1()); + record.d->exchange = decodeLabel(QStringView(ptr->Data.Mx.pNameExchange)); record.d->preference = ptr->Data.Mx.wPreference; record.d->timeToLive = ptr->dwTtl; reply->mailExchangeRecords.append(record); @@ -131,18 +130,18 @@ void QDnsLookupRunnable::query(QDnsLookupReply *reply) QDnsDomainNameRecord record; record.d->name = name; record.d->timeToLive = ptr->dwTtl; - record.d->value = QUrl::fromAce(QString::fromWCharArray(ptr->Data.Ns.pNameHost).toLatin1()); + record.d->value = decodeLabel(QStringView(ptr->Data.Ns.pNameHost)); reply->nameServerRecords.append(record); } else if (ptr->wType == QDnsLookup::PTR) { QDnsDomainNameRecord record; record.d->name = name; record.d->timeToLive = ptr->dwTtl; - record.d->value = QUrl::fromAce(QString::fromWCharArray(ptr->Data.Ptr.pNameHost).toLatin1()); + record.d->value = decodeLabel(QStringView(ptr->Data.Ptr.pNameHost)); reply->pointerRecords.append(record); } else if (ptr->wType == QDnsLookup::SRV) { QDnsServiceRecord record; record.d->name = name; - record.d->target = QUrl::fromAce(QString::fromWCharArray(ptr->Data.Srv.pNameTarget).toLatin1()); + record.d->target = decodeLabel(QStringView(ptr->Data.Srv.pNameTarget)); record.d->port = ptr->Data.Srv.wPort; record.d->priority = ptr->Data.Srv.wPriority; record.d->timeToLive = ptr->dwTtl; @@ -153,7 +152,7 @@ void QDnsLookupRunnable::query(QDnsLookupReply *reply) record.d->name = name; record.d->timeToLive = ptr->dwTtl; for (unsigned int i = 0; i < ptr->Data.Txt.dwStringCount; ++i) { - record.d->values << QString::fromWCharArray((ptr->Data.Txt.pStringArray[i])).toLatin1();; + record.d->values << QStringView(ptr->Data.Txt.pStringArray[i]).toLatin1(); } reply->textRecords.append(record); }