Fix writing from sequential QIODevices to HTTP(S)/1.(0/1) clients

When handling HTTP(S)/1.1 clients use chunked transfer encoding
for sequential QIODevices, because sequential QIODevices don't
know their length. The failure to do so caused clients to hang.
When handling HTTP(S)/1.0 clients disconnect after write because
such clients don't support chunked transfer encoding.

Removed expect_fail from tests that now work.

Fixes: QTBUG-137330
Pick-to: 6.8
Change-Id: Ie7a0e106c5475b8298697cdee0ba080acab3ce97
Reviewed-by: Lena Biliaieva <lena.biliaieva@qt.io>
Reviewed-by: Matthias Rauter <matthias.rauter@qt.io>
(cherry picked from commit ea669306b2)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 479b327ba2)
This commit is contained in:
Øystein Heskestad 2025-06-03 14:45:51 +02:00
parent 2d118b9392
commit 79f9d18e32
5 changed files with 72 additions and 42 deletions

View File

@ -111,16 +111,23 @@ struct IOChunkedTransfer
const QPointer<QIODevice> sink;
const QMetaObject::Connection bytesWrittenConnection;
const QMetaObject::Connection readyReadConnection;
const QMetaObject::Connection readChannelFinished;
bool inRead = false;
bool gotReadChannelFinished = false;
bool writingIsComplete = false;
bool useHttp1_1 = false;
IOChunkedTransfer(QIODevice *input, QIODevice *output) :
IOChunkedTransfer(QIODevice *input, QIODevice *output, bool http1_1) :
source(input),
sink(output),
bytesWrittenConnection(connectToBytesWritten(this, output)),
readyReadConnection(QObject::connect(source.data(), &QIODevice::readyRead, source.data(),
[this]() { readFromInput(); }))
[this]() { readFromInput(); })),
readChannelFinished(QObject::connect(source.data(), &QIODevice::readChannelFinished,
source.data(),
[this]() { readChannelFinishedHandler(); })),
useHttp1_1(http1_1)
{
Q_ASSERT(!source->atEnd()); // TODO error out
QObject::connect(sink.data(), &QObject::destroyed, source.data(), &QObject::deleteLater);
QObject::connect(source.data(), &QObject::destroyed, source.data(), [this]() {
delete this;
@ -132,6 +139,7 @@ struct IOChunkedTransfer
{
QObject::disconnect(bytesWrittenConnection);
QObject::disconnect(readyReadConnection);
QObject::disconnect(readChannelFinished);
}
static QMetaObject::Connection connectToBytesWritten(IOChunkedTransfer *that, QIODevice *device)
@ -152,6 +160,12 @@ struct IOChunkedTransfer
return beginIndex == endIndex;
}
void readChannelFinishedHandler()
{
gotReadChannelFinished = true;
readFromInput();
}
void readFromInput()
{
if (inRead)
@ -167,13 +181,19 @@ struct IOChunkedTransfer
beginIndex = 0;
endIndex = source->read(buffer, bufferSize);
if (endIndex < 0) {
endIndex = beginIndex; // Mark the buffer as empty
qCWarning(lcHttpServerHttp1Handler, "Error reading chunk: %ls",
qUtf16Printable(source->errorString()));
break;
endIndex = 0; // Mark the buffer as empty
if (!source->isSequential()) {
qCWarning(lcHttpServerHttp1Handler, "Error reading chunk: %ls",
qUtf16Printable(source->errorString()));
return;
}
}
if (endIndex == 0) { // Nothing was read
if (!writingIsComplete
&& ((!source->isSequential() && source->atEnd()) || gotReadChannelFinished))
completeWriting();
return;
}
if (endIndex == 0)
break;
memset(buffer + endIndex, 0, sizeof(buffer) - std::size_t(endIndex));
writeToOutput();
}
@ -183,7 +203,6 @@ struct IOChunkedTransfer
{
if (sink.isNull() || source.isNull())
return;
if (isBufferEmpty())
return;
@ -200,19 +219,38 @@ struct IOChunkedTransfer
}
#endif
const auto writtenBytes = sink->write(buffer + beginIndex, endIndex);
if (useHttp1_1 && source->isSequential()) {
sink->write(QByteArray::number(endIndex - beginIndex, 16));
sink->write("\r\n");
}
const auto writtenBytes = sink->write(buffer + beginIndex, endIndex - beginIndex);
if (writtenBytes < 0) {
qCWarning(lcHttpServerHttp1Handler, "Error writing chunk: %ls",
qUtf16Printable(sink->errorString()));
return;
}
if (useHttp1_1 && source->isSequential())
sink->write("\r\n");
beginIndex += writtenBytes;
if (isBufferEmpty()) {
if (source->bytesAvailable() && !inRead)
readFromInput();
else if (source->atEnd()) // Finishing
source->deleteLater();
if (isBufferEmpty() && !inRead)
readFromInput();
}
void completeWriting()
{
if (sink.isNull())
return;
Q_ASSERT(!source.isNull());
Q_ASSERT(isBufferEmpty());
if (source->isSequential()) {
if (useHttp1_1)
sink->write("0\r\n\r\n");
else
sink->close();
}
source->deleteLater();
writingIsComplete = true;
}
};
@ -315,6 +353,7 @@ void QHttpServerHttp1ProtocolHandler::handleReadyRead()
return; // Partial read
qCDebug(lcHttpServerHttp1Handler) << "Request:" << request;
useHttp1_1 = request.d->minorVersion == 1;
QHttpServerResponder responder(this);
@ -428,20 +467,19 @@ void QHttpServerHttp1ProtocolHandler::write(QIODevice *data, const QHttpHeaders
}
QHttpHeaders allHeaders(headers);
if (!input->isSequential()) { // Non-sequential QIODevice should know its data size
if (input->isSequential()) {
if (useHttp1_1)
allHeaders.append(QHttpHeaders::WellKnownHeader::TransferEncoding, "chunked");
else
allHeaders.append(QHttpHeaders::WellKnownHeader::Connection, "close");
} else { // Non-sequential QIODevice should know its data size
allHeaders.append(QHttpHeaders::WellKnownHeader::ContentLength,
QByteArray::number(input->size()));
}
writeStatusAndHeaders(status, allHeaders);
if (input->atEnd()) {
qCDebug(lcHttpServerHttp1Handler, "No more data available.");
return;
}
// input takes ownership of the IOChunkedTransfer pointer inside his constructor
new IOChunkedTransfer<>(input.release(), socket);
new IOChunkedTransfer<>(input.release(), socket, useHttp1_1);
state = TransferState::Ready;
}

View File

@ -85,6 +85,7 @@ private:
// a request is still being handled.
bool handlingRequest = false;
bool protocolChanged = false;
bool useHttp1_1 = false;
};
QT_END_NAMESPACE

