QFormData(Part)Builder: simplify filename formatting

We don't need the bodyName member, because it's the same as
originalBodyName, just encoded, and we can delay the encoding to
build() time.

This is not worse than the old code, since we anyway toString() the
QAnyStringView unconditionally.

So we don't need to visit the QASV and implement RFC2232 encoding for
all three view types, we can just use the QString version, after
toString().

This not only has the advantage of less code and not storing duplicate
data, but we now also encode u8"ä.txt" the same as "ä.txt"_L1 and
u"ä.txt", ie. using latin1, as required by Postel's Law, and not as
UTF-8, as the old code did.

Change-Id: If82a33a1cd09b859b3a4450a60083b1d3aedf7bc
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
Reviewed-by: Marc Mutz <marc.mutz@qt.io>

src/network/access/qformdatabuilder.cpp

@@ -65,47 +65,11 @@ QFormDataPartBuilder::QFormDataPartBuilder(QLatin1StringView name, PrivateConstr
 QFormDataPartBuilder::~QFormDataPartBuilder()
     = default;
 
-static QByteArray buildFileName(QLatin1StringView view)
+
+static auto encodeFileName(QStringView view)
 {
-    QByteArray fileName;
-    fileName += "; filename";
-    QByteArrayView encoding = "=";
+    struct R { QByteArrayView encoding; QByteArray encoded; };
 
-    for (uchar c : view) {
-        if (c > 127) {
-            encoding = "*=ISO-8859-1''";
-            break;
-        }
-    }
-
-    fileName += encoding;
-    fileName += QByteArray::fromRawData(view.data(), view.size()).toPercentEncoding();
-    return fileName;
-}
-
-static QByteArray buildFileName(QUtf8StringView view)
-{
-    QByteArrayView bv = view;
-    QByteArray fileName;
-    fileName += "; filename";
-    QByteArrayView encoding = "=";
-
-    for (uchar c : bv) {
-        if (c > 127) {
-            encoding = "*=UTF-8''";
-            break;
-        }
-    }
-
-    fileName += encoding;
-    fileName += QByteArray::fromRawData(bv.data(), bv.size()).toPercentEncoding();
-    return fileName;
-}
-
-static QByteArray buildFileName(QStringView view)
-{
-    QByteArray fileName;
-    fileName += "; filename";
     QByteArrayView encoding = "=";
     bool needsUtf8 = false;
 
@@ -119,24 +83,12 @@ static QByteArray buildFileName(QStringView view)
         }
     }
 
-    fileName += encoding;
-
-    if (needsUtf8)
-        fileName += view.toUtf8().toPercentEncoding();
-    else
-        fileName += view.toLatin1().toPercentEncoding();
-
-    return fileName;
+    return R{encoding, needsUtf8 ? view.toUtf8() : view.toLatin1()};
 }
 
 QFormDataPartBuilder &QFormDataPartBuilder::setBodyHelper(const QByteArray &data,
                                                           QAnyStringView fileName)
 {
-    if (fileName.isEmpty())
-        m_bodyName = QByteArray();
-    else
-        m_bodyName = fileName.visit([&](auto name) { return buildFileName(name); });
-
     m_originalBodyName = fileName.toString();
     m_body = data;
     return *this;
@@ -181,11 +133,6 @@ QFormDataPartBuilder &QFormDataPartBuilder::setBody(QByteArrayView data,
 
 QFormDataPartBuilder &QFormDataPartBuilder::setBodyDevice(QIODevice *body, QAnyStringView fileName)
 {
-    if (fileName.isEmpty())
-        m_bodyName = QByteArray();
-    else
-        m_bodyName = fileName.visit([&](auto name) { return buildFileName(name); });
-
     m_originalBodyName = fileName.toString();
     m_body = body;
     return *this;
@@ -216,8 +163,11 @@ QHttpPart QFormDataPartBuilder::build()
 {
     QHttpPart httpPart;
 
-    if (!m_bodyName.isEmpty())
-        m_headerValue += m_bodyName;     // RFC 5987 Section 3.2.1
+    if (!m_originalBodyName.isNull()) {
+        const auto enc = encodeFileName(m_originalBodyName);
+        m_headerValue += "; filename" + enc.encoding
+                                      + enc.encoded.toPercentEncoding(); // RFC 5987 Section 3.2.1
+    }
 
 #if QT_CONFIG(mimetype)
     QMimeDatabase db;
@@ -234,7 +184,6 @@ QHttpPart QFormDataPartBuilder::build()
 #endif
     httpPart.setHeader(QNetworkRequest::ContentDispositionHeader, m_headerValue);
 
-
     if (auto d = std::get_if<QIODevice*>(&m_body))
         httpPart.setBodyDevice(*d);
     else if (auto b = std::get_if<QByteArray>(&m_body))

src/network/access/qformdatabuilder.h

@@ -36,7 +36,6 @@ public:
 
     QFormDataPartBuilder(QFormDataPartBuilder &&other) noexcept
         : m_headerValue(std::move(other.m_headerValue)),
-          m_bodyName(std::move(other.m_bodyName)),
           m_originalBodyName(std::move(other.m_originalBodyName)),
           m_httpHeaders(std::move(other.m_httpHeaders)),
           m_body(std::move(other.m_body)),
@@ -49,7 +48,6 @@ public:
     void swap(QFormDataPartBuilder &other) noexcept
     {
         m_headerValue.swap(other.m_headerValue);
-        m_bodyName.swap(other.m_bodyName);
         m_originalBodyName.swap(other.m_originalBodyName);
         m_httpHeaders.swap(other.m_httpHeaders);
         m_body.swap(other.m_body);
@@ -75,7 +73,6 @@ private:
     Q_NETWORK_EXPORT QHttpPart build();
 
     QByteArray m_headerValue;
-    QByteArray m_bodyName;
     QString m_originalBodyName;
     QHttpHeaders m_httpHeaders;
     std::variant<QIODevice*, QByteArray> m_body;

tests/auto/network/access/qformdatabuilder/tst_qformdatabuilder.cpp

@@ -195,7 +195,6 @@ void tst_QFormDataBuilder::picksUtf8EncodingOnlyIfL1OrAsciiDontSuffice()
     }
 
     QVERIFY(msg.contains(expected_content_type_data));
-    QEXPECT_FAIL("u8-latin", "will be fixed in subsequent patch", Continue);
     QVERIFY(msg.contains(expected_content_disposition_data));
 }