Skip to content

Commit

Permalink
net: fix bytesWritten during writev
Browse files Browse the repository at this point in the history
When a writev is caused on a socket (sometimes through corking and
uncorking), previously net would call Buffer.byteLength on the array of
buffers and chunks. This throws a TypeError, because Buffer.byteLength
throws when passed a non-string.

In dbfe8c4, behavior changed to throw when passed a non-string. This is
correct behavior. Previously, it would cast the argument to a string, so
before this commit, bytesWritten would give an erroneous value. This
commit corrects the behavior equally both before and after dbfe8c4.

This commit fixes this bug by iterating over each chunk in the pending
stack and calculating the length individually. Also adds a regression
test. This additionally changes an `instanceof Buffer` check to `typeof
!== 'string'`, which should be equivalent.

PR-URL: #14236
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Refs: #2960
  • Loading branch information
brendanashworth authored and addaleax committed Aug 1, 2017
1 parent 8fea174 commit 5844691
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 2 deletions.
15 changes: 13 additions & 2 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -825,8 +825,19 @@ protoGetter('bytesWritten', function bytesWritten() {
bytes += Buffer.byteLength(el.chunk, el.encoding);
});

if (data) {
if (data instanceof Buffer)
if (Array.isArray(data)) {
// was a writev, iterate over chunks to get total length
for (var i = 0; i < data.length; i++) {
const chunk = data[i];

if (data.allBuffers || chunk instanceof Buffer)
bytes += chunk.length;
else
bytes += Buffer.byteLength(chunk.chunk, chunk.encoding);
}
} else if (data) {
// Writes are either a string or a Buffer.
if (typeof data !== 'string')
bytes += data.length;
else
bytes += Buffer.byteLength(data, encoding);
Expand Down
35 changes: 35 additions & 0 deletions test/parallel/test-net-socket-byteswritten.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const net = require('net');

const server = net.createServer(function(socket) {
socket.end();
});

server.listen(0, common.mustCall(function() {
const socket = net.connect(server.address().port);

// Cork the socket, then write twice; this should cause a writev, which
// previously caused an err in the bytesWritten count.
socket.cork();

socket.write('one');
socket.write(new Buffer('twø', 'utf8'));

socket.uncork();

// one = 3 bytes, twø = 4 bytes
assert.strictEqual(socket.bytesWritten, 3 + 4);

socket.on('connect', common.mustCall(function() {
assert.strictEqual(socket.bytesWritten, 3 + 4);
}));

socket.on('end', common.mustCall(function() {
assert.strictEqual(socket.bytesWritten, 3 + 4);

server.close();
}));
}));

0 comments on commit 5844691

Please sign in to comment.