Skip to content

SPR-16604 - Prevent response body from being read at DefaultResponseErrorHandler.hasError() on unknown HTTP status code #1742

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

denysivano
Copy link

@denysivano denysivano commented Mar 16, 2018

When response contains HTTP status code not defined in HttpStatus enum, DefaultResponseErrorHandler.hasError(ClientHttpResponse response) calls getHttpStatusCode(ClientHttpResponse response) which reads response body and throws UnknownHttpStatusCodeException. In this case false value will be returned (which means response doesn't have error) and ResponseExtractor.extract() in RestTemplate is called with response which body has been already read.

Issue: SPR-16604

When response contains unknown HTTP status code, do not read and waste
response body.
Issue: SPR-16604
@pivotal-issuemaster
Copy link

@denysivano Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-issuemaster
Copy link

@denysivano Thank you for signing the Contributor License Agreement!

@denysivano denysivano changed the title SPR-16604 - Do not allow DefaultResponseErrorHandler.hasError to read response body SPR-16604 - Prevent response body from being read at DefaultResponseErrorHandler.hasError() on unknown HTTP status code Mar 16, 2018
@jhoeller
Copy link
Contributor

I've resolved this through direct enum value comparisons in hasError (without an intermediate exception and therefore as analogous as possible to the 5.0 code), reusing your tests. Thanks for the PR, in any case!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants