Skip to content

Commit

Permalink
Fix splitting for large frames. Fixes #207
Browse files Browse the repository at this point in the history
I'm honestly not sure why this EVER worked - we were never limiting DATA
frames to anything less than the flow control window. This fixes that to
take into account the (default) max allowed payload size.

Tested and works fine for me with node 6.4.0
  • Loading branch information
nwgh committed Sep 6, 2016
1 parent 8a0869b commit ba87b52
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 3 deletions.
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
7 changes: 4 additions & 3 deletions lib/protocol/flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,9 @@ Flow.prototype._parentPush = function _parentPush(frame) {
// did not push the whole frame to the output queue (but maybe it did push part of the frame).
Flow.prototype._push = function _push(frame) {
var data = frame && (frame.type === 'DATA') && frame.data;
var maxFrameLength = (this._window < 16384) ? this._window : 16384;

if (!data || (data.length <= this._window)) {
if (!data || (data.length <= maxFrameLength)) {
return this._parentPush(frame);
}

Expand All @@ -261,12 +262,12 @@ Flow.prototype._push = function _push(frame) {
else {
this._log.trace({ frame: frame, size: frame.data.length, forwardable: this._window },
'Splitting out forwardable part of a DATA frame.');
frame.data = data.slice(this._window);
frame.data = data.slice(maxFrameLength);
this._parentPush({
type: 'DATA',
flags: {},
stream: frame.stream,
data: data.slice(0, this._window)
data: data.slice(0, maxFrameLength)
});
return null;
}
Expand Down

0 comments on commit ba87b52

Please sign in to comment.