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

Potential Regression/Change: Connection doesn't reset on HEAD #34758

Open
josephhackman opened this issue Aug 13, 2020 · 0 comments
Open

Potential Regression/Change: Connection doesn't reset on HEAD #34758

josephhackman opened this issue Aug 13, 2020 · 0 comments
Labels
http Issues or PRs related to the http subsystem.

Comments

@josephhackman
Copy link
Contributor

  • Version: v15.0.0-pre
  • Platform: Linux 4.19.121-microsoft-standard SMP Fri Jun 19 21:06:10 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
  • Subsystem: http

What steps will reproduce the bug?

HEAD request to server that responds with Content-Length or Content-Encoding, for example wikipedia.org, or any S3 file.

var http = require("http");

var agent = new http.Agent();
agent.maxSockets = 1;

var sockets = [];

function request(hostname, path, method, callback) {
    var options = {
        hostname: hostname,
        port: 80,
        path: path,
        agent: agent,
        method: method,
        headers: { "Connection": "keep-alive" }
    };
    var req = http.request(options, function (res) {
        console.log("Got response: " + res.statusCode);

        res.setEncoding('utf8');
        var body = "";
        res.on('data', function (chunk) {
            body += chunk;
            console.log("Data Recieved");
        });
        res.on('end', function () {
            console.log(JSON.stringify(res.headers));
            if (!res.complete) {
                console.error(
                    'The connection was terminated while the message was still being sent');
            }
            console.log(sockets.length)
        });
    });


    req.on('error', function (e) {
        console.log(e);
        return callback(e);
    });
    req.on("socket", function (socket) {
        if (sockets.indexOf(socket) === -1) {
            console.log("new socket created");
            sockets.push(socket);
            socket.on("close", function () {
                console.log("socket has been closed");
            });
        }
    });
    req.end()
}

function run(method) {

    request('wikipedia.org', '/', method, function (e, res, body) {
        console.log(e);
    });
}
console.log("HEAD")
run("HEAD");
run("HEAD");
run("HEAD");

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

Always, if the response headers contain payload headers (content-length, content-encoding).

What is the expected behavior?

According to #12396, this is a bug, and we would expect all HEAD requests to close the socket.

IMHO, the current behavior is equally compliant.

What do you see instead?

Keep-alive is respected, socket remains open for multiple requests.

Additional information

I don't think this is actually a bug, I just wanted to draw attention to the fact that either the behavior changed without people noticing since 2017, or the original reporter was testing exclusively against servers that don't report payloads for HEAD (which includes node).

This is also the thrust of my PR to add those headers to node's server: #34231
And related issue: #28438

@lpinca lpinca added the http Issues or PRs related to the http subsystem. label Aug 15, 2020
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

2 participants