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

RestTemplate - support response mapping to entity with potentially empty response body. [SPR-8016] #12671

Closed
spring-issuemaster opened this issue Mar 2, 2011 · 20 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Mar 2, 2011

David Victor opened SPR-8016* and commented

I am working with an API which returns an empty response body on http status 200 (OK) & a body which is populated on http status 400 (Bad Request).

When using RestTemplate.getForEntity() this is fine for the case where the body is populated, however I get an exception for the 'null body' case.

My understanding is one should provide a class implementing the ResponseBody interface & make the call via the execute() method on the RestTemplate.

This task is a request to consider that the use case here is common & whether Spring should offer something out of the box for this situation.

Also See: http://stackoverflow.com/questions/3322381/spring-resttemplate-behavior-when-handling-responses-with-a-status-of-no-content/3648447#3648447


Affects: 3.1 M1

Issue Links:

  • #12566 Better handling of 204 No Content in RestTemplate ("duplicates")
  • #17560 Resttemplate with HttpComponentsClientHttpRequestFactory will make HttpMessageConverterExtractor.extractData return null if the header contains content-encoding gzip, the connection is closed and the response isn't chunked ("is duplicated by")
  • #17309 RestTemplate cannot handle GZIP response since 4.1.3 ("is duplicated by")

9 votes, 19 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 19, 2011

sriram commented

Same issue for post calls that return 202 accepted with an empty body.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 22, 2011

emanuele commented

Same issue for post calls returning 204 - NO CONTENT. The only way to use rest template with this kind of response is to set "null" as ResponseEntity class, but doing so you can't know what response really you had with your call.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 11, 2011

Shai Yallin commented

Same issue for any request resulting in 304 Not Modified

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 17, 2011

Magnus Heino commented

Already fixed.

#12566

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 25, 2012

David Victor commented

Several options documented here too. http://stackoverflow.com/a/5170959/366073

:-)

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 1, 2014

Yann Petrick commented

I reopened the ticket because parts of the issue are still defect. Responses with status 200 (Ok), a single header "Connection close" and no body still don't get parsed.

According to RFC 2616 Section 4.4. Message Length, Part 2 a message can be terminated by closing the connection:

If a Transfer-Encoding header field (section 14.41) is present and
has any value other than "identity", then the transfer-length is
defined by use of the "chunked" transfer-coding (section 3.6),
unless the message is terminated by closing the connection.
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 2, 2014

Stéphane Nicoll commented

What about the documented approach on SO (previous comment)? Can you clarify what you're after please?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 2, 2014

Yann Petrick commented

The workaround on SO would do for me, but it would requiere a massive refactoring.
My problem is that I wanted to use nginx as reverse proxy to a legacy backend running on jetty.

In the current setup nginx modifies the requests to include a header "Connection close" .
If jetty recives this header in the request, it also sets a header "Connection close" to indicate the length of an empty body. The responose by jetty also had no headers for transfer-encoding, content-type nor content-length, just the header "Connection close".

If I use the rest-template to request an entity from the backend behind nginx (e.g. the next queued job) and none was found I get an exception.

If I use the rest-tempalte to directly communicate with the jetty the header "Connection close" is not send and jetty sets a header for content-length and it all works.

There are many solutions to my problem,
I could send a 204, I could wrap every response in a response object and never would get an empty response body, or I could change my nginx setup.

But this all are workarounds and in my oppinion the rest-template should be able to handle these responses directy, as they are valid.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 24, 2014

Colin Morelli commented

After this change, RestTemplate can no longer communicate with a spring server using Jackson serialization. The server doesn't include the Content-Length in the response, and the client treats that as an empty response body.

The commit introduced to fix this issue (9887025) actually appears to have caused the issue. The description indicates that support was to better fit RFC 7230 where a connection is closed and there is neither a content length header nor a transfer-encoding header. However, that is true only for requests. Responses follow a different set of rules with regards to the presence of a message body. Below is quoted from Section 3.3 of RFC 7230:

For response messages, whether or not a message-body is included with a message is dependent on both the request method and the response status code (section 6.1.1). All responses to the HEAD request method MUST NOT include a message-body, even though the presence of entity- header fields might lead one to believe they do. All 1xx (informational), 204 (no content), and 304 (not modified) responses MUST NOT include a message-body. All other responses do include a message-body, although it MAY be of zero length.

Because of this, the logic in the "hasMessageBody" method is incorrect in assuming that the lack of a content-length header and a closed connection implies that there is no body. HTTP/1.1 specification allows for the server to signal the send of the response body by closing the connection.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 29, 2014

Brian Clozel commented

Hi Colin Morelli

RestTemplate + Jackson serialization broken

Could you elaborate on this?
In those cases, the server sends a Transfer-Encoding: chunked HTTP header, and this fix checks for those.
I've tried to reproduce this with a sample project but couldn't - could you take a look at it?

RFC 7230

The RFC does state that this applies for HTTP responses:

