Skip to content
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

SimpleClientHttpResponse throws IOException when response body is empty and status code is >= 400 #33020

Closed
nrayburn-tech opened this issue Jun 13, 2024 · 3 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@nrayburn-tech
Copy link

Affects: Spring Versions >= 5.0.0, including 6.x.x, Java versions 8 and 21 (possibly others as well but these are the only ones I've looked at)

When using SimpleClientHttpRequestFactory, the response has an empty body, and an error status code, then an IOException is thrown when trying to read the response body. I think the expected behavior would be that an empty input stream would be returned instead of an IOException being thrown.

This causes an issue within a ClientHttpRequestInterceptor which has to accommodate this by handling the IOException.

Resolution
Instead of doing a null check on the errorStream, check the response status and use the appropriate stream. Below snippet is from the javadoc for HttpURLConnection#getErrorStream.

Returns: an error stream if any, null if there have been no errors, the connection is not connected or the server sent no useful data.

Relevant code that would need to be updated.

public InputStream getBody() throws IOException {
InputStream errorStream = this.connection.getErrorStream();
this.responseStream = (errorStream != null ? errorStream : this.connection.getInputStream());
return this.responseStream;
}

Alternatives

  • Use something other than SimpleClientHttpRequestFactory. This can be more involved because of the differences between the different HTTP client implementations, it isn't always as simple as changing and using the defaults for the new HTTP client.
  • Custom request factory in our apps that is identical to the SimpleClientHttpRequestFactory, but with the change described above.
  • Handle the IOException within the relevant ClientHttpRequestInterceptors.

Reproduction attached. Run the app and there will be 2 log messages output (1 for RestClient and 1 for RestTemplate). DemoClient uses a 3 second timeout before sending the http request to the local server. If the server takes longer than that to start, this timeout will need to be increased.
demo.zip

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 13, 2024
@nrayburn-tech
Copy link
Author

Probably a duplicate of #27380. The reporter there just did a check for the response status before attempting to use the response body. I'm not sure this always works because it might be an error response status with a response body available, in which case we would still want to consume the response body.

@nrayburn-tech
Copy link
Author

#13355 is also the same issue however it was related to the DefaultResponseErrorHandler. The solution for that was to catch the IOException within the error handler, but if the response status was checked before consuming the streams then that catch wouldn't be necessary (although, I'm not sure I would recommend removing it at this point since it's been there for a decade).

67fda70 is the relevant commit for that.

@jhoeller jhoeller added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Jun 13, 2024
@poutsma poutsma self-assigned this Jun 18, 2024
@poutsma poutsma added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jun 18, 2024
@poutsma poutsma added this to the 6.2.0-M5 milestone Jun 18, 2024
@poutsma
Copy link
Contributor

poutsma commented Jun 18, 2024

Thank you for the detailed report, this should be fixed in 6.2.0-M5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants