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

connection is not kept alive when receiving 204-no-content without content-length #563

Closed
pikazlou opened this issue Oct 24, 2016 · 3 comments

Comments

@pikazlou
Copy link

Finagle HTTP client doesn't follow HTTP 1.1 spec on keeping connection alive in case of 204 response.

Expected behavior

When receiving request with the following headers:

HTTP/1.1 204 No Content
Server: nginx/1.10.1
Date: Mon, 24 Oct 2016 15:12:11 GMT
Content-Type: application/json
Connection: keep-alive

finagle should not initiate closing of the connection because of the absence of "Content-length", since this is completely valid response, according to HTTP 1.1 spec:
https://tools.ietf.org/html/rfc7230#section-3.3.2

A server MUST NOT send a Content-Length header field in any response with a status code of 1xx (Informational) or 204 (No Content)

Actual behavior

Finagle tries to find either "Transfer-Encoding: chunked" or "Content-length: 123" headers. Then failing to find any of them, marks the connection as not kept-alive and closes it.

Steps to reproduce the behavior

Set-up finagle HTTP client to receive 204 response from a server (HTTP 1.1, no "Connection: close" header)
Finagle HTTP client will close the connection.

@bryce-anderson
Copy link
Contributor

Hi @pikazlou! This seems to be a hot topic. There is already a PR concerning this that you might be interested in: #521

We have just merged some work internally similar to this regarding HEAD requests, but it hasn't synced with github yet. I'll try and poke the PR ticket once the sync has happened. It should provide a better foundation for cleaning up some of the conformance issues in finagle.

I'm going to close this ticket as a duplicate of the PR mentioned above. If you feel that this is different in some manner, feel free to reopen it. Feel free to continue the discussion there if necessary.

@pikazlou
Copy link
Author

@bryce-anderson could you suggest any workaround to avoid ConnectionManager closing connection? I've tried adding "Content-length" header inside filter, but it seems ConnectionManager logic is applied before any filtering. So I'm running out of ideas how to solve the issue without forking/patching finagle for my needs.

@bryce-anderson
Copy link
Contributor

Right now, I don't think there is a solution. That is part of the motivation for #521. Part of getting us on that direction was done in this commit: fa8cef1
I hope that we can build on that.

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

No branches or pull requests

2 participants