---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116570/#review53047
---
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())
- Andrea Iacovitti
On March 7, 2014, 6:07 a.m., Dawit Alemayehu wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116570/
---
(Updated March 7, 2014, 6:07 a.m.)
Review request for kdelibs, Andrea Iacovitti and David Faure.
Repository: kdelibs
Description
---
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
