Http: Fix POST-to-GET redirects still uploading or transmitting CL

CL = Content-Length

The uploadByteDevice was kept after a redirect which caused the
internals to assume that we had to upload the data. Even if this was
not the case we still transmitted the Content-Length header from the
first request which was now stored in two places.

Fixes: QTBUG-84162
Pick-to: 5.15
Change-Id: Ic86b1ef0766ffcc50beeed96c1c915b721d40209
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>

src/network/access/qhttpnetworkheader.cpp

@@ -117,6 +117,11 @@ void QHttpNetworkHeaderPrivate::prependHeaderField(const QByteArray &name, const
     fields.prepend(qMakePair(name, data));
 }
 
+void QHttpNetworkHeaderPrivate::clearHeaders()
+{
+    fields.clear();
+}
+
 bool QHttpNetworkHeaderPrivate::operator==(const QHttpNetworkHeaderPrivate &other) const
 {
    return (url == other.url);

src/network/access/qhttpnetworkheader_p.h

@@ -93,6 +93,7 @@ public:
     QList<QByteArray> headerFieldValues(const QByteArray &name) const;
     void setHeaderField(const QByteArray &name, const QByteArray &data);
     void prependHeaderField(const QByteArray &name, const QByteArray &data);
+    void clearHeaders();
     bool operator==(const QHttpNetworkHeaderPrivate &other) const;
 
 };

src/network/access/qhttpnetworkrequest.cpp

@@ -286,6 +286,11 @@ void QHttpNetworkRequest::prependHeaderField(const QByteArray &name, const QByte
     d->prependHeaderField(name, data);
 }
 
+void QHttpNetworkRequest::clearHeaders()
+{
+    d->clearHeaders();
+}
+
 QHttpNetworkRequest &QHttpNetworkRequest::operator=(const QHttpNetworkRequest &other)
 {
     d = other.d;

src/network/access/qhttpnetworkrequest_p.h

@@ -103,6 +103,7 @@ public:
     QByteArray headerField(const QByteArray &name, const QByteArray &defaultValue = QByteArray()) const override;
     void setHeaderField(const QByteArray &name, const QByteArray &data) override;
     void prependHeaderField(const QByteArray &name, const QByteArray &data);
+    void clearHeaders();
 
     Operation operation() const;
     void setOperation(Operation operation);

src/network/access/qnetworkreplyhttpimpl.cpp

@@ -1155,6 +1155,26 @@ void QNetworkReplyHttpImplPrivate::onRedirected(const QUrl &redirectUrl, int htt
     redirectRequest = createRedirectRequest(originalRequest, url, maxRedirectsRemaining);
     operation = getRedirectOperation(operation, httpStatus);
 
+    // Clear stale headers, the relevant ones get set again later
+    httpRequest.clearHeaders();
+    if (operation == QNetworkAccessManager::GetOperation
+        || operation == QNetworkAccessManager::HeadOperation) {
+        // possibly changed from not-GET/HEAD to GET/HEAD, make sure to get rid of upload device
+        uploadByteDevice.reset();
+        uploadByteDevicePosition = 0;
+        if (outgoingData) {
+            QObject::disconnect(outgoingData, SIGNAL(readyRead()), q,
+                                SLOT(_q_bufferOutgoingData()));
+            QObject::disconnect(outgoingData, SIGNAL(readChannelFinished()), q,
+                                SLOT(_q_bufferOutgoingDataFinished()));
+        }
+        outgoingData = nullptr;
+        outgoingDataBuffer.reset();
+        // We need to explicitly unset these headers so they're not reapplied to the httpRequest
+        redirectRequest.setHeader(QNetworkRequest::ContentLengthHeader, QVariant());
+        redirectRequest.setHeader(QNetworkRequest::ContentTypeHeader, QVariant());
+    }
+
     if (const QNetworkCookieJar *const cookieJar = manager->cookieJar()) {
         auto cookies = cookieJar->cookiesForUrl(url);
         if (!cookies.empty()) {
@@ -1435,6 +1455,9 @@ void QNetworkReplyHttpImplPrivate::resetUploadDataSlot(bool *r)
 // Coming from QNonContiguousByteDeviceThreadForwardImpl in HTTP thread
 void QNetworkReplyHttpImplPrivate::sentUploadDataSlot(qint64 pos, qint64 amount)
 {
+    if (!uploadByteDevice) // uploadByteDevice is no longer available
+        return;
+
     if (uploadByteDevicePosition + amount != pos) {
         // Sanity check, should not happen.
         error(QNetworkReply::UnknownNetworkError, QString());
@@ -1449,6 +1472,9 @@ void QNetworkReplyHttpImplPrivate::wantUploadDataSlot(qint64 maxSize)
 {
     Q_Q(QNetworkReplyHttpImpl);
 
+    if (!uploadByteDevice) // uploadByteDevice is no longer available
+        return;
+
     // call readPointer
     qint64 currentUploadDataLength = 0;
     char *data = const_cast<char*>(uploadByteDevice->readPointer(maxSize, currentUploadDataLength));

tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp

@@ -492,6 +492,8 @@ private Q_SLOTS:
     void ioHttpRedirectMultipartPost();
     void ioHttpRedirectDelete();
     void ioHttpRedirectCustom();
+    void ioHttpRedirectWithUploadDevice_data();
+    void ioHttpRedirectWithUploadDevice();
 #ifndef QT_NO_SSL
     void putWithServerClosingConnectionImmediately();
 #endif
@@ -715,7 +717,8 @@ public slots:
 
         if (doubleEndlPos != -1) {
             const int endOfHeader = doubleEndlPos + 4;
-            hasContent = receivedData.startsWith("POST") || receivedData.startsWith("PUT");
+            hasContent = receivedData.startsWith("POST") || receivedData.startsWith("PUT")
+                    || receivedData.startsWith("CUSTOM_WITH_PAYLOAD");
             if (hasContent && contentLength == 0)
                 parseContentLength();
             contentRead = receivedData.length() - endOfHeader;
@@ -8780,6 +8783,75 @@ void tst_QNetworkReply::ioHttpRedirectCustom()
     QVERIFY2(target.receivedData.startsWith("CUSTOM"), "Target server called with the wrong method");
 }
 
+void tst_QNetworkReply::ioHttpRedirectWithUploadDevice_data()
+{
+    QTest::addColumn<QByteArray>("method");
+    QTest::addColumn<QString>("status");
+    QTest::addColumn<bool>("keepMethod");
+
+    QTest::addRow("post-301") << QByteArray("POST") << "301 Moved Permanently" << false;
+    QTest::addRow("post-307") << QByteArray("POST") << "307 Temporary Redirect" << true;
+
+    const QByteArray customVerb = QByteArray("CUSTOM_WITH_PAYLOAD");
+    QTest::addRow("custom-301") << customVerb << "301 Moved Permanently" << false;
+    QTest::addRow("custom-307") << customVerb << "307 Temporary Redirect" << true;
+}
+
+/*
+    Tests that we properly disregard the upload device when redirecting
+    and changing method to GET, and that it gets reset properly when
+    redirecting (without changing method) for POST and a custom method.
+*/
+void tst_QNetworkReply::ioHttpRedirectWithUploadDevice()
+{
+    QFETCH(QByteArray, method);
+    QFETCH(QString, status);
+    QFETCH(bool, keepMethod);
+
+    MiniHttpServer target(httpEmpty200Response, false);
+    QUrl targetUrl("http://localhost/");
+    targetUrl.setPort(target.serverPort());
+
+    QString redirectReply = QStringLiteral("HTTP/1.1 %1\r\n"
+                                           "Content-Type: text/plain\r\n"
+                                           "location: %2\r\n"
+                                           "\r\n").arg(status, targetUrl.toString());
+    MiniHttpServer redirectServer(redirectReply.toLatin1());
+    QUrl url("http://localhost/");
+    url.setPort(redirectServer.serverPort());
+
+    QNetworkRequest request(url);
+    request.setHeader(QNetworkRequest::ContentTypeHeader, QByteArray("text/plain"));
+    auto oldRedirectPolicy = manager.redirectPolicy();
+    manager.setRedirectPolicy(QNetworkRequest::RedirectPolicy::NoLessSafeRedirectPolicy);
+
+    QByteArray data = "Hello world";
+    QBuffer buffer(&data);
+
+    QNetworkReplyPtr reply;
+    if (method == "POST")
+        reply.reset(manager.post(request, &buffer));
+    else
+        reply.reset(manager.sendCustomRequest(request, method, &buffer));
+    // Restore previous policy:
+    manager.setRedirectPolicy(oldRedirectPolicy);
+
+    QCOMPARE(waitForFinish(reply), int(Success));
+    QByteArray expectedMethod = method;
+    if (!keepMethod)
+        expectedMethod = "GET";
+    QVERIFY2(target.receivedData.startsWith(expectedMethod), "Target server called with the wrong method");
+    if (keepMethod) { // keepMethod also means we resend the data
+        QVERIFY2(target.receivedData.endsWith(data), "Target server didn't receive the data");
+    } else {
+        // we shouldn't send Content-Length with not content (esp. for GET)
+        QVERIFY2(!target.receivedData.contains("Content-Length"),
+                 "Target server should not have received a Content-Length header");
+        QVERIFY2(!target.receivedData.contains("Content-Type"),
+                 "Target server should not have received a Content-Type header");
+    }
+}
+
 #ifndef QT_NO_SSL
 
 class PutWithServerClosingConnectionImmediatelyHandler: public QObject