1
0
Fork 0

Bug fix/buffer redux (#9229)

* Add failing tests

* Add tests for Buffer#slice

* Add tests for Buffer#new

* Skip failing tests

* Implement changes

* Unskip failing tests

This reverts commit 9f8a34bd74e152b7b3158fea79b12e6da8343001.

* Fix implementation

* Remove meaningless test

This test only succeeded because of a broken implementation of the Buffer constructor. It doesn't really do anything useful.

* Add note to CHANGELOG
This commit is contained in:
Alan Plum 2019-06-12 15:00:43 +02:00 committed by Michael Hackstein
parent 7119ccfdf1
commit a6b49e3a7e
4 changed files with 323 additions and 131 deletions

View File

@ -1,6 +1,10 @@
devel
-----
* fixed `Buffer.alloc` method
* `Buffer` is now iterable and accepts `ArrayBuffer` values as input
* fix timeout-response in case of simultaneous index create/drop in cluster
* allow pregel to select the shard key via `shardKeyAttribute` in pregel start parameters

View File

@ -19,7 +19,15 @@ global.DEFINE_MODULE('buffer', (function () {
var SlowBuffer = require('internal').SlowBuffer;
SlowBuffer.prototype._PRINT = function(context) {
context.output += '[Buffer, length: ' + this.length + ']';
context.output += '<SlowBuffer';
for (let i = 0; i < Math.min(this.length, 50); i++) {
context.output += ' ';
context.output += this.hexSlice(i, i + 1);
}
if (this.length > 50) {
context.output += '... ';
}
context.output += '>';
};
// Copyright Joyent, Inc. and other Node contributors.
@ -162,109 +170,108 @@ global.DEFINE_MODULE('buffer', (function () {
var len = this.length;
start = clamp(start, len, 0);
end = clamp(end, len, len);
return new Buffer(this, end - start, start);
return Buffer.from(this, end - start, start);
};
// Buffer
function Buffer (subject, encoding, offset) {
var handler = {
get: function(target, name) {
if (name === '__buffer__') { return true; }
if (typeof name === 'string' && name.match(/^\d+$/)) {
return target.parent[name];
}
return target[name];
},
set: function(target, name, value) {
if (typeof name === 'string' && name.match(/^\d+$/)) {
target.parent[name] = value;
} else {
target[name] = value;
}
return true;
}
};
if (!(this instanceof Buffer)) {
var b = new Buffer(subject, encoding, offset);
return new Proxy(b, handler);
function Buffer (subject, offsetOrEncoding, length) {
if (typeof subject === 'number') {
return Buffer.alloc(subject);
}
var type;
// Are we slicing?
if (typeof offset === 'number') {
if (!Buffer.isBuffer(subject)) {
throw new TypeError('First argument must be a Buffer when slicing');
}
this.length = +encoding > 0 ? Math.ceil(encoding) : 0;
this.parent = subject.parent ? subject.parent : subject;
this.offset = offset;
} else {
// Find the length
switch (type = typeof subject) {
case 'number':
this.length = +subject > 0 ? Math.ceil(subject) : 0;
break;
case 'string':
this.length = Buffer.byteLength(subject, encoding);
break;
case 'object': // Assume object is array-ish
this.length = +subject.length > 0 ? Math.ceil(subject.length) : 0;
break;
default:
throw new TypeError('First argument needs to be a number, ' +
'array or string.');
}
// note: the following condition means we'll always create a SlowBuffer
// for non-empty lengths. This is required to circumvent using the shared
// pool, which is only allocated but never freed (and therefore leaks
// memory on shutdown)
if (this.length > 0) {
// Big buffer, just alloc one.
this.parent = new SlowBuffer(this.length);
this.offset = 0;
} else {
// Zero-length buffer
this.parent = new SlowBuffer(0);
this.offset = 0;
}
// optimize by branching logic for new allocations
if (typeof subject !== 'number') {
if (type === 'string') {
// We are a string
this.length = this.write(subject, 0, encoding);
// if subject is buffer then use built-in copy method
} else if (Buffer.isBuffer(subject)) {
if (subject.parent)
subject.parent.copy(this.parent,
this.offset,
subject.offset,
this.length + subject.offset);
else
subject.copy(this.parent, this.offset, 0, this.length);
} else if (isArrayIsh(subject)) {
for (var i = 0; i < this.length; i++)
this.parent[i + this.offset] = subject[i];
}
}
}
return new Proxy(this, handler);
// SlowBuffer.makeFastBuffer(this.parent, this, this.offset, this.length)
return Buffer.from(subject, offsetOrEncoding, length);
// SlowBuffer.makeFastBuffer(this.parent, this, this.offset, this.length)
}
const BUFFER_PROXY_HANDLER = {
get (target, name) {
if (name === '__buffer__') { return true; }
if (typeof name === 'string' && name.match(/^\d+$/)) {
return target.parent[name];
}
return target[name];
},
set (target, name, value) {
if (typeof name === 'string' && name.match(/^\d+$/)) {
target.parent[name] = value;
} else {
target[name] = value;
}
return true;
}
};
function createBufferFromString(subject, encoding) {
const buffer = Object.create(Buffer.prototype);
const length = Buffer.byteLength(subject, encoding);
buffer.parent = new SlowBuffer(length);
buffer.offset = 0;
buffer.length = buffer.parent.write(subject, encoding);
return new Proxy(buffer, BUFFER_PROXY_HANDLER);
}
function createBufferFromArrayBuffer(subject, byteOffset = 0, length = subject.byteLength - byteOffset) {
const buffer = Object.create(Buffer.prototype);
const dv = new DataView(subject);
buffer.parent = new SlowBuffer(length);
buffer.offset = 0;
buffer.length = length;
for (let i = 0; i < length; i++) {
buffer.parent[i] = dv.getUint8(byteOffset + i);
}
return new Proxy(buffer, BUFFER_PROXY_HANDLER);
}
function createBufferFromByteArray(subject) {
const buffer = Object.create(Buffer.prototype);
buffer.parent = new SlowBuffer(subject.length);
buffer.offset = 0;
buffer.length = subject.length;
for (let i = 0; i < subject.length; i++) {
buffer.parent[i] = subject[i];
}
return new Proxy(buffer, BUFFER_PROXY_HANDLER);
}
function createBufferFromBuffer(subject, byteLength, start) {
// NOTE for historical reasons this behaves as a `copy` by default but as
// a `slice` if either `byteLength` or `start` is set.
if (typeof start !== "undefined" || typeof byteLength !== "undefined") {
if (start === undefined) start = 0;
if (byteLength === undefined) byteLength = subject.length - start;
const buffer = Object.create(Buffer.prototype);
buffer.parent = subject.parent || subject;
buffer.offset = start;
buffer.length = byteLength;
return new Proxy(buffer, BUFFER_PROXY_HANDLER);
}
const buffer = Buffer.allocUnsafe(subject.length);
subject.copy(buffer);
return buffer;
}
Buffer.prototype.values = Buffer.prototype[Symbol.iterator] = function * () {
for (let i = 0; i < this.length; i++) {
yield this[i];
}
};
Buffer.prototype.entries = function * () {
for (let i = 0; i < this.length; i++) {
yield [i, this[i]];
}
};
Buffer.prototype.keys = function * () {
for (let i = 0; i < this.length; i++) {
yield i;
}
};
Buffer.prototype._PRINT = function (context) {
context.output += '<Buffer';
for (let i = 0; i < Math.min(this.length, 50); i++) {
context.output += ' ';
context.output += this.parent.hexSlice(i, i + 1);
context.output += this.parent.hexSlice(this.offset + i, this.offset + i + 1);
}
if (this.length > 50) {
context.output += '... ';
@ -280,12 +287,6 @@ global.DEFINE_MODULE('buffer', (function () {
return this.parent.hexWrite(str, offset, maxLength);
};
function isArrayIsh (subject) {
return Array.isArray(subject) ||
subject && typeof subject === 'object' &&
typeof subject.length === 'number';
}
exports.SlowBuffer = SlowBuffer;
exports.Buffer = Buffer;
@ -421,9 +422,15 @@ global.DEFINE_MODULE('buffer', (function () {
Buffer.prototype.toJSON = function () {
if (this.parent) {
return Array.prototype.slice.call(this.parent, this.offset, this.offset + this.length);
return {
type: "Buffer",
data: Array.prototype.slice.call(this.parent, this.offset, this.offset + this.length)
};
}
return Array.prototype.slice.call(this, 0);
return {
type: "Buffer",
data: Array.prototype.slice.call(this, 0)
};
};
// toString(encoding, start=0, end=buffer.length)
@ -514,7 +521,7 @@ global.DEFINE_MODULE('buffer', (function () {
}
if (list.length === 0) {
return new Buffer(0);
return Buffer.alloc(0);
} else if (list.length === 1) {
return list[0];
}
@ -527,7 +534,7 @@ global.DEFINE_MODULE('buffer', (function () {
}
}
var buffer = new Buffer(length);
var buffer = Buffer.alloc(length);
var pos = 0;
for (var i = 0; i < list.length; i++) {
var buf = list[i];
@ -538,12 +545,7 @@ global.DEFINE_MODULE('buffer', (function () {
};
// copy(targetBuffer, targetStart=0, sourceStart=0, sourceEnd=buffer.length)
Buffer.prototype.copy = function (target, target_start, start, end) {
// set undefined/NaN or out of bounds values equal to their default
if (!(target_start >= 0)) target_start = 0;
if (!(start >= 0)) start = 0;
if (!(end < this.length)) end = this.length;
Buffer.prototype.copy = function (target, target_start = 0, start = 0, end = this.length) {
// Copy 0 bytes; we're done
if (end === start ||
target.length === 0 ||
@ -571,7 +573,7 @@ global.DEFINE_MODULE('buffer', (function () {
var len = this.length;
start = clamp(start, len, 0);
end = clamp(end, len, len);
return new Buffer(this.parent, end - start, start + this.offset);
return Buffer.from(this.parent, end - start, start + this.offset);
};
// Legacy methods for backwards compatibility.
@ -949,25 +951,56 @@ global.DEFINE_MODULE('buffer', (function () {
this.parent.writeDoubleBE(value, this.offset + offset, !!noAssert);
};
Buffer.from = function (value, encoding) {
if (typeof value === 'number') {
throw new TypeError('The "value" argument must not be of type number. Received type number');
Buffer.from = function (subject, offsetOrEncoding, length) {
if (typeof subject === 'number') {
throw new TypeError(`The "value" argument must not be of type number. Received type ${typeof subject}`);
}
return new Buffer(value, encoding);
if (Array.isArray(subject)) {
return createBufferFromByteArray(subject);
}
if (typeof subject === 'string') {
return createBufferFromString(subject, offsetOrEncoding);
}
if (subject instanceof ArrayBuffer) {
return createBufferFromArrayBuffer(subject, offsetOrEncoding, length);
}
if (Buffer.isBuffer(subject)) {
// NOTE for historical reasons the order of the offset and length
// arguments are inverted when calling `Buffer.from` with a buffer.
// This means offsetOrEncoding is the length and length is the offset.
return createBufferFromBuffer(subject, offsetOrEncoding, length);
}
if (typeof subject[Symbol.toPrimitive] === 'function') {
const value = subject[Symbol.toPrimitive]();
if (value !== subject) {
return Buffer.from(value, offsetOrEncoding, length);
}
}
if (typeof subject.valueOf === 'function') {
const value = subject.valueOf();
if (value !== subject) {
return Buffer.from(value, offsetOrEncoding, length);
}
}
throw new TypeError(`The first argument must be one of type string, Buffer, ArrayBuffer, Array or Array-like Object. Received type ${typeof subject}`);
};
Buffer.of = function (...values) {
return new Buffer(values);
return Buffer.from(values);
};
Buffer.alloc = function (size) {
const buf = new Buffer(size);
buf.alloc(size);
const buf = Buffer.allocUnsafe(size);
buf.fill(0);
return buf;
};
Buffer.allocUnsafe = Buffer.allocUnsafeSlow = function (size) {
return new Buffer(size);
const buffer = Object.create(Buffer.prototype);
buffer.length = size;
buffer.offset = 0;
buffer.parent = new SlowBuffer(size);
return new Proxy(buffer, BUFFER_PROXY_HANDLER);
};
return exports;

View File

@ -162,7 +162,7 @@ describe('Foxx arangoUser', function () {
expect(result.body).to.eql(JSON.stringify({user: null}));
});
it('should not set the arangoUser object if not authenticated correctly - str array', function () {
it('should not set the arangoUser object if not authenticated correctly - str empty', function () {
const opts = { headers: {
authorization: (
'Basic ' + new Buffer([]).toString('base64')
@ -173,15 +173,4 @@ describe('Foxx arangoUser', function () {
expect(result.body).to.eql(JSON.stringify({user: null}));
});
it('should not set the arangoUser object if not authenticated correctly - str obj', function () {
const opts = { headers: {
authorization: (
'Basic ' + new Buffer({}).toString('base64')
)
}};
const result = internal.download(url + mount, '', opts);
expect(result.code).to.equal(200);
expect(result.body).to.eql(JSON.stringify({user: null}));
});
});

View File

@ -0,0 +1,166 @@
/* global describe, it, beforeEach */
'use strict';
const { expect } = require('chai');
describe('Buffer', function () {
describe('new', function () {
it('should accept numbers', () => {
const buf = new Buffer(32);
buf.fill(0);
expect(buf.length).to.equal(32);
});
it('should accept a string', () => {
const value = "hello world";
const buf = new Buffer(value);
expect(buf.toString()).to.equal(value);
});
it('should accept a buffer and return a copy', () => {
const value = "hello world";
const buf0 = new Buffer(value);
const buf = new Buffer(buf0);
expect(buf.toString()).to.equal(value);
buf0.fill(0);
expect(buf.toString()).to.equal(value);
});
it('should accept a buffer with offset/length and return a slice', () => {
const value = "hello world";
const buf0 = new Buffer(value);
const buf = new Buffer(buf0, 3, 1);
expect(buf.length).to.equal(3);
expect(buf.toString()).to.equal(value.slice(1, 4));
buf.fill(0);
expect(buf0[0]).to.equal("hello world".charCodeAt(0));
expect(buf0[1]).to.equal(0);
expect(buf0[2]).to.equal(0);
expect(buf0[3]).to.equal(0);
expect(buf0[4]).to.equal("hello world".charCodeAt(4));
});
it('should accept an ArrayBuffer', () => {
const values = [4, 8, 15, 16, 23, 42];
const ab = new ArrayBuffer(values.length);
const dv = new DataView(ab);
for (let i = 0; i < values.length; i++) {
dv.setUint8(i, values[i]);
}
const buf = new Buffer(ab);
expect([...buf]).to.eql(values);
});
it('should accept an array of byte values', () => {
const values = [4, 8, 15, 16, 23, 42];
const buf = new Buffer(values);
expect([...buf]).to.eql(values);
});
});
describe('from', function () {
it('should not accept numbers', () => {
try {
Buffer.from(32);
} catch (e) {
expect(e).to.be.instanceof(TypeError);
return;
}
expect.fail("Expected an exception");
});
it('should accept a string', () => {
const value = "hello world";
const buf = Buffer.from(value);
expect(buf.toString()).to.equal(value);
});
it('should accept a buffer and return a copy', () => {
const value = "hello world";
const buf0 = Buffer.from(value);
const buf = Buffer.from(buf0);
expect(buf.toString()).to.equal(value);
buf0.fill(0);
expect(buf.toString()).to.equal(value);
});
it('should accept an ArrayBuffer', () => {
const values = [4, 8, 15, 16, 23, 42];
const ab = new ArrayBuffer(values.length);
const dv = new DataView(ab);
for (let i = 0; i < values.length; i++) {
dv.setUint8(i, values[i]);
}
const buf = Buffer.from(ab);
expect([...buf]).to.eql(values);
});
it('should accept an array of byte values', () => {
const values = [4, 8, 15, 16, 23, 42];
const buf = Buffer.from(values);
expect([...buf]).to.eql(values);
});
});
describe('of', function () {
it('should create a buffer of a byte sequence', () => {
const values = [4, 8, 15, 16, 23, 42];
const buf = Buffer.of(...values);
expect([...buf]).to.eql(values);
});
});
describe('alloc', function () {
it('should create a zeroed buffer of the given size', () => {
const size = 23;
const buf = Buffer.alloc(size);
expect(buf).to.have.property('length', size);
expect([...buf]).to.eql(Array(size).fill(0));
});
});
describe('allocUnsafe', function () {
it('should create an uninitialized buffer of the given size', () => {
const size = 23;
const buf = Buffer.alloc(size);
expect(buf).to.have.property('length', size);
});
});
describe('instance', function () {
it('should be iterable', () => {
const values = [4, 8, 15, 16, 23, 42];
expect(Buffer.prototype).to.have.property(Symbol.iterator);
expect([...Buffer.of(...values)]).to.eql(values);
});
describe('values', function () {
it('should return values iterator', () => {
const values = [4, 8, 15, 16, 23, 42];
const iterator = Buffer.of(...values).values();
expect(iterator).to.have.property('next');
expect([...iterator]).to.eql([...values.values()]);
});
});
describe('keys', function () {
it('should return keys iterator', () => {
const values = [4, 8, 15, 16, 23, 42];
const iterator = Buffer.of(...values).keys();
expect(iterator).to.have.property('next');
expect([...iterator]).to.eql([...values.keys()]);
});
});
describe('entries', function () {
it('should return key-value tuples', () => {
const values = [4, 8, 15, 16, 23, 42];
const iterator = Buffer.of(...values).entries();
expect(iterator).to.have.property('next');
expect([...iterator]).to.eql([...values.entries()]);
});
});
describe('slice', function () {
it('should propagate changes from parent to child', () => {
const parent = Buffer.from('hello');
const slice = parent.slice(1, 4);
slice.fill(0);
expect(parent[0]).to.equal('hello'.charCodeAt(0));
expect(parent[1]).to.equal(0);
expect(parent[2]).to.equal(0);
expect(parent[3]).to.equal(0);
expect(parent[4]).to.equal('hello'.charCodeAt(4));
});
it('should propagate changes from child to parent', () => {
const parent = Buffer.from('hello');
const slice = parent.slice(1, 4);
parent.fill(0);
expect(slice[0]).to.equal(0);
expect(slice[1]).to.equal(0);
expect(slice[2]).to.equal(0);
});
});
});
});