QNetworkReply: update decompress error message and handling

The error message was quite vague since it would then not require
any additional translations. However, in hindsight this was a mistake
since now developers just thought their downloads were being corrupted.

Pick-to: 6.2 6.3
Fixes: QTBUG-101942
Change-Id: Ie9af42510ca027d15248e5bcf21e836e709898d9
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>

src/network/access/qdecompresshelper.cpp

@@ -41,6 +41,7 @@
 
 #include <QtCore/private/qbytearray_p.h>
 #include <QtCore/qiodevice.h>
+#include <QtCore/qcoreapplication.h>
 
 #include <limits>
 #include <zlib.h>
@@ -131,13 +132,16 @@ bool QDecompressHelper::setEncoding(const QByteArray &encoding)
     Q_ASSERT(contentEncoding == QDecompressHelper::None);
     if (contentEncoding != QDecompressHelper::None) {
         qWarning("Encoding is already set.");
+        // This isn't an error, so it doesn't set errorStr, it's just wrong usage.
         return false;
     }
     ContentEncoding ce = encodingFromByteArray(encoding);
     if (ce == None) {
-        qWarning("An unsupported content encoding was selected: %s", encoding.data());
+        errorStr = QCoreApplication::translate("QHttp", "Unsupported content encoding: %1")
+                           .arg(QLatin1String(encoding));
         return false;
     }
+    errorStr = QString(); // clear error
     return setEncoding(ce);
 }
 
@@ -179,7 +183,8 @@ bool QDecompressHelper::setEncoding(ContentEncoding ce)
         break;
     }
     if (!decoderPointer) {
-        qWarning("Failed to initialize the decoder.");
+        errorStr = QCoreApplication::translate("QHttp",
+                                               "Failed to initialize the compression decoder.");
         contentEncoding = QDecompressHelper::None;
         return false;
     }
@@ -435,8 +440,13 @@ qsizetype QDecompressHelper::readInternal(char *data, qsizetype maxSize)
         clear();
 
     totalUncompressedBytes += bytesRead;
-    if (isPotentialArchiveBomb())
+    if (isPotentialArchiveBomb()) {
+        errorStr = QCoreApplication::translate(
+                "QHttp",
+                "The decompressed output exceeds the limits specified by "
+                "QNetworkRequest::decompressedSafetyCheckThreshold()");
         return -1;
+    }
 
     return bytesRead;
 }
@@ -517,11 +527,29 @@ qint64 QDecompressHelper::encodedBytesAvailable() const
     return compressedDataBuffer.byteAmount();
 }
 
+/*!
+    \internal
+    Returns whether or not the object is valid.
+    If it becomes invalid after an operation has been performed
+    then an error has occurred.
+    \sa errorString()
+*/
 bool QDecompressHelper::isValid() const
 {
     return contentEncoding != None;
 }
 
