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

Plumber sets a Content-Length header on HTTP responses which forbid it (e.g. 1xx, 204, 304) #758

Closed
atheriel opened this issue Feb 5, 2021 · 2 comments · Fixed by #760
Closed

Comments

@atheriel
Copy link
Contributor

atheriel commented Feb 5, 2021

Example application or steps to reproduce the problem

Given the following Plumber API which returns HTTP 204 No Content:

library(plumber)

pr() %>%
  pr_get("/", function(req, res) {
    res$status <- 204
    res
  }) %>%
  pr_run(port = 4444)

Querying this with curl yields:

$ curl -vs localhost:4444/
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 4444 (#0)
> GET / HTTP/1.1
> Host: localhost:4444
> User-Agent: curl/7.58.0
> Accept: */*
> 
< HTTP/1.1 204 No Content
< Date: Fri, 05 Feb 2021 05:07:53 GMT
< Content-Length: 0
< 
* Connection #0 to host localhost left intact

Describe the problem in detail

You can see in the response above that Plumber sets the Content-Length header. RFC 7230 (part of the HTTP 1.1 specification) says the following about the Content-Length header:

A server MAY send a Content-Length header field in a 304 (Not
Modified) response to a conditional GET request (Section 4.1 of
[RFC7232]); a server MUST NOT send Content-Length in such a response
unless its field-value equals the decimal number of octets that would
have been sent in the payload body of a 200 (OK) response to the same
request.

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

Ergo we're currently in violation of the HTTP spec.

The reason this happens is actually because the underlying httpuv package previously set the Content-Length header in all cases. I fixed this recently and the fix is now on CRAN (as v1.5.5).

Plumber should check if the status code is 1xx, 204, or 304 and set the response body to NULL in those cases, as well as require httpuv >= 1.5.5 so that this actually works.

@meztez
Copy link
Collaborator

meztez commented Feb 5, 2021

That is interesting...

check out line 20 in plumber-response

body <- self$body

I believe this was put in place because of httpuv original behavior.
#138
I will try a few things.

@atheriel
Copy link
Contributor Author

atheriel commented Feb 5, 2021

I believe this was put in place because of httpuv original behavior.

Yep, that's my understanding as well. httpuv would previously crash on a missing body.

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

Successfully merging a pull request may close this issue.

2 participants