From ec27877ffa070fa7d8343ab4496bd7b053f6e826 Mon Sep 17 00:00:00 2001 From: codedread Date: Fri, 19 Jan 2018 17:40:43 -0800 Subject: [PATCH] Improve parameter validation and test coverage. Also add errorws when trying to seek past the end of the stream --- io/bitstream.js | 73 +++++++++++++++++++++++++--------------------- io/bytestream.js | 65 ++++++++++++++++++++++++++++------------- tests/io-test.html | 71 ++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 150 insertions(+), 59 deletions(-) diff --git a/io/bitstream.js b/io/bitstream.js index e1077d4..4df013f 100644 --- a/io/bitstream.js +++ b/io/bitstream.js @@ -13,6 +13,8 @@ var bitjs = bitjs || {}; bitjs.io = bitjs.io || {}; +// TODO: Add method for pushing bits (multiple arrays) and have tests. +// TODO: Add method for tee-ing off the stream with tests. /** * This bit stream peeks and consumes bits out of a binary stream. */ @@ -25,7 +27,7 @@ bitjs.io.BitStream = class { * @param {Number} opt_length The length of this BitStream */ constructor(ab, rtl, opt_offset, opt_length) { - if (!ab || !ab.toString || ab.toString() !== '[object ArrayBuffer]') { + if (!(ab instanceof ArrayBuffer)) { throw 'Error! BitArray constructed with an invalid ArrayBuffer object'; } @@ -43,13 +45,14 @@ bitjs.io.BitStream = class { * * The bit pointer starts at bit0 of byte0 and moves left until it reaches * bit7 of byte0, then jumps to bit0 of byte1, etc. - * @param {number} n The number of bits to peek. + * @param {number} n The number of bits to peek, must be a positive integer. * @param {boolean=} movePointers Whether to move the pointer, defaults false. * @return {number} The peeked bits, as an unsigned number. */ peekBits_ltr(n, opt_movePointers) { - if (n <= 0 || typeof n != typeof 1) { - return 0; + let num = parseInt(n, 10); + if (n !== num || num <= 0) { + throw 'Error! Called peekBits_ltr() with a non-positive integer'; } const movePointers = opt_movePointers || false; @@ -63,30 +66,29 @@ bitjs.io.BitStream = class { // TODO: Consider putting all bits from bytes we will need into a variable and then // shifting/masking it to just extract the bits we want. // This could be considerably faster when reading more than 3 or 4 bits at a time. - while (n > 0) { + while (num > 0) { if (bytePtr >= bytes.length) { throw 'Error! Overflowed the bit stream! n=' + n + ', bytePtr=' + bytePtr + ', bytes.length=' + bytes.length + ', bitPtr=' + bitPtr; - return -1; } const numBitsLeftInThisByte = (8 - bitPtr); - if (n >= numBitsLeftInThisByte) { + if (num >= numBitsLeftInThisByte) { const mask = (bitjs.io.BitStream.BITMASK[numBitsLeftInThisByte] << bitPtr); result |= (((bytes[bytePtr] & mask) >> bitPtr) << bitsIn); bytePtr++; bitPtr = 0; bitsIn += numBitsLeftInThisByte; - n -= numBitsLeftInThisByte; + num -= numBitsLeftInThisByte; } else { - const mask = (bitjs.io.BitStream.BITMASK[n] << bitPtr); + const mask = (bitjs.io.BitStream.BITMASK[num] << bitPtr); result |= (((bytes[bytePtr] & mask) >> bitPtr) << bitsIn); - bitPtr += n; - bitsIn += n; - n = 0; + bitPtr += num; + bitsIn += num; + num = 0; } } @@ -104,13 +106,14 @@ bitjs.io.BitStream = class { * * The bit pointer starts at bit7 of byte0 and moves right until it reaches * bit0 of byte0, then goes to bit7 of byte1, etc. - * @param {number} n The number of bits to peek. + * @param {number} n The number of bits to peek. Must be a positive integer. * @param {boolean=} movePointers Whether to move the pointer, defaults false. * @return {number} The peeked bits, as an unsigned number. */ peekBits_rtl(n, opt_movePointers) { - if (n <= 0 || typeof n != typeof 1) { - return 0; + let num = parseInt(n, 10); + if (n !== num || num <= 0) { + throw 'Error! Called peekBits_rtl() with a non-positive integer'; } const movePointers = opt_movePointers || false; @@ -123,27 +126,27 @@ bitjs.io.BitStream = class { // TODO: Consider putting all bits from bytes we will need into a variable and then // shifting/masking it to just extract the bits we want. // This could be considerably faster when reading more than 3 or 4 bits at a time. - while (n > 0) { + while (num > 0) { if (bytePtr >= bytes.length) { throw 'Error! Overflowed the bit stream! n=' + n + ', bytePtr=' + bytePtr + ', bytes.length=' + bytes.length + ', bitPtr=' + bitPtr; - return -1; } const numBitsLeftInThisByte = (8 - bitPtr); - if (n >= numBitsLeftInThisByte) { + if (num >= numBitsLeftInThisByte) { result <<= numBitsLeftInThisByte; result |= (bitjs.io.BitStream.BITMASK[numBitsLeftInThisByte] & bytes[bytePtr]); bytePtr++; bitPtr = 0; - n -= numBitsLeftInThisByte; + num -= numBitsLeftInThisByte; } else { - result <<= n; - result |= ((bytes[bytePtr] & (bitjs.io.BitStream.BITMASK[n] << (8 - n - bitPtr))) >> (8 - n - bitPtr)); + result <<= num; + const numBits = 8 - num - bitPtr; + result |= ((bytes[bytePtr] & (bitjs.io.BitStream.BITMASK[num] << numBits)) >> numBits); - bitPtr += n; - n = 0; + bitPtr += num; + num = 0; } } @@ -169,7 +172,7 @@ bitjs.io.BitStream = class { /** * Reads n bits out of the stream, consuming them (moving the bit pointer). - * @param {number} n The number of bits to read. + * @param {number} n The number of bits to read. Must be a positive integer. * @return {number} The read bits, as an unsigned number. */ readBits(n) { @@ -180,29 +183,33 @@ bitjs.io.BitStream = class { * This returns n bytes as a sub-array, advancing the pointer if movePointers * is true. Only use this for uncompressed blocks as this throws away remaining * bits in the current byte. - * @param {number} n The number of bytes to peek. + * @param {number} n The number of bytes to peek. Must be a positive integer. * @param {boolean=} movePointers Whether to move the pointer, defaults false. * @return {Uint8Array} The subarray. */ peekBytes(n, opt_movePointers) { - if (n <= 0 || typeof n != typeof 1) { - return 0; + const num = parseInt(n, 10); + if (n !== num || num <= 0) { + throw 'Error! Called peekBytes() with a non-positive integer'; } + // Flush bits until we are byte-aligned. // from http://tools.ietf.org/html/rfc1951#page-11 // "Any bits of input up to the next byte boundary are ignored." while (this.bitPtr != 0) { this.readBits(1); } + if (this.bytePtr + num > this.bytes.byteLength) { + throw 'Error! Overflowed the bit stream! n=' + num + ', bytePtr=' + this.bytePtr + + ', bytes.length=' + this.bytes.length + ', bitPtr=' + this.bitPtr; + } + + const result = this.bytes.subarray(this.bytePtr, this.bytePtr + num); + const movePointers = opt_movePointers || false; - let bytePtr = this.bytePtr; - let bitPtr = this.bitPtr; - - const result = this.bytes.subarray(bytePtr, bytePtr + n); - if (movePointers) { - this.bytePtr += n; + this.bytePtr += num; } return result; diff --git a/io/bytestream.js b/io/bytestream.js index 2af361e..bf4b98c 100644 --- a/io/bytestream.js +++ b/io/bytestream.js @@ -13,6 +13,8 @@ var bitjs = bitjs || {}; bitjs.io = bitjs.io || {}; +// TODO: Add method for pushing bits (multiple arrays) and have tests. +// TODO: Add method for tee-ing off the stream with tests. /** * This object allows you to peek and consume bytes as numbers and strings * out of an ArrayBuffer. In this buffer, everything must be byte-aligned. @@ -24,6 +26,10 @@ bitjs.io.ByteStream = class { * @param {number=} opt_length The length of this BitStream */ constructor(ab, opt_offset, opt_length) { + if (!(ab instanceof ArrayBuffer)) { + throw 'Error! BitArray constructed with an invalid ArrayBuffer object'; + } + const offset = opt_offset || 0; const length = opt_length || ab.byteLength; this.bytes = new Uint8Array(ab, offset, length); @@ -35,19 +41,24 @@ bitjs.io.ByteStream = class { * Peeks at the next n bytes as an unsigned number but does not advance the * pointer * TODO: This apparently cannot read more than 4 bytes as a number? - * @param {number} n The number of bytes to peek at. + * @param {number} n The number of bytes to peek at. Must be a positive integer. * @return {number} The n bytes interpreted as an unsigned number. */ peekNumber(n) { - // TODO: return error if n would go past the end of the stream? - if (n <= 0 || typeof n != typeof 1) { - return -1; + let num = parseInt(n, 10); + if (n !== num || num <= 0) { + throw 'Error! Called peekNumber() with a non-positive integer'; } let result = 0; // read from last byte to first byte and roll them in - let curByte = this.ptr + n - 1; + let curByte = this.ptr + num - 1; while (curByte >= this.ptr) { + if (curByte >= this.bytes.byteLength) { + throw 'Error! Overflowed the byte stream while peekNumber()! n=' + num + + ', ptr=' + this.ptr + ', bytes.length=' + this.bytes.length; + } + result <<= 8; result |= this.bytes[curByte]; --curByte; @@ -59,11 +70,11 @@ bitjs.io.ByteStream = class { /** * Returns the next n bytes as an unsigned number (or -1 on error) * and advances the stream pointer n bytes. - * @param {number} n The number of bytes to read. + * @param {number} n The number of bytes to read. Must be a positive integer. * @return {number} The n bytes interpreted as an unsigned number. */ readNumber(n) { - const num = this.peekNumber( n ); + const num = this.peekNumber(n); this.ptr += n; return num; } @@ -72,7 +83,7 @@ bitjs.io.ByteStream = class { /** * Returns the next n bytes as a signed number but does not advance the * pointer. - * @param {number} n The number of bytes to read. + * @param {number} n The number of bytes to read. Must be a positive integer. * @return {number} The bytes interpreted as a signed number. */ peekSignedNumber(n) { @@ -88,7 +99,7 @@ bitjs.io.ByteStream = class { /** * Returns the next n bytes as a signed number and advances the stream pointer. - * @param {number} n The number of bytes to read. + * @param {number} n The number of bytes to read. Must be a positive integer. * @return {number} The bytes interpreted as a signed number. */ readSignedNumber(n) { @@ -101,19 +112,25 @@ bitjs.io.ByteStream = class { /** * This returns n bytes as a sub-array, advancing the pointer if movePointers * is true. - * @param {number} n The number of bytes to read. + * @param {number} n The number of bytes to read. Must be a positive integer. * @param {boolean} movePointers Whether to move the pointers. * @return {Uint8Array} The subarray. */ peekBytes(n, movePointers) { - if (n <= 0 || typeof n != typeof 1) { - return null; + let num = parseInt(n, 10); + if (n !== num || num <= 0) { + throw 'Error! Called peekBytes() with a non-positive integer'; } - const result = this.bytes.subarray(this.ptr, this.ptr + n); + if (this.ptr + num > this.bytes.byteLength) { + throw 'Error! Overflowed the byte stream! n=' + num + ', ptr=' + this.ptr + + ', bytes.length=' + this.bytes.length; + } + + const result = this.bytes.subarray(this.ptr, this.ptr + num); if (movePointers) { - this.ptr += n; + this.ptr += num; } return result; @@ -121,7 +138,7 @@ bitjs.io.ByteStream = class { /** * Reads the next n bytes as a sub-array. - * @param {number} n The number of bytes to read. + * @param {number} n The number of bytes to read. Must be a positive integer. * @return {Uint8Array} The subarray. */ readBytes(n) { @@ -129,17 +146,25 @@ bitjs.io.ByteStream = class { } /** - * Peeks at the next n bytes as a string but does not advance the pointer. - * @param {number} n The number of bytes to peek at. + * Peeks at the next n bytes as an ASCII string but does not advance the pointer. + * @param {number} n The number of bytes to peek at. Must be a positive integer. * @return {string} The next n bytes as a string. */ peekString(n) { - if (n <= 0 || typeof n != typeof 1) { - return ""; + let num = parseInt(n, 10); + if (n !== num || num <= 0) { + throw 'Error! Called peekString() with a non-positive integer'; } + // TODO: return error if n would go past the end of the stream. + let result = ""; for (let p = this.ptr, end = this.ptr + n; p < end; ++p) { + if (p >= this.bytes.byteLength) { + throw 'Error! Overflowed the byte stream while peekString()! n=' + num + + ', ptr=' + this.ptr + ', bytes.length=' + this.bytes.length; + } + result += String.fromCharCode(this.bytes[p]); } return result; @@ -148,7 +173,7 @@ bitjs.io.ByteStream = class { /** * Returns the next n bytes as an ASCII string and advances the stream pointer * n bytes. - * @param {number} n The number of bytes to read. + * @param {number} n The number of bytes to read. Must be a positive integer. * @return {string} The next n bytes as a string. */ readString(n) { diff --git a/tests/io-test.html b/tests/io-test.html index a19e824..dbda569 100644 --- a/tests/io-test.html +++ b/tests/io-test.html @@ -15,33 +15,63 @@ buffer = new bitjs.io.ByteBuffer(4); }, tests: { - testWriteNumberThenRead() { + testByteStreamReadBytesPastEnd() { + const stream = new bitjs.io.ByteStream(buffer.data.buffer); + let passed = false; + try { + stream.readBytes(5); + } catch (e) { + passed = true; + } + muther.assert(passed, 'Did not throw when trying to readBytes past end of stream'); + }, + + testWriteNumberThenPeekRead() { buffer.writeNumber(1234, 4); - const stream = new bitjs.io.ByteStream(buffer.data); + const stream = new bitjs.io.ByteStream(buffer.data.buffer); muther.assertEquals(1234, stream.peekNumber(4)); + muther.assertEquals(1234, stream.readNumber(4)); + let passed = false; + try { + stream.readNumber(1); + } catch (e) { + passed = true; + } + muther.assert(passed, 'Did not throw when trying to readNumber past end of stream'); + }, + + testReadStringPastEnd() { + const stream = new bitjs.io.ByteStream(buffer.data.buffer); + let passed = false; + try { + stream.readString(5); + } catch (e) { + passed = true; + } + muther.assert(passed, 'Did not throw when trying to readString past end of stream'); }, testWriteUnsignedByteThenRead() { buffer.writeNumber(192, 1); - const stream = new bitjs.io.ByteStream(buffer.data); + const stream = new bitjs.io.ByteStream(buffer.data.buffer); muther.assertEquals(192, stream.readNumber(1)); }, testWriteSignedByteThenRead() { buffer.writeSignedNumber(-120, 1); - const stream = new bitjs.io.ByteStream(buffer.data); + const stream = new bitjs.io.ByteStream(buffer.data.buffer); muther.assertEquals(-120, stream.readSignedNumber(1)); }, testWriteSignedPositiveNumberThenRead() { buffer.writeSignedNumber(1234, 4); - const stream = new bitjs.io.ByteStream(buffer.data); + const stream = new bitjs.io.ByteStream(buffer.data.buffer); muther.assertEquals(1234, stream.readSignedNumber(4)); }, testWriteSignedNegativeNumberThenRead() { buffer.writeSignedNumber(-128, 1); - const stream = new bitjs.io.ByteStream(buffer.data); + const stream = new bitjs.io.ByteStream(buffer.data.buffer); muther.assertEquals(-128, stream.readSignedNumber(1)); }, @@ -108,7 +138,9 @@ buffer.writeNumber(Number('0b01100101'), 1); const stream = new bitjs.io.BitStream(buffer.data.buffer, false /* rtl */); + // 0101 = 2 + 4 = 6 + muther.assertEquals(5, stream.peekBits(4)); muther.assertEquals(5, stream.readBits(4)); // 101 0110 = 2 + 4 + 16 + 64 = 86 muther.assertEquals(86, stream.readBits(7)); @@ -126,6 +158,33 @@ } muther.assert(passed, 'Did not throw'); }, + + testBitStreamReadBytes() { + buffer.writeNumber(Number('0b01100101'), 1); + buffer.writeNumber(Number('0b01010110'), 1); + + const stream = new bitjs.io.BitStream(buffer.data.buffer); + + let twoBytes = stream.peekBytes(2); + muther.assert(twoBytes instanceof Uint8Array); + muther.assertEquals(2, twoBytes.byteLength); + muther.assertEquals(Number('0b01100101'), twoBytes[0]); + muther.assertEquals(Number('0b01010110'), twoBytes[1]); + + twoBytes = stream.readBytes(2); + muther.assert(twoBytes instanceof Uint8Array); + muther.assertEquals(2, twoBytes.byteLength); + muther.assertEquals(Number('0b01100101'), twoBytes[0]); + muther.assertEquals(Number('0b01010110'), twoBytes[1]); + + let passed = false; + try { + stream.readBytes(3); + } catch (e) { + passed = true; + } + muther.assert(passed, 'Did not throw'); + } }, });