From d72c6f2c65e3111a841cbc6cfdd1c1e005f56458 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Fri, 9 Sep 2022 12:23:08 +0200 Subject: Fix using QNetworkReply with custom url schemes Sequential QIODevices are allowed to report atEnd() when they have no data to read, but add more data later. To avoid a regression with our own tests, treat a read error from a sequential device at end, as end-of-data. Fixes: QTBUG-106461 Change-Id: Iac1233e6daa978c827c37a7fd3131e2fce764111 Reviewed-by: Michal Klocek (cherry picked from commit aefd389bfdf7efcaf787d2cfb4683d7018cb377c) Reviewed-by: Qt Cherry-pick Bot --- src/core/net/custom_url_loader_factory.cpp | 9 +++- .../qwebengineprofile/tst_qwebengineprofile.cpp | 59 +++++++++++++++++++--- 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/src/core/net/custom_url_loader_factory.cpp b/src/core/net/custom_url_loader_factory.cpp index d607da656..892f21dac 100644 --- a/src/core/net/custom_url_loader_factory.cpp +++ b/src/core/net/custom_url_loader_factory.cpp @@ -416,13 +416,20 @@ private: m_totalBytesRead += bytesRead; m_client->OnTransferSizeUpdated(m_totalBytesRead); - if (m_device->atEnd() || (m_maxBytesToRead > 0 && m_totalBytesRead >= m_maxBytesToRead)) { + const bool deviceAtEnd = m_device->atEnd(); + if ((deviceAtEnd && !m_device->isSequential()) + || (m_maxBytesToRead > 0 && m_totalBytesRead >= m_maxBytesToRead)) { OnTransferComplete(MOJO_RESULT_OK); return true; // Done with reading } if (readResult == 0) return false; // Wait for readyRead + if (readResult < 0 && deviceAtEnd && m_device->isSequential()) { + // Failure on read, and sequential device claiming to be at end, so treat it as a successful end-of-data. + OnTransferComplete(MOJO_RESULT_OK); + return true; // Done with reading + } if (readResult < 0) break; } diff --git a/tests/auto/widgets/qwebengineprofile/tst_qwebengineprofile.cpp b/tests/auto/widgets/qwebengineprofile/tst_qwebengineprofile.cpp index c09f3b51a..ee4f5990e 100644 --- a/tests/auto/widgets/qwebengineprofile/tst_qwebengineprofile.cpp +++ b/tests/auto/widgets/qwebengineprofile/tst_qwebengineprofile.cpp @@ -66,6 +66,7 @@ private Q_SLOTS: void urlSchemeHandlerFailRequest(); void urlSchemeHandlerFailOnRead(); void urlSchemeHandlerStreaming(); + void urlSchemeHandlerStreaming2(); void urlSchemeHandlerRequestHeaders(); void urlSchemeHandlerInstallation(); void urlSchemeHandlerXhrStatus(); @@ -289,10 +290,12 @@ public: QList> m_buffers; }; -class StreamingIODevice : public QIODevice { +// an evil version constantly claiming to be at end, similar to QNetworkReply +class StreamingIODeviceBasic : public QIODevice +{ Q_OBJECT public: - StreamingIODevice(QObject *parent) : QIODevice(parent), m_bytesRead(0), m_bytesAvailable(0) + StreamingIODeviceBasic(QObject *parent) : QIODevice(parent), m_bytesRead(0), m_bytesAvailable(0) { setOpenMode(QIODevice::ReadOnly); m_timer.start(100, this); @@ -303,12 +306,11 @@ public: const std::lock_guard lock(m_mutex); return m_bytesAvailable; } - bool atEnd() const override +protected: + bool internalAtEnd() const { - const std::lock_guard lock(m_mutex); return (m_data.size() >= 1000 && m_bytesRead >= 1000); } -protected: void timerEvent(QTimerEvent *) override { const std::lock_guard lock(m_mutex); @@ -329,7 +331,7 @@ protected: memcpy(data, m_data.constData() + m_bytesRead, len); m_bytesAvailable -= len; m_bytesRead += len; - } else if (atEnd()) + } else if (internalAtEnd()) return -1; return len; @@ -339,14 +341,26 @@ protected: return 0; } -private: mutable QRecursiveMutex m_mutex; +private: QByteArray m_data; QBasicTimer m_timer; int m_bytesRead; int m_bytesAvailable; }; +// A nicer version implementing atEnd +class StreamingIODevice : public StreamingIODeviceBasic +{ +public: + StreamingIODevice(QObject *parent) : StreamingIODeviceBasic(parent) {} + bool atEnd() const override + { + const std::lock_guard lock(m_mutex); + return internalAtEnd(); + } +}; + class StreamingUrlSchemeHandler : public QWebEngineUrlSchemeHandler { public: @@ -361,6 +375,20 @@ public: } }; +class StreamingUrlSchemeHandler2 : public QWebEngineUrlSchemeHandler +{ +public: + StreamingUrlSchemeHandler2(QObject *parent = nullptr) + : QWebEngineUrlSchemeHandler(parent) + { + } + + void requestStarted(QWebEngineUrlRequestJob *job) override + { + job->reply("text/plain;charset=utf-8", new StreamingIODeviceBasic(job)); + } +}; + void tst_QWebEngineProfile::urlSchemeHandlers() { RedirectingUrlSchemeHandler lettertoHandler; @@ -502,6 +530,23 @@ void tst_QWebEngineProfile::urlSchemeHandlerStreaming() QCOMPARE(toPlainTextSync(view.page()), QString::fromLatin1(result)); } +void tst_QWebEngineProfile::urlSchemeHandlerStreaming2() +{ + StreamingUrlSchemeHandler2 handler; + QWebEngineProfile profile; + profile.installUrlSchemeHandler("stream", &handler); + QWebEngineView view; + QSignalSpy loadFinishedSpy(&view, SIGNAL(loadFinished(bool))); + view.setPage(new QWebEnginePage(&profile, &view)); + view.settings()->setAttribute(QWebEngineSettings::ErrorPageEnabled, false); + view.load(QUrl(QStringLiteral("stream://whatever"))); + view.show(); + QTRY_COMPARE_WITH_TIMEOUT(loadFinishedSpy.count(), 1, 30000); + QByteArray result; + result.append(1000, 'c'); + QCOMPARE(toPlainTextSync(view.page()), QString::fromLatin1(result)); +} + class ExtraHeaderInterceptor : public QWebEngineUrlRequestInterceptor { public: -- cgit v1.2.3