nodejs / nodejs/node

No keepAliveTimeout for HTTP server after answering a POST request synchronously

Open
#39,137 9 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

help wanted http
Dominant language
JavaScript
Stars
122k
Forks
37.3k
Avg merge
4d 2h
Merged PRs (30d)
283

Description

  • Version: v16.4.0
  • Platform: Linux test01 4.19.0-13-amd64 # 1 SMP Debian 4.19.160-2 (2020-11-28) x86_64 GNU/Linux
  • Subsystem: http
What steps will reproduce the bug?

When using the "normal" NodeJS "hello world" HTTP server example, which answers any request synchronously, and sending it a POST request with a body, server.keepAliveTimeout (default 5 sec) does not work. The HTTP connection remains open ("forever"?).

Complete Testcase

Here we test all 4 combinations of sync/async response and get/post request to a simple HTTP server, and expect a connection close from server after 1 sec - which works fine for all combinations except "synchronous response for a POST request". If the connection is still open, we try another request - to prove, the connection still works:

'use strict';
const http = require('http');
const net = require('net');

const server = http.createServer((req, res) => {
  if (req.url === '/async') {
    req.on('data', () => {});
    req.on('end', () => {
      res.end('test-body');
      console.log('Server: Sent response asynchronously');
    });
  } else {
    res.end('test-body');
    console.log('Server: Sent response synchronously');
  }
});

server.keepAliveTimeout = 1000;

server.listen(0, async () => {
  await sendRequests('async', 'get');
  await sendRequests('async', 'post');
  await sendRequests('sync', 'get');
  await sendRequests('sync', 'post'); // keepAliveTimeout does not work here!!

  server.close();
});

async function sendRequests(type, method) {
  console.log(`=== Testing ${type} response for method "${method}" and wait for keep-alive-close`);

  return new Promise((resolve) => {
    const client = new net.Socket();
    let nextReqTimeout = null;

    client.connect(server.address().port, '127.0.0.1', () => {
      console.log('Client: Connected to server');

      httpRequest();

      nextReqTimeout = setTimeout(() => {
        console.log('Client: ERROR: HTTP-Connection is still open - trying another request');
        httpRequest();

        nextReqTimeout = setTimeout(() => {
          console.log('Client: ERROR: HTTP-Connection is still open - closing from client side now');
          nextReqTimeout = null;
          client.end();
        }, server.keepAliveTimeout * 2);
      }, server.keepAliveTimeout * 2);
    });

    client.on('data', function(data) {
      console.log('Client: Got response data');
    });

    client.on('close', function() {
      if (nextReqTimeout) {
        console.log('Client: Server closed connection as expected');
        clearTimeout(nextReqTimeout);
      }
      resolve();
    });

    function httpRequest() {
      const rawRequests = {
        'post': 'POST /' + type + ' HTTP/1.1\r\n' +
          'Connection: keep-alive\r\n' +
          'Content-Type: application/x-www-form-urlencoded\r\n' +
          'Content-Length: 10\r\n' +
          '\r\n' +
          'Test=67890',
        'get': 'GET /' + type + ' HTTP/1.1\r\n' +
          'Connection: keep-alive\r\n' +
          '\r\n'
      };

      console.log('Client: Sending request');
      client.write(rawRequests[method]);
    }
  });
}
What is the expected behavior?

The keepAliveTimeout should also work after answering a POST request synchronously.

Possible cause

Maybe this is a race condition here:

  • In resOnFinish the server.keepAliveTimeout is set on the socket, after sending the response
  • After that, socketOnData is called internally for the remaining POST body, which calls onParserExecuteCommon, which resets the socket timeout first
  • Normally socketOnData is called before sending the response, so in most cases it works as expected

Maybe there should be a swicth, to not reset the socket-timeout when socketOnData is called after the response was sent?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Run the supplied JavaScript reproduction first, then read lib/_http_server.js around resOnFinish, socketOnData, and onParserExecuteCommon. Trace how the remaining POST body resets the socket timeout after the synchronous response. Done means the synchronous POST case closes after server.keepAliveTimeout like the other three cases, with regression coverage for the reported behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
api, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.