View File

@ -106,6 +106,8 @@ bool QHttpServerRequestPrivate::parseRequestLine(QByteArrayView line)
parser.setMajorVersion(protocol[5] - '0');
parser.setMinorVersion(protocol[7] - '0');
majorVersion = protocol[5] - '0';
minorVersion = protocol[7] - '0';
method = parseRequestMethod(requestMethod);
url = QUrl::fromEncoded(requestUrl.toByteArray());
@ -357,6 +359,9 @@ bool QHttpServerRequestPrivate::parse(QHttp2Stream *socket)
{
parser.clear();
majorVersion = 2;
minorVersion = 0;
for (const auto &pair : socket->receivedHeaders()) {
if (pair.name == ":method") {
method = parseRequestMethod(pair.value);
@ -403,6 +408,8 @@ void QHttpServerRequestPrivate::clear()
currentChunkRead = 0;
currentChunkSize = 0;
upgrade = false;
majorVersion = 0;
minorVersion = 0;
fragment.clear();
bodyBuffer.clear();

View File

@ -72,6 +72,8 @@ public:
quint16 remotePort;
QHostAddress localAddress;
quint16 localPort;
int majorVersion;
int minorVersion;
#if QT_CONFIG(ssl)
QSslConfiguration sslConfiguration;
#endif

View File

@ -1612,12 +1612,6 @@ void tst_QHttpServer::writeSequentialDevice()
QSignalSpy spy(reply.get(), &QNetworkReply::finished);
spy.wait(2s);
if (!useHttp2) {
QEXPECT_FAIL(
"",
"QTBUG-137330: Writing from a Sequential QIODevice to HTTP/1.1 Hangs the Client",
Abort);
}
QCOMPARE(spy.count(), 1);
checkReply(reply.release(), "GoGoGo");
}
@ -1638,12 +1632,6 @@ void tst_QHttpServer::writeMuchToSequentialDevice()
QSignalSpy spy(reply.get(), &QNetworkReply::finished);
spy.wait(2s);
if (!useHttp2) {
QEXPECT_FAIL(
"",
"QTBUG-137330: Writing from a Sequential QIODevice to HTTP/1.1 Hangs the Client",
Abort);
}
QCOMPARE(spy.count(), 1);
checkReply(reply.release(), u"a"_s.repeated(bufferSize * 2));
}
@ -1663,12 +1651,6 @@ void tst_QHttpServer::writeFromEmptySequentialDevice()
QSignalSpy spy(reply.get(), &QNetworkReply::finished);
spy.wait(2s);
if (!useHttp2) {
QEXPECT_FAIL(
"",
"QTBUG-137330: Writing from a Sequential QIODevice to HTTP/1.1 Hangs the Client",
Abort);
}
QCOMPARE(spy.count(), 1);
checkReply(reply.release(), "");
}