From b2d7fb1b8a64b611a7ff8744852aae08a44b6a8e Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Wed, 22 Mar 2023 10:45:37 -0400 Subject: [PATCH 1/3] stream: use private properties for compression --- lib/internal/webstreams/compression.js | 68 ++++++------------- .../test-whatwg-webstreams-compression.js | 12 ++-- 2 files changed, 29 insertions(+), 51 deletions(-) diff --git a/lib/internal/webstreams/compression.js b/lib/internal/webstreams/compression.js index 2ba8e53fa28e73..6cbaa3f3250e73 100644 --- a/lib/internal/webstreams/compression.js +++ b/lib/internal/webstreams/compression.js @@ -2,14 +2,10 @@ const { ObjectDefineProperties, - Symbol, } = primordials; const { - codes: { - ERR_INVALID_ARG_VALUE, - ERR_INVALID_THIS, - }, + codes: { ERR_INVALID_ARG_VALUE }, } = require('internal/errors'); const { @@ -29,42 +25,30 @@ function lazyZlib() { return zlib; } -const kHandle = Symbol('kHandle'); -const kTransform = Symbol('kTransform'); -const kType = Symbol('kType'); - /** * @typedef {import('./readablestream').ReadableStream} ReadableStream * @typedef {import('./writablestream').WritableStream} WritableStream */ -function isCompressionStream(value) { - return typeof value?.[kHandle] === 'object' && - value?.[kType] === 'CompressionStream'; -} - -function isDecompressionStream(value) { - return typeof value?.[kHandle] === 'object' && - value?.[kType] === 'DecompressionStream'; -} - class CompressionStream { + #handle; + #transform; + /** * @param {'deflate'|'gzip'} format */ constructor(format) { - this[kType] = 'CompressionStream'; switch (format) { case 'deflate': - this[kHandle] = lazyZlib().createDeflate(); + this.#handle = lazyZlib().createDeflate(); break; case 'gzip': - this[kHandle] = lazyZlib().createGzip(); + this.#handle = lazyZlib().createGzip(); break; default: throw new ERR_INVALID_ARG_VALUE('format', format); } - this[kTransform] = newReadableWritablePairFromDuplex(this[kHandle]); + this.#transform = newReadableWritablePairFromDuplex(this.#handle); } /** @@ -72,9 +56,7 @@ class CompressionStream { * @type {ReadableStream} */ get readable() { - if (!isCompressionStream(this)) - throw new ERR_INVALID_THIS('CompressionStream'); - return this[kTransform].readable; + return this.#transform.readable; } /** @@ -82,38 +64,36 @@ class CompressionStream { * @type {WritableStream} */ get writable() { - if (!isCompressionStream(this)) - throw new ERR_INVALID_THIS('CompressionStream'); - return this[kTransform].writable; + return this.#transform.writable; } [kInspect](depth, options) { - if (!isCompressionStream(this)) - throw new ERR_INVALID_THIS('CompressionStream'); customInspect(depth, options, 'CompressionStream', { - readable: this[kTransform].readable, - writable: this[kTransform].writable, + readable: this.#transform.readable, + writable: this.#transform.writable, }); } } class DecompressionStream { + #handle; + #transform; + /** * @param {'deflate'|'gzip'} format */ constructor(format) { - this[kType] = 'DecompressionStream'; switch (format) { case 'deflate': - this[kHandle] = lazyZlib().createInflate(); + this.#handle = lazyZlib().createInflate(); break; case 'gzip': - this[kHandle] = lazyZlib().createGunzip(); + this.#handle = lazyZlib().createGunzip(); break; default: throw new ERR_INVALID_ARG_VALUE('format', format); } - this[kTransform] = newReadableWritablePairFromDuplex(this[kHandle]); + this.#transform = newReadableWritablePairFromDuplex(this.#handle); } /** @@ -121,9 +101,7 @@ class DecompressionStream { * @type {ReadableStream} */ get readable() { - if (!isDecompressionStream(this)) - throw new ERR_INVALID_THIS('DecompressionStream'); - return this[kTransform].readable; + return this.#transform.readable; } /** @@ -131,17 +109,13 @@ class DecompressionStream { * @type {WritableStream} */ get writable() { - if (!isDecompressionStream(this)) - throw new ERR_INVALID_THIS('DecompressionStream'); - return this[kTransform].writable; + return this.#transform.writable; } [kInspect](depth, options) { - if (!isDecompressionStream(this)) - throw new ERR_INVALID_THIS('DecompressionStream'); customInspect(depth, options, 'DecompressionStream', { - readable: this[kTransform].readable, - writable: this[kTransform].writable, + readable: this.#transform.readable, + writable: this.#transform.writable, }); } } diff --git a/test/parallel/test-whatwg-webstreams-compression.js b/test/parallel/test-whatwg-webstreams-compression.js index 859df4e07bed88..c527fc1f13d10e 100644 --- a/test/parallel/test-whatwg-webstreams-compression.js +++ b/test/parallel/test-whatwg-webstreams-compression.js @@ -51,17 +51,21 @@ Promise.all(['gzip', 'deflate'].map((i) => test(i))).then(common.mustCall()); assert.throws( () => Reflect.get(CompressionStream.prototype, 'readable', {}), { - code: 'ERR_INVALID_THIS', + name: 'TypeError', + message: /Cannot read private member/, }); assert.throws( () => Reflect.get(CompressionStream.prototype, 'writable', {}), { - code: 'ERR_INVALID_THIS', + name: 'TypeError', + message: /Cannot read private member/, }); assert.throws( () => Reflect.get(DecompressionStream.prototype, 'readable', {}), { - code: 'ERR_INVALID_THIS', + name: 'TypeError', + message: /Cannot read private member/, }); assert.throws( () => Reflect.get(DecompressionStream.prototype, 'writable', {}), { - code: 'ERR_INVALID_THIS', + name: 'TypeError', + message: /Cannot read private member/, }); From 750abfa6209c224943bc9908ce1138ff2dc82c25 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Wed, 22 Mar 2023 11:23:36 -0400 Subject: [PATCH 2/3] stream: use private properties for encoding --- lib/internal/webstreams/encoding.js | 102 +++++++----------- .../test-whatwg-webstreams-encoding.js | 24 +++-- 2 files changed, 53 insertions(+), 73 deletions(-) diff --git a/lib/internal/webstreams/encoding.js b/lib/internal/webstreams/encoding.js index b95441e12e0cf1..b5533b4287b65a 100644 --- a/lib/internal/webstreams/encoding.js +++ b/lib/internal/webstreams/encoding.js @@ -4,7 +4,6 @@ const { ObjectDefineProperties, String, StringPrototypeCharCodeAt, - Symbol, Uint8Array, } = primordials; @@ -31,32 +30,19 @@ const { kEnumerableProperty, } = require('internal/util'); -const kHandle = Symbol('kHandle'); -const kTransform = Symbol('kTransform'); -const kType = Symbol('kType'); -const kPendingHighSurrogate = Symbol('kPendingHighSurrogate'); - /** * @typedef {import('./readablestream').ReadableStream} ReadableStream * @typedef {import('./writablestream').WritableStream} WritableStream */ -function isTextEncoderStream(value) { - return typeof value?.[kHandle] === 'object' && - value?.[kType] === 'TextEncoderStream'; -} - -function isTextDecoderStream(value) { - return typeof value?.[kHandle] === 'object' && - value?.[kType] === 'TextDecoderStream'; -} - class TextEncoderStream { + #pendingHighSurrogate = null; + #handle; + #transform; + constructor() { - this[kPendingHighSurrogate] = null; - this[kType] = 'TextEncoderStream'; - this[kHandle] = new TextEncoder(); - this[kTransform] = new TransformStream({ + this.#handle = new TextEncoder(); + this.#transform = new TransformStream({ transform: (chunk, controller) => { // https://encoding.spec.whatwg.org/#encode-and-enqueue-a-chunk chunk = String(chunk); @@ -64,9 +50,9 @@ class TextEncoderStream { for (let i = 0; i < chunk.length; i++) { const item = chunk[i]; const codeUnit = StringPrototypeCharCodeAt(item, 0); - if (this[kPendingHighSurrogate] !== null) { - const highSurrogate = this[kPendingHighSurrogate]; - this[kPendingHighSurrogate] = null; + if (this.#pendingHighSurrogate !== null) { + const highSurrogate = this.#pendingHighSurrogate; + this.#pendingHighSurrogate = null; if (0xDC00 <= codeUnit && codeUnit <= 0xDFFF) { finalChunk += highSurrogate + item; continue; @@ -74,7 +60,7 @@ class TextEncoderStream { finalChunk += '\uFFFD'; } if (0xD800 <= codeUnit && codeUnit <= 0xDBFF) { - this[kPendingHighSurrogate] = item; + this.#pendingHighSurrogate = item; continue; } if (0xDC00 <= codeUnit && codeUnit <= 0xDFFF) { @@ -84,13 +70,13 @@ class TextEncoderStream { finalChunk += item; } if (finalChunk) { - const value = this[kHandle].encode(finalChunk); + const value = this.#handle.encode(finalChunk); controller.enqueue(value); } }, flush: (controller) => { // https://encoding.spec.whatwg.org/#encode-and-flush - if (this[kPendingHighSurrogate] !== null) { + if (this.#pendingHighSurrogate !== null) { controller.enqueue(new Uint8Array([0xEF, 0xBF, 0xBD])); } }, @@ -102,9 +88,7 @@ class TextEncoderStream { * @type {string} */ get encoding() { - if (!isTextEncoderStream(this)) - throw new ERR_INVALID_THIS('TextEncoderStream'); - return this[kHandle].encoding; + return this.#handle.encoding; } /** @@ -112,9 +96,7 @@ class TextEncoderStream { * @type {ReadableStream} */ get readable() { - if (!isTextEncoderStream(this)) - throw new ERR_INVALID_THIS('TextEncoderStream'); - return this[kTransform].readable; + return this.#transform.readable; } /** @@ -122,23 +104,24 @@ class TextEncoderStream { * @type {WritableStream} */ get writable() { - if (!isTextEncoderStream(this)) - throw new ERR_INVALID_THIS('TextEncoderStream'); - return this[kTransform].writable; + return this.#transform.writable; } [kInspect](depth, options) { - if (!isTextEncoderStream(this)) + if (this == null) throw new ERR_INVALID_THIS('TextEncoderStream'); return customInspect(depth, options, 'TextEncoderStream', { - encoding: this[kHandle].encoding, - readable: this[kTransform].readable, - writable: this[kTransform].writable, + encoding: this.#handle.encoding, + readable: this.#transform.readable, + writable: this.#transform.writable, }); } } class TextDecoderStream { + #handle; + #transform; + /** * @param {string} [encoding] * @param {{ @@ -147,16 +130,15 @@ class TextDecoderStream { * }} [options] */ constructor(encoding = 'utf-8', options = kEmptyObject) { - this[kType] = 'TextDecoderStream'; - this[kHandle] = new TextDecoder(encoding, options); - this[kTransform] = new TransformStream({ + this.#handle = new TextDecoder(encoding, options); + this.#transform = new TransformStream({ transform: (chunk, controller) => { - const value = this[kHandle].decode(chunk, { stream: true }); + const value = this.#handle.decode(chunk, { stream: true }); if (value) controller.enqueue(value); }, flush: (controller) => { - const value = this[kHandle].decode(); + const value = this.#handle.decode(); if (value) controller.enqueue(value); controller.terminate(); @@ -169,9 +151,7 @@ class TextDecoderStream { * @type {string} */ get encoding() { - if (!isTextDecoderStream(this)) - throw new ERR_INVALID_THIS('TextDecoderStream'); - return this[kHandle].encoding; + return this.#handle.encoding; } /** @@ -179,9 +159,7 @@ class TextDecoderStream { * @type {boolean} */ get fatal() { - if (!isTextDecoderStream(this)) - throw new ERR_INVALID_THIS('TextDecoderStream'); - return this[kHandle].fatal; + return this.#handle.fatal; } /** @@ -189,9 +167,7 @@ class TextDecoderStream { * @type {boolean} */ get ignoreBOM() { - if (!isTextDecoderStream(this)) - throw new ERR_INVALID_THIS('TextDecoderStream'); - return this[kHandle].ignoreBOM; + return this.#handle.ignoreBOM; } /** @@ -199,9 +175,7 @@ class TextDecoderStream { * @type {ReadableStream} */ get readable() { - if (!isTextDecoderStream(this)) - throw new ERR_INVALID_THIS('TextDecoderStream'); - return this[kTransform].readable; + return this.#transform.readable; } /** @@ -209,20 +183,18 @@ class TextDecoderStream { * @type {WritableStream} */ get writable() { - if (!isTextDecoderStream(this)) - throw new ERR_INVALID_THIS('TextDecoderStream'); - return this[kTransform].writable; + return this.#transform.writable; } [kInspect](depth, options) { - if (!isTextDecoderStream(this)) + if (this == null) throw new ERR_INVALID_THIS('TextDecoderStream'); return customInspect(depth, options, 'TextDecoderStream', { - encoding: this[kHandle].encoding, - fatal: this[kHandle].fatal, - ignoreBOM: this[kHandle].ignoreBOM, - readable: this[kTransform].readable, - writable: this[kTransform].writable, + encoding: this.#handle.encoding, + fatal: this.#handle.fatal, + ignoreBOM: this.#handle.ignoreBOM, + readable: this.#transform.readable, + writable: this.#transform.writable, }); } } diff --git a/test/parallel/test-whatwg-webstreams-encoding.js b/test/parallel/test-whatwg-webstreams-encoding.js index 97061650496c0d..b2b18df73bf1fe 100644 --- a/test/parallel/test-whatwg-webstreams-encoding.js +++ b/test/parallel/test-whatwg-webstreams-encoding.js @@ -48,23 +48,28 @@ const kEuro = Buffer.from([0xe2, 0x82, 0xac]).toString(); assert.throws( () => Reflect.get(TextDecoderStream.prototype, 'encoding', {}), { - code: 'ERR_INVALID_THIS', + name: 'TypeError', + message: /Cannot read private member/, }); assert.throws( () => Reflect.get(TextDecoderStream.prototype, 'fatal', {}), { - code: 'ERR_INVALID_THIS', + name: 'TypeError', + message: /Cannot read private member/, }); assert.throws( () => Reflect.get(TextDecoderStream.prototype, 'ignoreBOM', {}), { - code: 'ERR_INVALID_THIS', + name: 'TypeError', + message: /Cannot read private member/, }); assert.throws( () => Reflect.get(TextDecoderStream.prototype, 'readable', {}), { - code: 'ERR_INVALID_THIS', + name: 'TypeError', + message: /Cannot read private member/, }); assert.throws( () => Reflect.get(TextDecoderStream.prototype, 'writable', {}), { - code: 'ERR_INVALID_THIS', + name: 'TypeError', + message: /Cannot read private member/, }); } @@ -89,14 +94,17 @@ const kEuro = Buffer.from([0xe2, 0x82, 0xac]).toString(); assert.throws( () => Reflect.get(TextEncoderStream.prototype, 'encoding', {}), { - code: 'ERR_INVALID_THIS', + name: 'TypeError', + message: /Cannot read private member/, }); assert.throws( () => Reflect.get(TextEncoderStream.prototype, 'readable', {}), { - code: 'ERR_INVALID_THIS', + name: 'TypeError', + message: /Cannot read private member/, }); assert.throws( () => Reflect.get(TextEncoderStream.prototype, 'writable', {}), { - code: 'ERR_INVALID_THIS', + name: 'TypeError', + message: /Cannot read private member/, }); } From 70ed3b26a4a30e10083936aeff15910ae0dc7fcf Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Wed, 22 Mar 2023 11:32:24 -0400 Subject: [PATCH 3/3] stream: use private properties for strategies --- lib/internal/webstreams/queuingstrategies.js | 40 ++++++------------- .../test-whatwg-webstreams-coverage.js | 12 ++++-- 2 files changed, 20 insertions(+), 32 deletions(-) diff --git a/lib/internal/webstreams/queuingstrategies.js b/lib/internal/webstreams/queuingstrategies.js index df114a44cc8adc..8fbc87642ebc0c 100644 --- a/lib/internal/webstreams/queuingstrategies.js +++ b/lib/internal/webstreams/queuingstrategies.js @@ -8,7 +8,6 @@ const { const { codes: { - ERR_INVALID_THIS, ERR_MISSING_OPTION, }, } = require('internal/errors'); @@ -20,21 +19,12 @@ const { const { customInspect, - isBrandCheck, - kType, - kState, } = require('internal/webstreams/util'); const { validateObject, } = require('internal/validators'); -const isByteLengthQueuingStrategy = - isBrandCheck('ByteLengthQueuingStrategy'); - -const isCountQueuingStrategy = - isBrandCheck('CountQueuingStrategy'); - /** * @callback QueuingStrategySize * @param {any} chunk @@ -68,7 +58,8 @@ const getNonWritablePropertyDescriptor = (value) => { * @type {QueuingStrategy} */ class ByteLengthQueuingStrategy { - [kType] = 'ByteLengthQueuingStrategy'; + #state; + #byteSizeFunction = byteSizeFunction; /** * @param {{ @@ -82,7 +73,7 @@ class ByteLengthQueuingStrategy { // The highWaterMark value is not checked until the strategy // is actually used, per the spec. - this[kState] = { + this.#state = { highWaterMark: +init.highWaterMark, }; } @@ -92,22 +83,18 @@ class ByteLengthQueuingStrategy { * @type {number} */ get highWaterMark() { - if (!isByteLengthQueuingStrategy(this)) - throw new ERR_INVALID_THIS('ByteLengthQueuingStrategy'); - return this[kState].highWaterMark; + return this.#state.highWaterMark; } /** * @type {QueuingStrategySize} */ get size() { - if (!isByteLengthQueuingStrategy(this)) - throw new ERR_INVALID_THIS('ByteLengthQueuingStrategy'); - return byteSizeFunction; + return this.#byteSizeFunction; } [kInspect](depth, options) { - return customInspect(depth, options, this[kType], { + return customInspect(depth, options, 'ByteLengthQueuingStrategy', { highWaterMark: this.highWaterMark, }); } @@ -123,7 +110,8 @@ ObjectDefineProperties(ByteLengthQueuingStrategy.prototype, { * @type {QueuingStrategy} */ class CountQueuingStrategy { - [kType] = 'CountQueuingStrategy'; + #state; + #countSizeFunction = countSizeFunction; /** * @param {{ @@ -137,7 +125,7 @@ class CountQueuingStrategy { // The highWaterMark value is not checked until the strategy // is actually used, per the spec. - this[kState] = { + this.#state = { highWaterMark: +init.highWaterMark, }; } @@ -147,22 +135,18 @@ class CountQueuingStrategy { * @type {number} */ get highWaterMark() { - if (!isCountQueuingStrategy(this)) - throw new ERR_INVALID_THIS('CountQueuingStrategy'); - return this[kState].highWaterMark; + return this.#state.highWaterMark; } /** * @type {QueuingStrategySize} */ get size() { - if (!isCountQueuingStrategy(this)) - throw new ERR_INVALID_THIS('CountQueuingStrategy'); - return countSizeFunction; + return this.#countSizeFunction; } [kInspect](depth, options) { - return customInspect(depth, options, this[kType], { + return customInspect(depth, options, 'CountQueuingStrategy', { highWaterMark: this.highWaterMark, }); } diff --git a/test/parallel/test-whatwg-webstreams-coverage.js b/test/parallel/test-whatwg-webstreams-coverage.js index f0036723b05977..4d0ffe818f43f8 100644 --- a/test/parallel/test-whatwg-webstreams-coverage.js +++ b/test/parallel/test-whatwg-webstreams-coverage.js @@ -26,25 +26,29 @@ assert(isPromisePending(new Promise(() => {}))); assert.throws(() => { Reflect.get(ByteLengthQueuingStrategy.prototype, 'highWaterMark', {}); }, { - code: 'ERR_INVALID_THIS' + name: 'TypeError', + message: /Cannot read private member/, }); assert.throws(() => { Reflect.get(ByteLengthQueuingStrategy.prototype, 'size', {}); }, { - code: 'ERR_INVALID_THIS' + name: 'TypeError', + message: /Cannot read private member/, }); assert.throws(() => { Reflect.get(CountQueuingStrategy.prototype, 'highWaterMark', {}); }, { - code: 'ERR_INVALID_THIS' + name: 'TypeError', + message: /Cannot read private member/, }); assert.throws(() => { Reflect.get(CountQueuingStrategy.prototype, 'size', {}); }, { - code: 'ERR_INVALID_THIS' + name: 'TypeError', + message: /Cannot read private member/, }); // Custom Inspect Works