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

http server respond with inappropriate default headers for HEAD method #28438

Open
abbshr opened this issue Jun 26, 2019 · 3 comments · Fixed by #34231
Open

http server respond with inappropriate default headers for HEAD method #28438

abbshr opened this issue Jun 26, 2019 · 3 comments · Fixed by #34231
Labels
help wanted Issues that need assistance from volunteers or PRs that need help to proceed. http Issues or PRs related to the http subsystem.

Comments

@abbshr
Copy link
Contributor

abbshr commented Jun 26, 2019

  • Version: >= 10.x
  • Platform: any
  • Subsystem: http

To implement a HTTP keep alive mechanism, I use the http.Agent({ keepAlive: true }) and http.request({ agent, method, host, port }).

Everything is ok when the method is GET or TRACE and others, the agent reuse the TCP connection as I expect.

But when changing the method to HEAD, the agent close the connection when server responded.

From the response header, the only different is that: for HEAD method, there isn't a Content-Length: 0 or Transfer-Encoding: chunked in the default header (but GET has by default), so the HTTP parser think this connection should not be kept alive.

But from the RFC, we can infer that if keep-alive was enabled, either Content-Length: 0 or Transfer-Encoding: chunked should be setted in response header, I think this should be a default behavior.

# server
const http = require("http")

const server = http.createServer((req, res) => {
  // res.setHeader("Content-Length", 0)
  // res.setHeader("Transfer-Encoding", "chunked")
  res.end()
})

server.keepAliveTimeout = 0
server.listen(2333, "x.x.x.x")
# client
const http = require("http")

const agent = new http.Agent({
  keepAlive: true,
})

request = () => {
  const req = http.request({
    agent,
    host: "x.x.x.x",
    port: 2333,
    method: 'HEAD',
  })

  req.end()
}

request()
@abbshr abbshr changed the title http server respond with inappropriate headers for HEAD method http server respond with inappropriate default headers for HEAD method Jun 26, 2019
@jimmiehansson
Copy link

Hey @abbshr
In contrary to GET this is the intended behavior for the HEAD requests as its simply a meta request of GET, excluding the body contents that you would typically have in GET, this seems like an expected behavior from the http server as there is no buffer or chunk to expect.
RFC-accordingly, depending on the request payload this may also lead to early termination.
Did i understand your problem correctly?
Also see the semantics of the HTTP payload https://tools.ietf.org/html/rfc7231#section-3.3

@lpinca lpinca added the http Issues or PRs related to the http subsystem. label Jul 28, 2019
@jasnell jasnell added the help wanted Issues that need assistance from volunteers or PRs that need help to proceed. label Jun 26, 2020
@aduh95 aduh95 closed this as completed in 7afa533 Jan 3, 2021
danielleadams pushed a commit that referenced this issue Jan 12, 2021
Fixes: #28438

PR-URL: #34231
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this issue May 1, 2021
Fixes: #28438

PR-URL: #34231
Reviewed-By: James M Snell <jasnell@gmail.com>
@wa-Nadoo
Copy link

It seems this issue should be reopened because the bugfix commit was reverted.

@targos targos reopened this Jun 15, 2021
@nawed2611
Copy link

Hey! Can I work on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issues that need assistance from volunteers or PRs that need help to proceed. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants