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/1.0 responses missing Content-Length #56277

Open
jamesdiacono opened this issue Dec 17, 2024 · 5 comments
Open

HTTP/1.0 responses missing Content-Length #56277

jamesdiacono opened this issue Dec 17, 2024 · 5 comments
Labels
http Issues or PRs related to the http subsystem.

Comments

@jamesdiacono
Copy link

Version

v22.10.0

Platform

Darwin ... 24.2.0 Darwin Kernel Version 24.2.0: ...; root:xnu-11215.61.5~2/RELEASE_ARM64_T6000 arm64 arm Darwin

Subsystem

http

What steps will reproduce the bug?

Start a simple HTTP server.

import http from "node:http";
http.createServer(function (req, res) {
    res.end("hello");
}).listen(3000);

Send it an HTTP/1.0 request using cURL.

$ curl --http1.0 -XGET -I 127.0.0.1:3000
HTTP/1.1 200 OK
Date: Tue, 17 Dec 2024 03:26:02 GMT
Connection: close

How often does it reproduce? Is there a required condition?

Consistently reproducible.

What is the expected behavior? Why is that the expected behavior?

I would expect the response to include a Content-Length header.

What do you see instead?

The response does not include a Content-Length header.

Additional information

If we instead send an HTTP/1.1 request (either with or without Keep-Alive), the Content-Length header is included.

$ curl --http1.1 -XGET -I 127.0.0.1:3000
HTTP/1.1 200 OK
Date: Tue, 17 Dec 2024 03:31:41 GMT
Connection: keep-alive
Keep-Alive: timeout=5
Content-Length: 5

$ curl --http1.1 -H"Connection: close" -XGET -I 127.0.0.1:3000
HTTP/1.1 200 OK
Date: Tue, 17 Dec 2024 03:32:35 GMT
Connection: close
Content-Length: 5

This seems strange. I would expect to see a Content-Length header regardless of whether the request was HTTP/1.0 or HTTP/1.1.

Attaching a debugger, the HTTP/1.0 request triggers the former clause and the HTTP/1.1 request triggers the latter clause in the following HTTP server code.

node/lib/_http_outgoing.js

Lines 553 to 559 in 2cd385e

} else if (!this.useChunkedEncodingByDefault) {
this._last = true;
} else if (!state.trailer &&
!this._removedContLen &&
typeof this._contentLength === 'number') {
header += 'Content-Length: ' + this._contentLength + '\r\n';
} else if (!this._removedTE) {

@jamesdiacono
Copy link
Author

I noticed this bug because I was using Nginx as a reverse proxy in front of a Node.js process. Nginx, by default, proxies all requests upsteam as HTTP/1.0. (This behavior can be overridden using Nginx's proxy_http_version option.) As a result, Nginx was serving proxied responses without a Content-Length header.

As a sidenote, omitting the Content-Length header seems to be risky in HTTP/1.0, though this is probably irrelevant because Node.js does not appear to send HTTP/1.0 responses.

In HTTP=1.0, if the sender does not include a Content-Length header, the recipient cannot tell if the message has been truncated due to transmission problems. This ambiguity leads to errors, especially when truncated responses are stored in caches.

From https://www.ra.ethz.ch/cdstore/www8/data/2136/pdf/pd1.pdf

@jakecastelli jakecastelli added the http Issues or PRs related to the http subsystem. label Dec 18, 2024
@bnoordhuis
Copy link
Member

I guess no one is going to object to a pull request but sending Content-Length is at best a marginal improvement because that's only possible when the response size is known upfront.

Node can't send Transfer-Encoding headers to http/1.0 clients, so your nginx proxy would still be serving up responses without Content-Length.

@jamesdiacono
Copy link
Author

The problem is that Content-Length is omitted even when the response size is known up front, a common case in my experience.

It is precisely the lack of chunked encoding that makes a Content-Length header desirable. Nginx quite happily propagates the Content-Length header if it is present. Am I missing something?

@bnoordhuis
Copy link
Member

Is the lack of Content-Length actually causing issues for you or is it just something you noticed? Because you're going to have to deal with responses without it anyway so what exactly do you gain when some include it?

@jamesdiacono
Copy link
Author

It is my clients, not me, that has to deal with my responses, so I like to include a Content-Length because it is sometimes useful.

I worked around the problem by configuring Nginx to send HTTP/1.1 requests to Node.js, so this is no longer causing any issues for me. But because it took me several hours to diagnose, I felt I had a duty to report it as inconsistent behavior. If fixing it is too hard, others might at least find this issue when they search.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http Issues or PRs related to the http subsystem.
Projects
None yet
Development

No branches or pull requests

3 participants