Bug 37097

Created attachment 52540 [details] Patch

Comment on attachment 52540 [details] Patch > + QUrl newUrl = m_reply->url().resolved(redirection); > + if (newUrl == m_reply->url()) { // Infinite redirection loop! // avoid redirecting to the same url as it causes infinite recursion > + ResourceError error(newUrl.host(), 400 /*bad request*/, > + newUrl.toString(), > + QCoreApplication::translate("QWebPage", "Ininite Loop in Redirect Request")); Spelling issue. "Infinite recursion in redirection request" > + client->didFail(m_resourceHandle, error); > + return; > + } > + > m_redirected = true; > > - QUrl newUrl = m_reply->url().resolved(redirection); > ResourceRequest newRequest = m_resourceHandle->request(); > newRequest.setURL(newUrl); > > -- > 1.6.3.3 >

Created attachment 52541 [details] Updated Patch Updated per Kenneth's comments

Manually Committed r57085.

This fix will break sites that purposefully redirect to the very same url. IMHO to detect and avoid infinite recursion to the same url, you have to have additional checks such as a counter as it is done in KDE's KIO: // Some websites keep redirecting to themselves where each redirection // acts as the stage in a state-machine. We define "endless redirections" // as 5 redirections to the same URL. if (d->m_redirectionList.count(url) > 5) { kDebug(7007) << "CYCLIC REDIRECTION!"; setError( ERR_CYCLIC_LINK ); setErrorText( d->m_url.pathOrUrl() ); } See lines #1014-1022 of kio/kio/job.cpp: http://websvn.kde.org/trunk/KDE/kdelibs/kio/kio/job.cpp?revision=1108333&view=markup

Created attachment 52546 [details] User Counters

Comment on attachment 52546 [details] User Counters I did a bit of research and Mozilla doesn't verify where the redirect comes from, and your patch won't handles cases as page1 -> page2 -> page1! Let's introduce a max redirection count (default 10 as Mozilla) and count it down for each redirect (not just the case of newUrl == m_reply->url()). When it reaches 0, we fail with an error such as "Redirection limit reached!" or similar. Later we can add a setting for this. Info: http://kb.mozillazine.org/Network.http.redirection-limit (look at the bug report)

Created attachment 52550 [details] Updated Counters Patch

Attachment 52550 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/platform/network/qt/QNetworkReplyHandler.cpp:342: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 52554 [details] Updated Patch

Attachment 52554 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/platform/network/qt/QNetworkReplyHandler.cpp:343: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 52557 [details] Updated Patch

Attachment 52557 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/platform/network/qt/QNetworkReplyHandler.cpp:343: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 52557 [details] Updated Patch Clearing flags on attachment: 52557 Committed r57117: <http://trac.webkit.org/changeset/57117>

Revision r57085 cherry-picked into qtwebkit-2.0 with commit ca03a15b28a427cd9a0e9d8844780f01a7a2ad13 Revision r57117 cherry-picked into qtwebkit-2.0 with commit e830618c892e2e48ad038fc8ecc826c04c513905