hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dariusz Kordonski (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (HTTPCLIENT-1716) DefaultRedirectStrategy seems to disregard HTTP spec for PUT/POST/DELETE request redirects
Date Thu, 28 Jan 2016 04:31:39 GMT

     [ https://issues.apache.org/jira/browse/HTTPCLIENT-1716?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Dariusz Kordonski updated HTTPCLIENT-1716:
------------------------------------------
    Description: 
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}

  was:
Observed on {{trunk}} branch that has 5.0-alpha2-SNAPSHOT mvn version.

The docs for {{DefaultRedirectStrategy}} correctly state:

{quote}
his 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}


> 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


Mime
View raw message