You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "Oleg Kalnichevski (JIRA)" <ji...@apache.org> on 2016/01/28 09:58:39 UTC

[jira] [Commented] (HTTPCLIENT-1716) DefaultRedirectStrategy seems to disregard HTTP spec for PUT/POST/DELETE request redirects

    [ https://issues.apache.org/jira/browse/HTTPCLIENT-1716?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15121060#comment-15121060 ] 

Oleg Kalnichevski commented on HTTPCLIENT-1716:
-----------------------------------------------

My understand of [RFC7231|https://tools.ietf.org/html/rfc7231#section-6.4] is that those restrictions no longer apply. In some cases non-safe methods such as POST need to be converted to GET, but PUT but being idempotent is considered safe.

Javadocs are clearly out of sync with code, but this can happen in the ALPHA phase.

Oleg 

> DefaultRedirectStrategy seems to disregard HTTP spec for PUT/POST/DELETE request redirects
> ------------------------------------------------------------------------------------------
>
>                 Key: HTTPCLIENT-1716
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-1716
>             Project: HttpComponents HttpClient
>          Issue Type: Bug
>          Components: HttpClient
>    Affects Versions: 5.0 Alpha1
>            Reporter: Dariusz Kordonski
>
> Observed on {{trunk}} branch that has 5.0-alpha2-SNAPSHOT mvn version.
> The docs for {{DefaultRedirectStrategy}} correctly state:
> {quote}
> This strategy honors the restrictions on automatic redirection of entity enclosing methods such as POST and PUT imposed by the HTTP specification. \{@code 302 Moved Temporarily\}, \{@code 301 Moved Permanently\} and \{@code 307 Temporary Redirect\} status codes will result in an automatic redirect of HEAD and GET methods only. POST and PUT methods will not be automatically redirected as requiring user confirmation.
> {quote}
> (NB: in fact to be more precise I think DELETE requests should also be *not* automatically redirected)
> However the actual implementation does not seem to follow this, whereby {{isRedirected}} pretty much lets all requests through:
> {code}
> switch (statusCode) {
>             case HttpStatus.SC_MOVED_PERMANENTLY:
>             case HttpStatus.SC_MOVED_TEMPORARILY:
>             case HttpStatus.SC_SEE_OTHER:
>             case HttpStatus.SC_TEMPORARY_REDIRECT:
>                 return true;
>             default:
>                 return false;
>         }
> {code}
> A simple failing test case that confirms the problem for a PUT request resulting with 302 (PUT should only be redirected automatically for 303):
> {code}
>     @Test
>     public void testIsRedirectedForTemporaryRedirectPut() throws Exception {
>         final DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
>         final HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
>                 HttpStatus.SC_TEMPORARY_REDIRECT, "Temporary Redirect");
>         response.addHeader("Location", "http://localhost/stuff");
>         final HttpContext context = new BasicHttpContext();
>         assertFalse(redirectStrategy.isRedirected(new HttpPut("http://localhost/"), response, context));
>     }
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org