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.10 6.9 6.8
Change-Id: Ie7a0e106c5475b8298697cdee0ba080acab3ce97
Reviewed-by: Lena Biliaieva <lena.biliaieva@qt.io>
Reviewed-by: Matthias Rauter <matthias.rauter@qt.io>
This commit is contained in:
Øystein Heskestad 2025-06-03 14:45:51 +02:00
parent c5c80cb7b9
commit ea669306b2
7 changed files with 76 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;
}
};
@ -325,6 +363,7 @@ void QHttpServerHttp1ProtocolHandler::handleReadyRead()
return; // Partial read
qCDebug(lcHttpServerHttp1Handler) << "Request:" << request;
useHttp1_1 = request.d->minorVersion == 1;
QHttpServerResponder responder(this);
@ -442,20 +481,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

@ -89,6 +89,7 @@ private:
bool handlingRequest = false;
bool protocolChanged = false;
QElapsedTimer lastActiveTimer;
bool useHttp1_1 = false;
};
QT_END_NAMESPACE

View File

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

View File

@ -80,6 +80,8 @@ public:
qsizetype currentChunkRead;
qsizetype currentChunkSize;
bool upgrade;
int majorVersion;
int minorVersion;
QByteArray fragment;
QByteDataBuffer bodyBuffer;

View File

@ -306,6 +306,8 @@ QHttpServerRequest QHttpServerRequest::create(const QHttpServerParser &parser)
request.d->method = parser.method;
request.d->headers = parser.headers;
request.d->body = parser.body;
request.d->majorVersion = parser.majorVersion;
request.d->minorVersion = parser.minorVersion;
return request;
}

View File

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

View File

@ -1653,12 +1653,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");
}
@ -1679,12 +1673,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));
}
@ -1704,12 +1692,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(), "");
}