elastic / elastic-transport-js

Transport classes and utilities shared among Node.js Elastic client libraries
Apache License 2.0
4 stars 24 forks source link

[HttpConnection] Handles malformed HTTP (HEAD) responses #69

Closed afharo closed 11 months ago

afharo commented 11 months ago

We learned that attempting to perform a HEAD request to an HTTP server that doesn't comply with the HTTP spec may lead to crashing the app.

This is because the HTTP client cleans the error listener of the request object once we receive the response object, but Node.js emits the malformed error on the request object.

This PR adds a no-op handler to silence those spec-parse errors.

Testing

To prove the fix works, comment line 132 in src/connection/HttpConnection.ts and see how the test crashes.

afharo commented 11 months ago

I know Kibana isn't using UndiciConnection still, so I'll have to investigate whether we need to include a similar fix there.

@JoshMock, I tested the issue in this code snippet:

const elasticsearch = require('@elastic/elasticsearch');
const net = require('net');

const HOST = 'http://localhost:8080';

function createTcpServer() {
  const server = net.createServer();
  server.on('connection', (socket) => {
    socket.write(`HTTP/1.1 200 OK\r\n`)
    socket.write(`Content-Type: text/html\r\n`)
    socket.write(`Content-Length: 155\r\n`)
    socket.write(`\r\n`)
    socket.write(`<!DOCTYPE html>
<html>
<head>
 <meta charset="UTF-8">
</head>
<body>
<h1>Hi there</h1>
<p>This is a bad implementation of an HTTP server</p></body>
</html>`)

    socket.end()
  })
  return new Promise((resolve) => server.listen(8080, resolve));
}

async function main() {
  await createTcpServer();

  try {
    console.info('Running Elasticsearch.ping...');
    const esClient = new elasticsearch.Client({
      nodes: [HOST],
      username: '',
      password: '',
      ssl: { verificationMode: 'none' },
      Connection: elasticsearch.HttpConnection,
      ConnectionPool: elasticsearch.ClusterConnectionPool,
    });
    await esClient.ping();
    console.info('Ping succeeded');
  } catch (err) {
    console.error('Elasticsearch.ping', err);
  }
}

main();

// Keep the program running forever to make it super-obvious when it crashes.
setInterval(() => { }, 1000)

Changing the Connection to elasticsearch.UndiciConnection (or even not setting it at all and leaving the default) handles the issue correctly.