204 No Content on HTTP/1.1 generates an empty chunk

[zecke]

My code is doing:

response.writeHead(Tufao::HttpResponseStatus::NO_CONTENT);
response.end();

This generates a 204 No Content, with Transfer-Encoding: chunked and includes and empty chunk '0\r\n\r\n'. But this is not valid according to the RFC2616:

10.2.5 204 No Content
...
The 204 response MUST NOT include a message-body, and thus is always terminated by the first empty line after the header fields.

The Microsoft .NET HTTP Client is upset by the extra chunk being sent. The only work-around is to disable keep-alive by adding Connection: close to the request.

[vinipsmaker]

I've read the mentioned RFC2616 paragraph.

Bug confirmed.

I'll review the code over the week and merge it on all Tufão branches (with changes if needed).

Thanks for the feedback.

[zecke]

Thanks. My patch is against an old version (1.0.2), doesn't come with a unit test and as indicated in the commit message with some open questions I couldn't answer myself. E.g. how strictly do you want to enforce this rule? Should ::write() just return false and these things.

I am looking forward for a proper fix.

[vinipsmaker]

My patch is against an old version (1.0.2)

For this feature/patch, ain't a problem.

doesn't come with a unit test and

HttpServerResponse acts upon a QIODevice and unit tests could be done using QBuffer.

The tests cannot be done using headers, because the order is not guaranteed. Parsing the generated buffer if headers are present require a lot of implementation work. I'll leave this for later.

Having said that, I can implement the tests, but I'll first focus on the documentation changes.

as indicated in the commit message with some open questions I couldn't answer myself. E.g. how strictly do you want to enforce this rule? Should ::write() just return false and these things.

HTTP rules aren't very orthogonal and we end up with hacks like this.

The only path I see to fix this bug is breaking orthogonality also on Tufão and make the body aware of the response status code.

I don't want to enforce this rule to the user. Some behaviours of HTTP are unknown to Tufão and left to the user handle. For instance, RFC2616 document some HTTP verbs, but more were created by other protocols and Tufão doesn't know how to handle them, but it may be interesting for the user, then the task is left for the user to resolve.

I'm heading to a compromise now, but my schedule is less busy now and I'll be able to review and merge the patch when I came back.

Thanks for the responsive collaboration with the contribution.

[vinipsmaker]

Fixed.

@zecke, can you test the new version and confirm the fix? Then I'll release another stable version.

[zecke]

I didn't verify it with REST Sharp (my downstream was using REST Sharp) but using curl and looking at the output in wireshark the last thing in the response is the \r\n and Content-Length is 0.

[vinipsmaker]

using curl and looking at the output in wireshark the last thing in the response is the \r\n and Content-Length is 0.

The final "\r\n" indicates the end of all headers and must be present.

Quoting section 4.4 of RFC 2616:

1.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, a single "\r\n" is expected. I'll create a tag and release the new version with the bugfix tomorrow.