Skip to content

Commit

Permalink
http: reset stream to unconsumed in unconsume()
Browse files Browse the repository at this point in the history
Reset the underlying socket of an HTTP stream to be marked as
unconsume after the HTTP parser no longer owns it.

Fixes: #14407
PR-URL: #14410
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
  • Loading branch information
addaleax authored and MylesBorins committed Sep 19, 2017
1 parent ca61f3b commit 6707411
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 0 deletions.
1 change: 1 addition & 0 deletions src/node_http_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,7 @@ class Parser : public AsyncWrap {

stream->set_alloc_cb(parser->prev_alloc_cb_);
stream->set_read_cb(parser->prev_read_cb_);
stream->Unconsume();
}

parser->prev_alloc_cb_.clear();
Expand Down
5 changes: 5 additions & 0 deletions src/stream_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,11 @@ class StreamBase : public StreamResource {
consumed_ = true;
}

inline void Unconsume() {
CHECK_EQ(consumed_, true);
consumed_ = false;
}

template <class Outer>
inline Outer* Cast() { return static_cast<Outer*>(Cast()); }

Expand Down
29 changes: 29 additions & 0 deletions test/parallel/test-http-upgrade-reconsume-stream.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
'use strict';
const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');

const tls = require('tls');
const http = require('http');

// Tests that, after the HTTP parser stopped owning a socket that emits an
// 'upgrade' event, another C++ stream can start owning it (e.g. a TLSSocket).

const server = http.createServer(common.mustNotCall());

server.on('upgrade', common.mustCall((request, socket, head) => {
// This should not crash.
new tls.TLSSocket(socket);
server.close();
socket.destroy();
}));

server.listen(0, common.mustCall(() => {
http.get({
port: server.address().port,
headers: {
'Connection': 'Upgrade',
'Upgrade': 'websocket'
}
}).on('error', () => {});
}));

0 comments on commit 6707411

Please sign in to comment.