From 18f0a45964e62bf3db7b657847902a356fd31f51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= Date: Thu, 19 Oct 2017 12:22:16 +0200 Subject: Fix redirecting all the other methods for HTTP 307 and 308 c4cf90b1f739c47383672de3d66b1d9d5427f5db made POST requests be redirected properly, but this wasn't enough and should have included every method/verb. Change-Id: I37b12dc9fdffcbf2aadbd2360d4fc2584c024939 Reviewed-by: Edward Welbourne Reviewed-by: Timur Pocheptsov --- src/network/access/qnetworkreplyhttpimpl.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'src/network/access/qnetworkreplyhttpimpl.cpp') diff --git a/src/network/access/qnetworkreplyhttpimpl.cpp b/src/network/access/qnetworkreplyhttpimpl.cpp index fa49c8e05b..c1a9f628a0 100644 --- a/src/network/access/qnetworkreplyhttpimpl.cpp +++ b/src/network/access/qnetworkreplyhttpimpl.cpp @@ -1110,16 +1110,14 @@ QNetworkAccessManager::Operation QNetworkReplyHttpImplPrivate::getRedirectOperat // HTTP status code can be used to decide if we can redirect with a GET // operation or not. See http://www.ietf.org/rfc/rfc2616.txt [Sec 10.3] for // more details - Q_UNUSED(httpStatus); + + // We MUST keep using the verb that was used originally when being redirected with 307 or 308. + if (httpStatus == 307 || httpStatus == 308) + return currentOp; switch (currentOp) { case QNetworkAccessManager::HeadOperation: return QNetworkAccessManager::HeadOperation; - case QNetworkAccessManager::PostOperation: - // We MUST keep using POST when being redirected with 307 or 308. - if (statusCode == 307 || statusCode == 308) - return QNetworkAccessManager::PostOperation; - break; default: break; } -- cgit v1.2.3 From 0bd4b194d13b76456fa74330f40cc76a0a2d2b80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= Date: Mon, 23 Oct 2017 12:50:52 +0200 Subject: Replace unneeded nullptr-checks with ASSERTS The manager and managerPrivate member variables are set in the constructor and never changed after that. Change-Id: I7cc2fd2eb3f50bdc529ed29485e74bf9c016b0c0 Coverity-Id: 185272 Reviewed-by: Timur Pocheptsov Reviewed-by: Edward Welbourne --- src/network/access/qnetworkreplyhttpimpl.cpp | 72 ++++++++++++++-------------- 1 file changed, 36 insertions(+), 36 deletions(-) (limited to 'src/network/access/qnetworkreplyhttpimpl.cpp') diff --git a/src/network/access/qnetworkreplyhttpimpl.cpp b/src/network/access/qnetworkreplyhttpimpl.cpp index c1a9f628a0..73f88025ac 100644 --- a/src/network/access/qnetworkreplyhttpimpl.cpp +++ b/src/network/access/qnetworkreplyhttpimpl.cpp @@ -182,6 +182,7 @@ QNetworkReplyHttpImpl::QNetworkReplyHttpImpl(QNetworkAccessManager* const manage : QNetworkReply(*new QNetworkReplyHttpImplPrivate, manager) { Q_D(QNetworkReplyHttpImpl); + Q_ASSERT(manager); d->manager = manager; d->managerPrivate = manager->d_func(); d->request = request; @@ -394,9 +395,9 @@ bool QNetworkReplyHttpImpl::canReadLine () const void QNetworkReplyHttpImpl::ignoreSslErrors() { Q_D(QNetworkReplyHttpImpl); + Q_ASSERT(d->managerPrivate); - if (d->managerPrivate && d->managerPrivate->stsEnabled - && d->managerPrivate->stsCache.isKnownHost(url())) { + if (d->managerPrivate->stsEnabled && d->managerPrivate->stsCache.isKnownHost(url())) { // We cannot ignore any Security Transport-related errors for this host. return; } @@ -407,9 +408,9 @@ void QNetworkReplyHttpImpl::ignoreSslErrors() void QNetworkReplyHttpImpl::ignoreSslErrorsImplementation(const QList &errors) { Q_D(QNetworkReplyHttpImpl); + Q_ASSERT(d->managerPrivate); - if (d->managerPrivate && d->managerPrivate->stsEnabled - && d->managerPrivate->stsCache.isKnownHost(url())) { + if (d->managerPrivate->stsEnabled && d->managerPrivate->stsCache.isKnownHost(url())) { // We cannot ignore any Security Transport-related errors for this host. return; } @@ -1144,6 +1145,8 @@ QNetworkRequest QNetworkReplyHttpImplPrivate::createRedirectRequest(const QNetwo void QNetworkReplyHttpImplPrivate::onRedirected(const QUrl &redirectUrl, int httpStatus, int maxRedirectsRemaining) { Q_Q(QNetworkReplyHttpImpl); + Q_ASSERT(manager); + Q_ASSERT(managerPrivate); if (isFinished) return; @@ -1178,7 +1181,7 @@ void QNetworkReplyHttpImplPrivate::onRedirected(const QUrl &redirectUrl, int htt redirectRequest = createRedirectRequest(originalRequest, url, maxRedirectsRemaining); operation = getRedirectOperation(operation, httpStatus); - if (const QNetworkCookieJar *const cookieJar = (manager ? manager->cookieJar() : nullptr)) { + if (const QNetworkCookieJar *const cookieJar = manager->cookieJar()) { auto cookies = cookieJar->cookiesForUrl(url); if (!cookies.empty()) { redirectRequest.setHeader(QNetworkRequest::KnownHeaders::CookieHeader, @@ -1195,6 +1198,7 @@ void QNetworkReplyHttpImplPrivate::onRedirected(const QUrl &redirectUrl, int htt void QNetworkReplyHttpImplPrivate::followRedirect() { Q_Q(QNetworkReplyHttpImpl); + Q_ASSERT(managerPrivate); rawHeaders.clear(); cookedHeaders.clear(); @@ -1206,7 +1210,7 @@ void QNetworkReplyHttpImplPrivate::followRedirect() // If the original request didn't need a session (i.e. it was to localhost) // then we might not have a session open, to which to redirect, if the // new URL is remote. When this happens, we need to open the session now: - if (managerPrivate && isSessionNeeded(url)) { + if (isSessionNeeded(url)) { if (auto session = managerPrivate->getNetworkSession()) { if (session->state() != QNetworkSession::State::Connected || !session->isOpen()) { startWaitForSession(session); @@ -2039,9 +2043,7 @@ void QNetworkReplyHttpImplPrivate::_q_bufferOutgoingData() void QNetworkReplyHttpImplPrivate::_q_networkSessionConnected() { Q_Q(QNetworkReplyHttpImpl); - - if (!manager) - return; + Q_ASSERT(managerPrivate); QSharedPointer session = managerPrivate->getNetworkSession(); if (!session) @@ -2171,28 +2173,27 @@ void QNetworkReplyHttpImplPrivate::finished() if (preMigrationDownloaded != Q_INT64_C(-1)) totalSize = totalSize.toLongLong() + preMigrationDownloaded; - if (manager) { #ifndef QT_NO_BEARERMANAGEMENT - QSharedPointer session = managerPrivate->getNetworkSession(); - if (session && session->state() == QNetworkSession::Roaming && - state == Working && errorCode != QNetworkReply::OperationCanceledError) { - // only content with a known size will fail with a temporary network failure error - if (!totalSize.isNull()) { - if (bytesDownloaded != totalSize) { - if (migrateBackend()) { - // either we are migrating or the request is finished/aborted - if (state == Reconnecting || state == WaitingForSession) { - return; // exit early if we are migrating. - } - } else { - error(QNetworkReply::TemporaryNetworkFailureError, - QNetworkReply::tr("Temporary network failure.")); + Q_ASSERT(managerPrivate); + QSharedPointer session = managerPrivate->getNetworkSession(); + if (session && session->state() == QNetworkSession::Roaming && + state == Working && errorCode != QNetworkReply::OperationCanceledError) { + // only content with a known size will fail with a temporary network failure error + if (!totalSize.isNull()) { + if (bytesDownloaded != totalSize) { + if (migrateBackend()) { + // either we are migrating or the request is finished/aborted + if (state == Reconnecting || state == WaitingForSession) { + return; // exit early if we are migrating. } + } else { + error(QNetworkReply::TemporaryNetworkFailureError, + QNetworkReply::tr("Temporary network failure.")); } } } -#endif } +#endif // if we don't know the total size of or we received everything save the cache if (totalSize.isNull() || totalSize == -1 || bytesDownloaded == totalSize) @@ -2250,17 +2251,16 @@ void QNetworkReplyHttpImplPrivate::_q_metaDataChanged() Q_Q(QNetworkReplyHttpImpl); // 1. do we have cookies? // 2. are we allowed to set them? - if (manager) { - const auto it = cookedHeaders.constFind(QNetworkRequest::SetCookieHeader); - if (it != cookedHeaders.cend() - && request.attribute(QNetworkRequest::CookieSaveControlAttribute, - QNetworkRequest::Automatic).toInt() == QNetworkRequest::Automatic) { - QNetworkCookieJar *jar = manager->cookieJar(); - if (jar) { - QList cookies = - qvariant_cast >(it.value()); - jar->setCookiesFromUrl(cookies, url); - } + Q_ASSERT(manager); + const auto it = cookedHeaders.constFind(QNetworkRequest::SetCookieHeader); + if (it != cookedHeaders.cend() + && request.attribute(QNetworkRequest::CookieSaveControlAttribute, + QNetworkRequest::Automatic).toInt() == QNetworkRequest::Automatic) { + QNetworkCookieJar *jar = manager->cookieJar(); + if (jar) { + QList cookies = + qvariant_cast >(it.value()); + jar->setCookiesFromUrl(cookies, url); } } emit q->metaDataChanged(); -- cgit v1.2.3