<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/116570/">https://git.reviewboard.kde.org/r/116570/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">First of all it's worth to mention about th proposed update for rfc2616 (http://tools.ietf.org/html/draft-ietf-httpbis-p2-semantics-26) that makes asking user confirmation optional (see http://tools.ietf.org/html/draft-ietf-httpbis-p2-semantics-26#page-53 and subsequents). It states: "...the user agent MAY automatically redirect its request to the URI referenced by the Location field value..".
I tested some other clients and it seems that only Firefox prompts the user about redirection when method of subsequent request is considered unsafe (namely DELETE, POST, PUT).
About the patch i can see two issues.
1) I think "real method string" sent in the request should be checked instead, as for example http_post + CustomHTTPMethod can have been used.
2) When the user do not approve the redirection no data is sent back to client (data == server's redirection response payload).
May be prompting the user *before* the redirection handling code take place (and only if the server have sent back a valid a Location header) is a way to solve these issues.
What i mean is something like that (proof of concept patch):
diff --git a/kioslave/http/http.cpp b/kioslave/http/http.cpp
index e4f1eba..33fbda6 100644
--- a/kioslave/http/http.cpp
+++ b/kioslave/http/http.cpp
@@ -2925,6 +2925,7 @@ try_again:
char buffer[maxHeaderSize];
bool cont = false;
bool bCanResume = false;
+ bool askRedirectionConfirm = false;
if (!isConnected()) {
kDebug(7113) << "No connection.";
@@ -3105,6 +3106,8 @@ try_again:
case 302: // Found
if (m_request.sentMethodString == "POST") {
setMetaData(QLatin1String("redirect-to-get"), QLatin1String("true"));
+ } else if (m_request.sentMethodString == "DELETE" || m_request.sentMethodString == "PUT") {
+ askRedirectionConfirm = true;
}
break;
case 303: // See Other
@@ -3112,8 +3115,16 @@ try_again:
setMetaData(QLatin1String("redirect-to-get"), QLatin1String("true"));
}
break;
+ case 307:
+ if (m_request.sentMethodString == "POST" || m_request.sentMethodString == "DELETE" || m_request.sentMethodString == "PUT") {
+ askRedirectionConfirm = true;
+ }
+ break;
case 308: // Permanent Redirect
setMetaData(QLatin1String("permanent-redirect"), QLatin1String("true"));
+ if (m_request.sentMethodString == "POST" || m_request.sentMethodString == "DELETE" || m_request.sentMethodString == "PUT") {
+ askRedirectionConfirm = true;
+ }
break;
default:
break;
@@ -3484,8 +3495,18 @@ endParsing:
if (tIt.hasNext() && m_request.responseCode > 299 && m_request.responseCode < 400) {
locationStr = QString::fromUtf8(tIt.next().trimmed());
}
- // We need to do a redirect
- if (!locationStr.isEmpty())
+
+ // Should we redirect?
+ bool shouldRedirect = !locationStr.isEmpty();
+
+ if (shouldRedirect && askRedirectionConfirm) {
+ const int userWish = messageBox(WarningYesNo, i18nc("@info redir", "redir"), i18nc("@title:window", "Confirm Redir"));
+ if (userWish == KMessageBox::No) {
+ shouldRedirect = false;
+ }
+ }
+
+ if (shouldRedirect)
{
KUrl u(m_request.url, locationStr);
if(!u.isValid())
</pre>
<br />
<p>- Andrea Iacovitti</p>
<br />
<p>On March 7th, 2014, 6:07 a.m. UTC, Dawit Alemayehu wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kdelibs, Andrea Iacovitti and David Faure.</div>
<div>By Dawit Alemayehu.</div>
<p style="color: grey;"><i>Updated March 7, 2014, 6:07 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdelibs
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This patch is a companion to the recent POST->POST redirection implementation in KIO, https://git.reviewboard.kde.org/r/116017/. It prompts the user to approve the redirection as explicitly required in sections 10.3.[2|3] of RFC 2616:
If the 301 status code is received in response to a request other
than GET or HEAD, the user agent MUST NOT automatically redirect the
request unless it can be confirmed by the user, since this might
change the conditions under which the request was issued.
Please note that this patch only prompts the user for confirmation on POST->POST redirections. It can be expanded to include redirections for other requests such as PUT.
There is also an issue of whether this patch should be part of the 4.13 release? Since we are in a freeze and the patch has both message changes as well as a new API, I have simply marked it for inclusion in master branch, i.e. 4.14.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">http://greenbytes.de/tech/tc/httpredirects/t307methods.html</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kio/kio/job.cpp <span style="color: grey">(50b4afb)</span></li>
<li>kio/kio/jobuidelegate.h <span style="color: grey">(17fd554)</span></li>
<li>kio/kio/jobuidelegate.cpp <span style="color: grey">(5aff330)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/116570/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<ul>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/03/07/e77dd03e-cb37-49bb-8554-cca991c8c546__post_redirection_confirmation.png">POST redirection confirm dialog</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>