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

Node.js abort the response if the body length do not match "Content-Length" during 304 #31037

Closed
RomainLanz opened this issue Dec 20, 2019 · 3 comments
Labels
http Issues or PRs related to the http subsystem.

Comments

@RomainLanz
Copy link
Contributor

RomainLanz commented Dec 20, 2019

Hey 👋

  • Version: 13.X.X
  • Platform: Windows & macOS

Since the version 13 of Node.js, the response is aborted if the Content-Length isn't matched in the body when sending back a 304.

According to the HTTP spec, we have the right to provide a Content-Length header.

A server MAY send a Content-Length header field in a 304 (Not Modified) response to a conditional GET request

Reproduction:

"use strict";

const http = require("http");

const server = http.createServer((req, res) => {
  console.log("got request");
  res.setHeader("Content-Length", 11);
  res.statusCode = 304;
  res.end(null);
});

server.listen(3000, () => {
  console.log("listening");
  const request = http.request({
    hostname: "localhost",
    port: 3000,
    method: "GET",
    path: "/"
  });
  request.on("response", response => {
    console.log("response");
    response.on("aborted", () => {
      console.log("aborted");
    });
    response.on("close", () => {
      console.log("close");
    });
    response.on("end", () => {
      console.log("end");
    });
    response.on("data", console.log);
  });
  request.end("");
});

Logs in Node 12:

listening
got request
response
aborted
end
close

Logs in Node 13:

listening
got request
response
aborted
close

c/c @targos

@targos
Copy link
Member

targos commented Dec 20, 2019

I just added a listener to the "aborted" event, to show that in both Node 12 and 13, the request is aborted prematurely

@targos targos changed the title Node 13 abort the response if the body length do not match "Content-Length" during 304 Node.js abort the response if the body length do not match "Content-Length" during 304 Dec 21, 2019
@targos targos added the http Issues or PRs related to the http subsystem. label Dec 21, 2019
@targos
Copy link
Member

targos commented Dec 21, 2019

/cc @nodejs/http

@BlackYoup
Copy link
Contributor

Hello, I found a way to fix this issue. I'll be able to open a PR in the next few days, I still need to come up with a working test :)

@Trott Trott closed this as completed in 23d6c42 Sep 11, 2020
ruyadorno pushed a commit that referenced this issue Sep 17, 2020
Fixes: #31037

PR-URL: #34835
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
joesepi pushed a commit to joesepi/node that referenced this issue Jan 8, 2021
Fixes: nodejs#31037

PR-URL: nodejs#34835
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
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