From d8f6425fef1050525480afec662a417a7645c22e Mon Sep 17 00:00:00 2001 From: Juha Vuolle Date: Tue, 27 Feb 2024 10:43:11 +0200 Subject: [PATCH] Add a QHttpHeaders convenience method for unique header name setting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The function replaces one of the found entries with the new value, and removes any other entries. If no entries are found, a new entry will be appended. The replacement search is done for performance reasons; it's cheaper to replace an existing value. All in all the function is a more convenient and performant alternative for this sequence (which proved to be common while porting QtNetwork internals to QHttpHeaders): header.removeAll(); header.append(, ); [ChangeLog][QtNetwork][QHttpHeaders] Added replaceOrAppend() convenience method, which either replaces previous entries with a single entry, or appends a new one if no entries existed Fixes: QTBUG-122175 Change-Id: I03957645d7e916a732ac7b8d3ae724bb6b16af87 Reviewed-by: Lena Biliaieva Reviewed-by: Mårten Nordheim --- src/network/access/qhttpheaders.cpp | 59 ++++++++++++++++++ src/network/access/qhttpheaders.h | 3 + .../access/qhttpheaders/tst_qhttpheaders.cpp | 60 +++++++++++++++++++ 3 files changed, 122 insertions(+) diff --git a/src/network/access/qhttpheaders.cpp b/src/network/access/qhttpheaders.cpp index 4a65e662cf7..c63da899a84 100644 --- a/src/network/access/qhttpheaders.cpp +++ b/src/network/access/qhttpheaders.cpp @@ -789,6 +789,7 @@ public: // we can define common methods which 'detach()' the private itself. using Self = QExplicitlySharedDataPointer; static void removeAll(Self &d, const HeaderName &name); + static void replaceOrAppend(Self &d, const HeaderName &name, const QByteArray &value); void combinedValue(const HeaderName &name, QByteArray &result) const; void values(const HeaderName &name, QList &result) const; @@ -852,6 +853,23 @@ QByteArrayView QHttpHeadersPrivate::value(const HeaderName &name, QByteArrayView return defaultValue; } +void QHttpHeadersPrivate::replaceOrAppend(Self &d, const HeaderName &name, const QByteArray &value) +{ + d.detach(); + auto it = std::find_if(d->headers.begin(), d->headers.end(), headerNameMatches(name)); + if (it != d->headers.end()) { + // Found something to replace => replace, and then rearrange any remaining + // matches to the end and erase them + it->value = value; + d->headers.erase( + std::remove_if(it + 1, d->headers.end(), headerNameMatches(name)), + d->headers.end()); + } else { + // Found nothing to replace => append + d->headers.append(Header{name, value}); + } +} + /*! Creates a new QHttpHeaders object. */ @@ -1216,6 +1234,47 @@ bool QHttpHeaders::replace(qsizetype i, WellKnownHeader name, QAnyStringView new return true; } +/*! + \since 6.8 + + If QHttpHeaders already contains \a name, replaces its value with + \a newValue and removes possible additional \a name entries. + If \a name didn't exist, appends a new entry. Returns \c true + if successful. + + This function is a convenience method for setting a unique + \a name : \a newValue header. For most headers the relative order does not + matter, which allows reusing an existing entry if one exists. + + \sa replaceOrAppend(QAnyStringView, QAnyStringView) +*/ +bool QHttpHeaders::replaceOrAppend(WellKnownHeader name, QAnyStringView newValue) +{ + if (isEmpty()) + return append(name, newValue); + + if (!isValidHttpHeaderValueField(newValue)) + return false; + + QHttpHeadersPrivate::replaceOrAppend(d, HeaderName{name}, normalizedValue(newValue)); + return true; +} + +/*! + \overload replaceOrAppend(WellKnownHeader, QAnyStringView) +*/ +bool QHttpHeaders::replaceOrAppend(QAnyStringView name, QAnyStringView newValue) +{ + if (isEmpty()) + return append(name, newValue); + + if (!isValidHttpHeaderNameField(name) || !isValidHttpHeaderValueField(newValue)) + return false; + + QHttpHeadersPrivate::replaceOrAppend(d, HeaderName{name}, normalizedValue(newValue)); + return true; +} + /*! Returns whether the headers contain header with \a name. diff --git a/src/network/access/qhttpheaders.h b/src/network/access/qhttpheaders.h index fb426eed809..97dc415e55e 100644 --- a/src/network/access/qhttpheaders.h +++ b/src/network/access/qhttpheaders.h @@ -219,6 +219,9 @@ public: Q_NETWORK_EXPORT bool replace(qsizetype i, QAnyStringView name, QAnyStringView newValue); Q_NETWORK_EXPORT bool replace(qsizetype i, WellKnownHeader name, QAnyStringView newValue); + Q_NETWORK_EXPORT bool replaceOrAppend(QAnyStringView name, QAnyStringView newValue); + Q_NETWORK_EXPORT bool replaceOrAppend(WellKnownHeader name, QAnyStringView newValue); + Q_NETWORK_EXPORT bool contains(QAnyStringView name) const; Q_NETWORK_EXPORT bool contains(WellKnownHeader name) const; diff --git a/tests/auto/network/access/qhttpheaders/tst_qhttpheaders.cpp b/tests/auto/network/access/qhttpheaders/tst_qhttpheaders.cpp index 4401a5c4950..457d30feeb8 100644 --- a/tests/auto/network/access/qhttpheaders/tst_qhttpheaders.cpp +++ b/tests/auto/network/access/qhttpheaders/tst_qhttpheaders.cpp @@ -21,6 +21,7 @@ private slots: void headerNameField(); void headerValueField(); void valueEncoding(); + void replaceOrAppend(); private: static constexpr QAnyStringView n1{"name1"}; @@ -488,5 +489,64 @@ void tst_QHttpHeaders::valueEncoding() QCOMPARE(h1.values(n1).at(0), "foo%E2%82%AC"); } +void tst_QHttpHeaders::replaceOrAppend() +{ + QHttpHeaders h1; + +#define REPLACE_OR_APPEND(NAME, VALUE, INDEX, TOTALSIZE) \ + do { \ + QVERIFY(h1.replaceOrAppend(NAME, VALUE)); \ + QCOMPARE(h1.size(), TOTALSIZE); \ + QCOMPARE(h1.nameAt(INDEX), NAME); \ + QCOMPARE(h1.valueAt(INDEX), VALUE); \ + } while (false) + + // Append to empty container and replace it + REPLACE_OR_APPEND(n1, v1, 0, 1); // Appends + REPLACE_OR_APPEND(n1, v2, 0, 1); // Replaces + + // Replace at beginning, middle, and end + h1.clear(); + REPLACE_OR_APPEND(n1, v1, 0, 1); // Appends + REPLACE_OR_APPEND(n2, v2, 1, 2); // Appends + REPLACE_OR_APPEND(n3, v3, 2, 3); // Appends + REPLACE_OR_APPEND(n1, V1, 0, 3); // Replaces at beginning + REPLACE_OR_APPEND(n2, V2, 1, 3); // Replaces at middle + REPLACE_OR_APPEND(n3, V3, 2, 3); // Replaces at end + + // Pre-existing multiple values (n2) are removed + h1.clear(); + h1.append(n1, v1); + h1.append(n2, v2); // First n2 is at index 1 + h1.append(n2, v2); + h1.append(n3, v3); + h1.append(n2, v2); + QCOMPARE(h1.size(), 5); + QCOMPARE(h1.combinedValue(n2), "value2, value2, value2"); + REPLACE_OR_APPEND(n2, V2, 1, 3); // Replaces value at index 1, and removes the rest + QCOMPARE(h1.combinedValue(n2), "VALUE2"); +#undef REPLACE_OR_APPEND + + // Implicit sharing / detaching + h1.clear(); + h1.append(n1, v1); + QHttpHeaders h2 = h1; + QCOMPARE(h1.size(), h2.size()); + QCOMPARE(h1.valueAt(0), h2.valueAt(0)); // Iniially values are equal + h1.replaceOrAppend(n1, v2); // Change value in h1 => detaches h1 + QCOMPARE_NE(h1.valueAt(0), h2.valueAt(0)); // Values are no more equal + QCOMPARE(h1.valueAt(0), v2); // Value in h1 changed + QCOMPARE(h2.valueAt(0), v1); // Value in h2 remained + + // Failed attempts + h1.clear(); + h1.append(n1, v1); + QRegularExpression re("HTTP header*"); + QTest::ignoreMessage(QtMsgType::QtWarningMsg, re); + QVERIFY(!h1.replaceOrAppend("", V1)); + QTest::ignoreMessage(QtMsgType::QtWarningMsg, re); + QVERIFY(!h1.replaceOrAppend(v1, "foo\x08")); +} + QTEST_MAIN(tst_QHttpHeaders) #include "tst_qhttpheaders.moc"