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 committed Jul 27, 2017
1 parent 6078dea commit 50447e8
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 @@ -527,6 +527,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 50447e8

Please sign in to comment.