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.

Fixes: QTBUG-101942
Change-Id: Ie9af42510ca027d15248e5bcf21e836e709898d9
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
(cherry picked from commit d642c16fe745e156a93129fbb3750784b361ee39)
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
This commit is contained in:
Mårten Nordheim 2022-03-31 18:48:42 +02:00
parent 606ca8a590
commit 427d2c0966
7 changed files with 70 additions and 13 deletions

View File

@ -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;
}
@ -386,8 +391,13 @@ qsizetype QDecompressHelper::read(char *data, qsizetype maxSize)
uncompressedBytes -= bytesRead;
totalUncompressedBytes += bytesRead;
if (isPotentialArchiveBomb())
if (isPotentialArchiveBomb()) {
errorStr = QCoreApplication::translate(
"QHttp",
"The decompressed output exceeds the limits specified by "
"QNetworkRequest::decompressedSafetyCheckThreshold()");
return -1;
}
return bytesRead;
}
@ -458,11 +468,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) {
@ -504,6 +532,8 @@ void QDecompressHelper::clear()
uncompressedBytes = 0;
totalUncompressedBytes = 0;
totalCompressedBytes = 0;
errorStr.clear();
}
qsizetype QDecompressHelper::readZLib(char *data, const qsizetype maxSize)
@ -659,8 +689,9 @@ qsizetype QDecompressHelper::readBrotli(char *data, const qsizetype maxSize)
switch (result) {
case BROTLI_DECODER_RESULT_ERROR:
qWarning("Brotli error: %s",
BrotliDecoderErrorString(BrotliDecoderGetErrorCode(brotliDecoderState)));
errorStr = QLatin1String("Brotli error: %1")
.arg(QString::fromUtf8(BrotliDecoderErrorString(
BrotliDecoderGetErrorCode(brotliDecoderState))));
return -1;
case BROTLI_DECODER_RESULT_SUCCESS:
BrotliDecoderDestroyInstance(brotliDecoderState);
@ -706,7 +737,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;

View File

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

View File

@ -1297,9 +1297,11 @@ void QHttp2ProtocolHandler::updateStream(Stream &stream, const Frame &frame,
output.resize(read);
replyPrivate->responseData.append(std::move(output));
} else if (read < 0) {
finishStreamWithError(
stream, QNetworkReply::ProtocolFailure,
QCoreApplication::translate("QHttp", "Data corrupted"));
const QString decompressError =
QCoreApplication::translate("QHttp", "Decompression failed: %1")
.arg(replyPrivate->decompressHelper.errorString());
finishStreamWithError(stream, QNetworkReply::UnknownContentError,
decompressError);
return;
}
}

View File

@ -203,7 +203,13 @@ void QHttpProtocolHandler::_q_receiveReply()
}
} else if (haveRead == -1) {
// Some error occurred
m_connection->d_func()->emitReplyError(m_socket, m_reply, QNetworkReply::ProtocolFailure);
if (replyPrivate->autoDecompress && !replyPrivate->decompressHelper.isValid()) {
m_connection->d_func()->emitReplyError(m_socket, m_reply,
QNetworkReply::UnknownContentError);
} else {
m_connection->d_func()->emitReplyError(m_socket, m_reply,
QNetworkReply::ProtocolFailure);
}
break;
}
}

View File

@ -193,4 +193,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()}.
*/

View File

@ -424,10 +424,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()

View File

@ -7077,7 +7077,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