Skip to content

Commit

Permalink
Merge pull request #1 from simllll/master
Browse files Browse the repository at this point in the history
latest http2 including mem leak fix and stream.upstream not defined check
  • Loading branch information
florianreinhart authored Nov 8, 2016
2 parents 9e36236 + 5421660 commit aec8aaa
Show file tree
Hide file tree
Showing 13 changed files with 125 additions and 94 deletions.
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,6 @@ node_modules
.idea
coverage
doc
.vscode
.vscode/.browse*
npm-debug.log
typings
6 changes: 6 additions & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
Version history
===============

### 3.3.6 (2016-09-16) ###
* We were not appropriately sending HPACK context updates when receiving SETTINGS_HEADER_TABLE_SIZE. This release fixes that bug.

### 3.3.5 (2016-09-06) ###
* Fix issues with large DATA frames (https://github.com/molnarg/node-http2/issues/207)

### 3.3.4 (2016-04-22) ###
* More PR bugfixes (https://github.com/molnarg/node-http2/issues?q=milestone%3Av3.3.4)

Expand Down
11 changes: 11 additions & 0 deletions example/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,17 @@ function onRequest(request, response) {
fileStream.on('finish',response.end);
}

// Example for testing large (boundary-sized) frames.
else if (request.url === "/largeframe") {
response.writeHead(200);
var body = 'a';
for (var i = 0; i < 14; i++) {
body += body;
}
body = body + 'a';
response.end(body);
}

// Otherwise responding with 404.
else {
response.writeHead(404);
Expand Down
49 changes: 21 additions & 28 deletions lib/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,15 @@
// ------------------------------------
//
// - **Class: http2.Endpoint**: an API for using the raw HTTP/2 framing layer. For documentation
// see the [lib/endpoint.js](endpoint.html) file.
// see [protocol/endpoint.js](protocol/endpoint.html).
//
// - **Class: http2.Server**
// - **Event: 'connection' (socket, [endpoint])**: there's a second argument if the negotiation of
// HTTP/2 was successful: the reference to the [Endpoint](endpoint.html) object tied to the
// HTTP/2 was successful: the reference to the [Endpoint](protocol/endpoint.html) object tied to the
// socket.
//
// - **http2.createServer(options, [requestListener])**: additional option:
// - **log**: an optional [bunyan](https://github.com/trentm/node-bunyan) logger object
// - **plain**: if `true`, the server will accept HTTP/2 connections over plain TCP instead of
// TLS
//
// - **Class: http2.ServerResponse**
// - **response.push(options)**: initiates a server push. `options` describes the 'imaginary'
Expand All @@ -33,20 +31,17 @@
// - **new Agent(options)**: additional option:
// - **log**: an optional [bunyan](https://github.com/trentm/node-bunyan) logger object
// - **agent.sockets**: only contains TCP sockets that corresponds to HTTP/1 requests.
// - **agent.endpoints**: contains [Endpoint](endpoint.html) objects for HTTP/2 connections.
// - **agent.endpoints**: contains [Endpoint](protocol/endpoint.html) objects for HTTP/2 connections.
//
// - **http2.request(options, [callback])**: additional option:
// - **plain**: if `true`, the client will not try to build a TLS tunnel, instead it will use
// the raw TCP stream for HTTP/2
// - **http2.request(options, [callback])**:
// - similar to http.request
//
// - **http2.get(options, [callback])**: additional option:
// - **plain**: if `true`, the client will not try to build a TLS tunnel, instead it will use
// the raw TCP stream for HTTP/2
// - this performs request and then also calls request.end() for request instance
// - **http2.get(options, [callback])**:
// - similar to http.get
//
// - **Class: http2.ClientRequest**
// - **Event: 'socket' (socket)**: in case of an HTTP/2 incoming message, `socket` is a reference
// to the associated [HTTP/2 Stream](stream.html) object (and not to the TCP socket).
// to the associated [HTTP/2 Stream](protocol/stream.html) object (and not to the TCP socket).
// - **Event: 'push' (promise)**: signals the intention of a server push associated to this
// request. `promise` is an IncomingPromise. If there's no listener for this event, the server
// push is cancelled.
Expand All @@ -57,7 +52,7 @@
// - has two subclasses for easier interface description: **IncomingRequest** and
// **IncomingResponse**
// - **message.socket**: in case of an HTTP/2 incoming message, it's a reference to the associated
// [HTTP/2 Stream](stream.html) object (and not to the TCP socket).
// [HTTP/2 Stream](protocol/stream.html) object (and not to the TCP socket).
//
// - **Class: http2.IncomingRequest (IncomingMessage)**
// - **message.url**: in case of an HTTP/2 incoming request, the `url` field always contains the
Expand Down Expand Up @@ -92,11 +87,11 @@
// but will function normally when falling back to using HTTP/1.1.
//
// - **Class: http2.Server**
// - **Event: 'checkContinue'**: not in the spec, yet (see [http-spec#18][expect-continue])
// - **Event: 'checkContinue'**: not in the spec
// - **Event: 'upgrade'**: upgrade is deprecated in HTTP/2
// - **Event: 'timeout'**: HTTP/2 sockets won't timeout because of application level keepalive
// (PING frames)
// - **Event: 'connect'**: not in the spec, yet (see [http-spec#230][connect])
// - **Event: 'connect'**: not yet supported
// - **server.setTimeout(msecs, [callback])**
// - **server.timeout**
//
Expand Down Expand Up @@ -124,11 +119,9 @@
// - **Event: 'close'**
// - **message.setTimeout(timeout, [callback])**
//
// [1]: http://nodejs.org/api/https.html
// [2]: http://nodejs.org/api/http.html
// [3]: http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-8.1.2.4
// [expect-continue]: https://github.com/http2/http2-spec/issues/18
// [connect]: https://github.com/http2/http2-spec/issues/230
// [1]: https://nodejs.org/api/https.html
// [2]: https://nodejs.org/api/http.html
// [3]: https://tools.ietf.org/html/rfc7540#section-8.1.2.4

// Common server and client side code
// ==================================
Expand Down Expand Up @@ -162,7 +155,7 @@ var deprecatedHeaders = [
// When doing NPN/ALPN negotiation, HTTP/1.1 is used as fallback
var supportedProtocols = [protocol.VERSION, 'http/1.1', 'http/1.0'];

// Ciphersuite list based on the recommendations of http://wiki.mozilla.org/Security/Server_Side_TLS
// Ciphersuite list based on the recommendations of https://wiki.mozilla.org/Security/Server_Side_TLS
// The only modification is that kEDH+AESGCM were placed after DHE and ECDHE suites
var cipherSuites = [
'ECDHE-RSA-AES128-GCM-SHA256',
Expand Down Expand Up @@ -226,7 +219,7 @@ exports.serializers = protocol.serializers;
// ---------------------

function IncomingMessage(stream) {
// * This is basically a read-only wrapper for the [Stream](stream.html) class.
// * This is basically a read-only wrapper for the [Stream](protocol/stream.html) class.
PassThrough.call(this);
stream.pipe(this);
this.socket = this.stream = stream;
Expand All @@ -250,7 +243,7 @@ function IncomingMessage(stream) {
}
IncomingMessage.prototype = Object.create(PassThrough.prototype, { constructor: { value: IncomingMessage } });

// [Request Header Fields](http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-8.1.2.3)
// [Request Header Fields](https://tools.ietf.org/html/rfc7540#section-8.1.2.3)
// * `headers` argument: HTTP/2.0 request and response header fields carry information as a series
// of key-value pairs. This includes the target URI for the request, the status code for the
// response, as well as HTTP header fields.
Expand Down Expand Up @@ -326,7 +319,7 @@ IncomingMessage.prototype._validateHeaders = function _validateHeaders(headers)
// ---------------------

function OutgoingMessage() {
// * This is basically a read-only wrapper for the [Stream](stream.html) class.
// * This is basically a read-only wrapper for the [Stream](protocol/stream.html) class.
Writable.call(this);

this._headers = {};
Expand Down Expand Up @@ -535,7 +528,7 @@ Server.prototype._fallback = function _fallback(socket) {

// There are [3 possible signatures][1] of the `listen` function. Every arguments is forwarded to
// the backing TCP or HTTPS server.
// [1]: http://nodejs.org/api/http.html#http_server_listen_port_hostname_backlog_callback
// [1]: https://nodejs.org/api/http.html#http_server_listen_port_hostname_backlog_callback
Server.prototype.listen = function listen(port, hostname) {
this._log.info({ on: ((typeof hostname === 'string') ? (hostname + ':' + port) : port) },
'Listening for incoming connections');
Expand Down Expand Up @@ -659,7 +652,7 @@ function IncomingRequest(stream) {
}
IncomingRequest.prototype = Object.create(IncomingMessage.prototype, { constructor: { value: IncomingRequest } });

// [Request Header Fields](http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-8.1.2.3)
// [Request Header Fields](https://tools.ietf.org/html/rfc7540#section-8.1.2.3)
// * `headers` argument: HTTP/2.0 request and response header fields carry information as a series
// of key-value pairs. This includes the target URI for the request, the status code for the
// response, as well as HTTP header fields.
Expand Down Expand Up @@ -1214,7 +1207,7 @@ function IncomingResponse(stream) {
}
IncomingResponse.prototype = Object.create(IncomingMessage.prototype, { constructor: { value: IncomingResponse } });

// [Response Header Fields](http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-8.1.2.4)
// [Response Header Fields](https://tools.ietf.org/html/rfc7540#section-8.1.2.4)
// * `headers` argument: HTTP/2.0 request and response header fields carry information as a series
// of key-value pairs. This includes the target URI for the request, the status code for the
// response, as well as HTTP header fields.
Expand Down
10 changes: 5 additions & 5 deletions lib/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// [node-http2][homepage] is an [HTTP/2 (draft 16)][http2] implementation for [node.js][node].
// [node-http2][homepage] is an [HTTP/2][http2] implementation for [node.js][node].
//
// The core of the protocol is implemented in the protocol sub-directory. This directory provides
// two important features on top of the protocol:
Expand All @@ -10,10 +10,10 @@
// (which is in turn very similar to the [HTTP module API][node-http]).
//
// [homepage]: https://github.com/molnarg/node-http2
// [http2]: http://tools.ietf.org/html/draft-ietf-httpbis-http2-16
// [node]: http://nodejs.org/
// [node-https]: http://nodejs.org/api/https.html
// [node-http]: http://nodejs.org/api/http.html
// [http2]: https://tools.ietf.org/html/rfc7540
// [node]: https://nodejs.org/
// [node-https]: https://nodejs.org/api/https.html
// [node-http]: https://nodejs.org/api/http.html

module.exports = require('./http');

Expand Down
46 changes: 32 additions & 14 deletions lib/protocol/compressor.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
// provide a layer between the [framer](framer.html) and the
// [connection handling component](connection.html).
//
// [node-transform]: http://nodejs.org/api/stream.html#stream_class_stream_transform
// [node-objectmode]: http://nodejs.org/api/stream.html#stream_new_stream_readable_options
// [http2-compression]: http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-07
// [node-transform]: https://nodejs.org/api/stream.html#stream_class_stream_transform
// [node-objectmode]: https://nodejs.org/api/stream.html#stream_new_stream_readable_options
// [http2-compression]: https://tools.ietf.org/html/rfc7541

exports.HeaderTable = HeaderTable;
exports.HuffmanTable = HuffmanTable;
Expand All @@ -35,8 +35,8 @@ var util = require('util');
// The [Header Table] is a component used to associate headers to index values. It is basically an
// ordered list of `[name, value]` pairs, so it's implemented as a subclass of `Array`.
// In this implementation, the Header Table and the [Static Table] are handled as a single table.
// [Header Table]: http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-07#section-3.1.2
// [Static Table]: http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-07#appendix-B
// [Header Table]: https://tools.ietf.org/html/rfc7541#section-2.3.2
// [Static Table]: https://tools.ietf.org/html/rfc7541#section-2.3.1
function HeaderTable(log, limit) {
var self = HeaderTable.staticTable.map(entryFromPair);
self._log = log;
Expand Down Expand Up @@ -70,7 +70,7 @@ function size(entry) {
}

// The `add(index, entry)` can be used to [manage the header table][tablemgmt]:
// [tablemgmt]: http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-07#section-3.3
// [tablemgmt]: https://tools.ietf.org/html/rfc7541#section-4
//
// * it pushes the new `entry` at the beggining of the table
// * before doing such a modification, it has to be ensured that the header table size will stay
Expand Down Expand Up @@ -115,9 +115,8 @@ HeaderTable.prototype.setSizeLimit = function setSizeLimit(limit) {
this._enforceLimit(this._limit);
};

// [The Static Table](http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-07#appendix-B)
// [The Static Table](https://tools.ietf.org/html/rfc7541#section-2.3.1)
// ------------------
// [statictable]:http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-07#appendix-B

// The table is generated with feeding the table from the spec to the following sed command:
//
Expand Down Expand Up @@ -208,14 +207,14 @@ function HeaderSetDecompressor(log, table) {

// `_transform` is the implementation of the [corresponding virtual function][_transform] of the
// TransformStream class. It collects the data chunks for later processing.
// [_transform]: http://nodejs.org/api/stream.html#stream_transform_transform_chunk_encoding_callback
// [_transform]: https://nodejs.org/api/stream.html#stream_transform_transform_chunk_encoding_callback
HeaderSetDecompressor.prototype._transform = function _transform(chunk, encoding, callback) {
this._chunks.push(chunk);
callback();
};

// `execute(rep)` executes the given [header representation][representation].
// [representation]: http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-07#section-3.1.4
// [representation]: https://tools.ietf.org/html/rfc7541#section-6

// The *JavaScript object representation* of a header representation:
//
Expand Down Expand Up @@ -281,7 +280,7 @@ HeaderSetDecompressor.prototype._execute = function _execute(rep) {
// `_flush` is the implementation of the [corresponding virtual function][_flush] of the
// TransformStream class. The whole decompressing process is done in `_flush`. It gets called when
// the input stream is over.
// [_flush]: http://nodejs.org/api/stream.html#stream_transform_flush_callback
// [_flush]: https://nodejs.org/api/stream.html#stream_transform_flush_callback
HeaderSetDecompressor.prototype._flush = function _flush(callback) {
var buffer = concat(this._chunks);

Expand Down Expand Up @@ -327,7 +326,7 @@ HeaderSetCompressor.prototype.send = function send(rep) {

// `_transform` is the implementation of the [corresponding virtual function][_transform] of the
// TransformStream class. It processes the input headers one by one:
// [_transform]: http://nodejs.org/api/stream.html#stream_transform_transform_chunk_encoding_callback
// [_transform]: https://nodejs.org/api/stream.html#stream_transform_transform_chunk_encoding_callback
HeaderSetCompressor.prototype._transform = function _transform(pair, encoding, callback) {
var name = pair[0].toLowerCase();
var value = pair[1];
Expand Down Expand Up @@ -373,12 +372,12 @@ HeaderSetCompressor.prototype._transform = function _transform(pair, encoding, c

// `_flush` is the implementation of the [corresponding virtual function][_flush] of the
// TransformStream class. It gets called when there's no more header to compress. The final step:
// [_flush]: http://nodejs.org/api/stream.html#stream_transform_flush_callback
// [_flush]: https://nodejs.org/api/stream.html#stream_transform_flush_callback
HeaderSetCompressor.prototype._flush = function _flush(callback) {
callback();
};

// [Detailed Format](http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-07#section-4)
// [Detailed Format](https://tools.ietf.org/html/rfc7541#section-5)
// -----------------

// ### Integer representation ###
Expand Down Expand Up @@ -1091,18 +1090,37 @@ function Compressor(log, type) {

assert((type === 'REQUEST') || (type === 'RESPONSE'));
this._table = new HeaderTable(this._log);

this.tableSizeChangePending = false;
this.lowestTableSizePending = 0;
this.tableSizeSetting = DEFAULT_HEADER_TABLE_LIMIT;
}

// Changing the header table size
Compressor.prototype.setTableSizeLimit = function setTableSizeLimit(size) {
this._table.setSizeLimit(size);
if (!this.tableSizeChangePending || size < this.lowestTableSizePending) {
this.lowestTableSizePending = size;
}
this.tableSizeSetting = size;
this.tableSizeChangePending = true;
};

// `compress` takes a header set, and compresses it using a new `HeaderSetCompressor` stream
// instance. This means that from now on, the advantages of streaming header encoding are lost,
// but the API becomes simpler.
Compressor.prototype.compress = function compress(headers) {
var compressor = new HeaderSetCompressor(this._log, this._table);

if (this.tableSizeChangePending) {
if (this.lowestTableSizePending < this.tableSizeSetting) {
compressor.send({contextUpdate: true, newMaxSize: this.lowestTableSizePending,
name: "", value: "", index: 0});
}
compressor.send({contextUpdate: true, newMaxSize: this.tableSizeSetting,
name: "", value: "", index: 0});
this.tableSizeChangePending = false;
}
var colonHeaders = [];
var nonColonHeaders = [];

Expand Down
1 change: 1 addition & 0 deletions lib/protocol/connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ priority_loop:
while (bucket.length > 0) {
for (var index = 0; index < bucket.length; index++) {
var stream = bucket[index];
if(!stream || !stream.upstream) continue;
var frame = stream.upstream.read((this._window > 0) ? this._window : -1);

if (!frame) {
Expand Down
Loading

0 comments on commit aec8aaa

Please sign in to comment.