I was getting a strange response from a custom API endpoint, and it turned out that a firewall on the network was killing my API's 204 responses. (They were issued with a call to services_error().)
After further research, we found that including a message body with 204 and 304 HTTP status codes is not allowed according to IETF's RFC2616:
"Any response message which "MUST NOT" include a message-body (such
as the 1xx, 204, and 304 responses and any response to a HEAD
request) is always terminated by the first empty line after the
header fields, regardless of the entity-header fields present in
the message."
So, the firewall saw a response from my API like "204 No Content : Blah blah blah" and correctly threw it out.
I see that services_error() causes the REST server to throw an exception that, in turn, creates the HTTP header for the response. The problem is that RESTServer::formatHttpHeaderStatusMessage() always includes " : " in the header's message body, even if no message is passed through to the method. So, the closest I could get was "204 No Content : " which was still (correctly) zapped by the firewall.
Rolled a patch that addresses this. It also allows for backwards compatibility, in that you must explicitly pass an empty message to services_error() in order to create the header without a body.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | services-204_304_response_body-2698025-8.patch | 1.23 KB | majorrobot |
| #4 | services-204_304_response_body-2698025-4.patch | 1.61 KB | majorrobot |
| #2 | services-204_304_response_body-2698025-1.patch | 836 bytes | majorrobot |
Comments
Comment #2
majorrobot commentedComment #3
marcingy commentedOne comment would be make the list a variable so as it can be tweaked in future opposed to a hardcoded list. Also maybe add a boolean to indicate if the new format should be used or not...it is possible some apps care about there being a body.
Comment #4
majorrobot commentedGood call. Thought of that, too, but then started thinking about settings etc., etc., etc. and decided to keep it simple.
Then again, there's no harm in just getting vars. If someone like me wants to set them via drush, I'm still able to do so. I could see these being useful as settings per server, but am not sure it's necessary at this point.
Also noted the new configurable variable in README.txt so devs would know about it.
Let me know what you think.
Comment #5
marcingy commentedComment #6
kylebrowning commentedComment #7
majorrobot commentedI hate to do this, but after further testing with out firewall, it looks like it's not the header message that's violating the standard. Rather, no matter what, the JSONFormatter for the REST server returns a body. With a 204 or 304 error, it likely only returns an empty string:
[""]But this technically has a Content-length of 4, violating RFC2616.
Will give this some more thought as to how to fix. A change to RESTServer::render() may be the best way forward. Happy to hear anyone else's thoughts!
Comment #8
majorrobot commentedAll right, new strategy, pretty much the same idea.
Since 204s and 304s should have no body to format, the patch adds a conditional to RESTServer::handle() that keeps $result from running through a formatter if vars from the previous patch are set.
Another way to do this would be to add the conditional further down line, in each formatter, but this makes less sense to me, since if there's no body, there's nothing to format.
Comment #9
kylebrowning commentedComment #11
kylebrowning commented