A response that has neither chunked transfer coding nor Content-Length is terminated by closure of the connection and, thus, is considered complete regardless of the number of message body octets received, provided that the header section was received intact.

Also, Section 3.3.3 explains a bit better how to asses the message length in each case.
So it all boils down to "should the hasMessageBody return true if the response has a message body of length 0"?

This is another question that could be raised in a different JIRA issue; but in my opinion it's not related to the RestTemplate+Jackson issue, which I think is more important to investigate first.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 29, 2014

Colin Morelli commented

Brian,

Looks like you're right here. After digging deeper into the problem, I found out that it was actually the HAProxy load balancer decoding the transfer encoding and removing the header. It doesn't add a content-length in this process, although that appears to be intentional as indicated in this section from the HAProxy documentation:

In this mode, called the "HTTP close" mode, there are as many connection establishments as there are HTTP transactions. Since the connection is closed by the server after the response, the client does not need to know the content length.

But this still seems to be a problem, given that the response that HAProxy responds with is a valid HTTP response according to the RFC. My interpretation of:

A response that has neither chunked transfer coding nor Content-Length is terminated by closure of the connection and, thus, is considered complete regardless of the number of message body octets received, provided that the header section was received intact.

Is that a response can contain neither transfer-encoding nor content-length, and still contain a 0-n length body where n is the number of bytes received between the completion of the headers and the termination of the connection. In other words, on a closed connection, any content past the HTTP headers should be interpreted as a body. It would appear that browsers treat it this way, given that accessing a page that fails to load in RestTemplate works fine in Chrome, Safari and Firefox.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 29, 2014

Brian Clozel commented

Hi Colin,

You're right, there's something wrong about this fix.
In fact, it looks like you can't really tell if there's a (non null) body without actually reading the input stream.

I'll dig into this, but it really looks like I've got to revisit this and HttpMessageConverters implementations to make sure they don't barf when the body is of size 0 and correctly return a null.

Unfortunately, the next version is scheduled tomorrow (CET), so this new fix won't make it.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 29, 2014

Colin Morelli commented

No problem Brian, thanks for looking into this. In the meantime I can drop down my spring versions and/or change the haproxy configuration to do chunked encoding. A path of least resistance might be to simply wrap the input stream with something that is capable of peeking in on the stream (reading a single byte) to see if there's any content (and caching the received byte to be returned on any calls to read the stream). Not necessarily the best solution, but potentially saves you from having to mess with the HttpMessageConverters.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 31, 2014

Brian Clozel commented

OK, I've got a fix ready in a PR

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 9, 2015

Koos Gadellaa commented

The fix is giving us issues, and I think it's wrong...
Current code in the HttpMessageConverterExtractor, last lines in the hasMessageBody method:

boolean chunked = headers.containsKey(HttpHeaders.TRANSFER_ENCODING)
&& headers.get(HttpHeaders.TRANSFER_ENCODING).contains("chunked");
boolean closed = headers.containsKey(HttpHeaders.CONNECTION)
&& headers.getConnection().contains("close");
if(!chunked && contentLength == -1 && closed) {
return false;
}

The spec reads:

A response that has neither chunked
transfer coding nor Content-Length is terminated by closure of the
connection and, thus, is considered complete regardless of the number
of message body octets received, provided that the header section was
received intact.

i.e. it is valid if:

  • a content-length is available
  • the content is chunked
  • a close header is available.

so it should read

if(!chunked && contentLength == -1 && !closed)
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 9, 2015

Brian Clozel commented

Hi Koos Gadellaa
The code you're referring to does not exist anymore and has been replaced by the latest fix, this will be released with 4.1.5.
Do you still have concerns about that new fix?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 9, 2015

Koos Gadellaa commented

Ah! I was looking at the 4.1.x branch, and apparently the fix hasn't been merged to it yet...

It'll probably work for me, but I currently can't see any checks if the body is valid as given in https://tools.ietf.org/html/rfc7230#section-3.4 (which was originaly referred).
However, adhering to that part is a bit of a nuisance, as it effectively blocks users from getting a body, and I think that should be left to the user of the library (it's better to have the body as is, even if it turns out to be invalid, then no body at all)... so I'm all for it!
(in the meantime, I'll work around it myself, as I can't wait for tomorrow ;) )

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 9, 2015

Brian Clozel commented

I'll backport this right now.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 16, 2015

Satyapal Reddy commented

we are upgrading from Spring 3.2.2 to Spring 4.1.3/4.1.4 and what used to work earlier started failing due to the logic in hasMessageBody. I looked at 4.1.5 and that may work. But it may be a while before 4.1.5 is released. In our case, server is responding with a body, but with no Content-Length header and with Connection: close.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 16, 2015

Brian Clozel commented

Hi Satyapal Reddy
Sorry for this inconvenience.
4.1.5 is scheduled in a few days, you can check its status at any given time here.
You can already test this by using 4.1.5-BUILD-SNAPSHOT builds; you just need to add https://repo.spring.io/libs-snapshot/ as a snapshot-enabled repository in your maven configuration.

Thanks,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.