+/*!
+    \internal
+    Returns a string describing the error that occurred or an empty
+    string if no error occurred.
+    \sa isValid()
+*/
+QString QDecompressHelper::errorString() const
+{
+    return errorStr;
+}
+
 void QDecompressHelper::clear()
 {
     switch (contentEncoding) {
@@ -564,6 +592,8 @@ void QDecompressHelper::clear()
     totalBytesRead = 0;
     totalUncompressedBytes = 0;
     totalCompressedBytes = 0;
+
+    errorStr.clear();
 }
 
 qsizetype QDecompressHelper::readZLib(char *data, const qsizetype maxSize)
@@ -719,8 +749,9 @@ qsizetype QDecompressHelper::readBrotli(char *data, const qsizetype maxSize)
 
         switch (result) {
         case BROTLI_DECODER_RESULT_ERROR:
-            qWarning("Brotli error: %s",
+            errorStr = QLatin1String("Brotli error: %1")
-                     BrotliDecoderErrorString(BrotliDecoderGetErrorCode(brotliDecoderState)));
+                               .arg(QString::fromUtf8(BrotliDecoderErrorString(
+                                       BrotliDecoderGetErrorCode(brotliDecoderState))));
             return -1;
         case BROTLI_DECODER_RESULT_SUCCESS:
             BrotliDecoderDestroyInstance(brotliDecoderState);
@@ -766,7 +797,8 @@ qsizetype QDecompressHelper::readZstandard(char *data, const qsizetype maxSize)
     while (outBuf.pos < outBuf.size && (inBuf.pos < inBuf.size || decoderHasData)) {
         size_t retValue = ZSTD_decompressStream(zstdStream, &outBuf, &inBuf);
         if (ZSTD_isError(retValue)) {
-            qWarning("ZStandard error: %s", ZSTD_getErrorName(retValue));
+            errorStr = QLatin1String("ZStandard error: %1")
+                               .arg(QString::fromUtf8(ZSTD_getErrorName(retValue)));
             return -1;
         } else {
             decoderHasData = false;

src/network/access/qdecompresshelper_p.h

@@ -96,6 +96,8 @@ public:
     static bool isSupportedEncoding(const QByteArray &encoding);
     static QByteArrayList acceptedEncoding();
 
+    QString errorString() const;
+
 private:
     bool isPotentialArchiveBomb() const;
     bool hasDataInternal() const;
@@ -120,6 +122,8 @@ private:
     bool countDecompressed = false;
     std::unique_ptr<QDecompressHelper> countHelper;
 
+    QString errorStr;
+
     // Used for calculating the ratio
     qint64 archiveBombCheckThreshold = 10 * 1024 * 1024;
     qint64 totalUncompressedBytes = 0;

src/network/access/qnetworkreplyhttpimpl.cpp

@@ -359,9 +359,10 @@ qint64 QNetworkReplyHttpImpl::readData(char* data, qint64 maxlen)
             return 0;
         const qint64 bytesRead = d->decompressHelper.read(data, maxlen);
         if (!d->decompressHelper.isValid()) {
-            // error occurred, error copied from QHttpNetworkConnectionPrivate::errorDetail
+            d->error(QNetworkReplyImpl::NetworkError::UnknownContentError,
-            d->error(QNetworkReplyImpl::NetworkError::ProtocolFailure,
+                     QCoreApplication::translate("QHttp", "Decompression failed: %1")
-                     QCoreApplication::translate("QHttp", "Data corrupted"));
+                             .arg(d->decompressHelper.errorString()));
+            return -1;
         }
         if (d->cacheSaveDevice) {
             // Need to write to the cache now that we have the data
@@ -1084,9 +1085,9 @@ void QNetworkReplyHttpImplPrivate::replyDownloadData(QByteArray d)
         decompressHelper.feed(std::move(d));
 
         if (!decompressHelper.isValid()) {
-            // error occurred, error copied from QHttpNetworkConnectionPrivate::errorDetail
+            error(QNetworkReplyImpl::NetworkError::UnknownContentError,
-            error(QNetworkReplyImpl::NetworkError::ProtocolFailure,
+                  QCoreApplication::translate("QHttp", "Decompression failed: %1")
-                  QCoreApplication::translate("QHttp", "Data corrupted"));
+                          .arg(decompressHelper.errorString()));
             return;
         }
 
@@ -1103,12 +1104,19 @@ void QNetworkReplyHttpImplPrivate::replyDownloadData(QByteArray d)
             while (decompressHelper.hasData()) {
                 quint64 nextSize = quint64(d.size()) + quint64(increments);
                 if (nextSize > quint64(std::numeric_limits<QByteArray::size_type>::max())) {
-                    error(QNetworkReplyImpl::NetworkError::ProtocolFailure,
+                    error(QNetworkReplyImpl::NetworkError::UnknownContentError,
-                          QCoreApplication::translate("QHttp", "Data corrupted"));
+                          QCoreApplication::translate("QHttp",
+                                                      "Data downloaded is too large to store"));
                     return;
                 }
                 d.resize(nextSize);
                 bytesRead += decompressHelper.read(d.data() + bytesRead, increments);
+                if (!decompressHelper.isValid()) {
+                    error(QNetworkReplyImpl::NetworkError::UnknownContentError,
+                          QCoreApplication::translate("QHttp", "Decompression failed: %1")
+                                  .arg(decompressHelper.errorString()));
+                    return;
+                }
             }
             d.resize(bytesRead);
             // we're synchronous so we're not calling this function again; reset the decompressHelper
@@ -1374,9 +1382,10 @@ void QNetworkReplyHttpImplPrivate::replyDownloadMetaData(const QList<QPair<QByte
                 decompressHelper.setCountingBytesEnabled(true);
 
             if (!decompressHelper.setEncoding(it->second)) {
-                // error occurred, error copied from QHttpNetworkConnectionPrivate::errorDetail
+                error(QNetworkReplyImpl::NetworkError::UnknownContentError,
-                error(QNetworkReplyImpl::NetworkError::ProtocolFailure,
+                      QCoreApplication::translate("QHttp", "Failed to initialize decompression: %1")
-                      QCoreApplication::translate("QHttp", "Data corrupted"));
+                              .arg(decompressHelper.errorString()));
+                return;
             }
             decompressHelper.setDecompressedSafetyCheckThreshold(
                     request.decompressedSafetyCheckThreshold());

src/network/doc/src/qt6-changes.qdoc

@@ -196,4 +196,14 @@
     \code
     request.setAttribute(QNetworkRequest::Http2AllowedAttribute, false);
     \endcode
+
+    \section2 QNetworkAccessManager now guards against archive bombs
+
+    Starting with Qt 6.2 QNetworkAccessManager will guard against compressed
+    files that decompress to files which are much larger than their compressed
+    form by erroring out the reply if the decompression ratio exceeds a certain
+    threshold.
+    This check is only applied to files larger than a certain size, which can be
+    customized (or disabled by passing -1) by calling
+    \l{QNetworkRequest::setDecompressedSafetyCheckThreshold()}.
 */

tests/auto/network/access/qdecompresshelper/tst_qdecompresshelper.cpp

@@ -432,10 +432,13 @@ void tst_QDecompressHelper::archiveBomb()
     QVERIFY(bytesRead <= output.size());
     QVERIFY(helper.isValid());
 
-    if (shouldFail)
+    if (shouldFail) {
         QCOMPARE(bytesRead, -1);
-    else
+        QVERIFY(!helper.errorString().isEmpty());
+    } else {
         QVERIFY(bytesRead > 0);
+        QVERIFY(helper.errorString().isEmpty());
+    }
 }
 
 void tst_QDecompressHelper::bigZlib()

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

@@ -532,6 +532,8 @@ private Q_SLOTS:
     void cacheWithContentEncoding();
     void downloadProgressWithContentEncoding_data();
     void downloadProgressWithContentEncoding();
+    void contentEncodingError_data();
+    void contentEncodingError();
 
     // NOTE: This test must be last!
     void parentingRepliesToTheApp();
@@ -7105,7 +7107,7 @@ void tst_QNetworkReply::compressedHttpReplyBrokenGzip()
 
     QCOMPARE(waitForFinish(reply), int(Failure));
 
-    QCOMPARE(reply->error(), QNetworkReply::ProtocolFailure);
+    QCOMPARE(reply->error(), QNetworkReply::UnknownContentError);
 }
 
 // TODO add similar test for FTP
@@ -9808,6 +9810,37 @@ void tst_QNetworkReply::downloadProgressWithContentEncoding()
     QCOMPARE(bytesReceived, expected.size());
 }
 
+void tst_QNetworkReply::contentEncodingError_data()
+{
+    QTest::addColumn<QByteArray>("encoding");
+    QTest::addColumn<QString>("path");
+    QTest::addColumn<QNetworkReply::NetworkError>("expectedError");
+
+    QTest::addRow("archive-bomb") << QByteArray("gzip") << (":/4G.gz")
+                                  << QNetworkReply::UnknownContentError;
+}
+
+void tst_QNetworkReply::contentEncodingError()
+{
+    QFETCH(QString, path);
+    QFile compressedFile(path);
+    QVERIFY(compressedFile.open(QIODevice::ReadOnly));
+    QByteArray body = compressedFile.readAll();
+
+    QFETCH(QByteArray, encoding);
+    QString header("HTTP/1.0 200 OK\r\nContent-Encoding: %1\r\nContent-Length: %2\r\n\r\n");
+    header = header.arg(encoding, QString::number(body.size()));
+
+    MiniHttpServer server(header.toLatin1() + body);
+
+    QNetworkRequest request(
+            QUrl(QLatin1String("http://localhost:%1").arg(QString::number(server.serverPort()))));
+    QNetworkReplyPtr reply(manager.get(request));
+
+    QTRY_VERIFY2_WITH_TIMEOUT(reply->isFinished(), qPrintable(reply->errorString()), 15000);
+    QTEST(reply->error(), "expectedError");
+}
+
 // NOTE: This test must be last testcase in tst_qnetworkreply!
 void tst_QNetworkReply::parentingRepliesToTheApp()
 {