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

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 5.0 Alpha1
• Fix Version/s: 5.0 Alpha2
• Component/s: HttpClient (classic)
• Assignee: Unassigned
• Reporter: Dariusz Kordonski
• Created: 28/Jan/16 04:30
• Updated: 04/May/17 09:49
• Resolved: 28/Jan/16 16:17

Description

Observed on trunk branch that has 5.0-alpha2-SNAPSHOT mvn version.

The docs for DefaultRedirectStrategy correctly state:

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.

(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:

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;
        }

A simple failing test case that confirms the problem for a PUT request resulting with 302 (PUT should only be redirected automatically for 303):

    @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));
    }