diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 59931d34573e..ee2c3ef3e1b6 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -4126,6 +4126,9 @@ and [`crypto.setEngine()`][] all depend on this functionality from OpenSSL. -Type: Runtime +Type: End-of-Life -Instantiating classes without the `new` qualifier exported by the `node:zlib` module is deprecated. -It is recommended to use the `new` qualifier instead. This applies to all Zlib classes, such as `Deflate`, -`DeflateRaw`, `Gunzip`, `Inflate`, `InflateRaw`, `Unzip`, and `Zlib`. +Instantiating classes without the `new` qualifier exported by the `node:zlib` module is no longer +supported. The `new` qualifier must be used instead. This applies to all Zlib classes, such as +`Deflate`, `DeflateRaw`, `Gunzip`, `Inflate`, `InflateRaw`, `Unzip`, `BrotliCompress`, +`BrotliDecompress`, `ZstdCompress`, and `ZstdDecompress`. ### DEP0185: Instantiating `node:repl` classes without `new` diff --git a/doc/api/http2.md b/doc/api/http2.md index d53548f76730..7cf7a813c443 100644 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -3410,6 +3410,142 @@ client.close(); added: v8.4.0 --> +#### Header name constants + +The `HTTP2_HEADER_*` constants provide names for HTTP/2 pseudo-headers and +known HTTP header names. Using these string constants is optional. For example, +`http2.constants.HTTP2_HEADER_CONTENT_TYPE` is equal to `'content-type'`. +For APIs that accept regular header names, +`http2.constants.HTTP2_HEADER_CONTENT_TYPE`, `'content-type'`, and +`'Content-Type'` have the same effect; Node.js serializes the name in +lower-case. + +Regular header constants can be used with the compatibility API wherever the +corresponding literal header name is accepted. In compatibility API request +handlers, prefer `request.method`, `request.authority`, `request.scheme`, and +`request.url` for the corresponding pseudo-headers. Other incoming +pseudo-headers remain available through `request.headers`. Set response status +through `response.statusCode` or the `statusCode` argument to +`response.writeHead()`. Passing `HTTP2_HEADER_STATUS` (`':status'`) to +`response.setHeader()` or in `response.writeHead()`'s headers object throws +`ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED`. `HTTP2_HEADER_PROTOCOL` is a request +pseudo-header and cannot be sent in a response. + +Incoming header object keys are lower-case, so use a constant or a lower-case +literal when accessing them as object properties. Using a constant does not +change header validation, and the availability of a constant does not imply +that the header is valid in every HTTP/2 context. See [HTTP/2 Headers Object][] +and [Invalid character handling in header names and values][] for details about +header casing and validation. + +##### Pseudo-header constants + +`HTTP2_HEADER_METHOD`, `HTTP2_HEADER_AUTHORITY`, `HTTP2_HEADER_SCHEME`, and +`HTTP2_HEADER_PATH` identify request pseudo-headers. `HTTP2_HEADER_STATUS` +identifies the response pseudo-header. `HTTP2_HEADER_PROTOCOL` identifies the +extended `CONNECT` request pseudo-header. Pseudo-headers are not permitted in +trailers. + +| Constant | Value | +| ---------------------------------------- | -------------- | +| `http2.constants.HTTP2_HEADER_STATUS` | `':status'` | +| `http2.constants.HTTP2_HEADER_METHOD` | `':method'` | +| `http2.constants.HTTP2_HEADER_AUTHORITY` | `':authority'` | +| `http2.constants.HTTP2_HEADER_SCHEME` | `':scheme'` | +| `http2.constants.HTTP2_HEADER_PATH` | `':path'` | +| `http2.constants.HTTP2_HEADER_PROTOCOL` | `':protocol'` | + +##### Regular header constants + +The `HTTP2_HEADER_CONNECTION`, `HTTP2_HEADER_UPGRADE`, +`HTTP2_HEADER_HTTP2_SETTINGS`, `HTTP2_HEADER_KEEP_ALIVE`, +`HTTP2_HEADER_PROXY_CONNECTION`, and `HTTP2_HEADER_TRANSFER_ENCODING` +constants identify connection-specific headers that HTTP/2 does not permit. +`HTTP2_HEADER_TE` is permitted only when its value is `'trailers'`. + +| Constant | Value | +| --------------------------------------------------------------- | ------------------------------------ | +| `http2.constants.HTTP2_HEADER_ACCEPT_ENCODING` | `'accept-encoding'` | +| `http2.constants.HTTP2_HEADER_ACCEPT_LANGUAGE` | `'accept-language'` | +| `http2.constants.HTTP2_HEADER_ACCEPT_RANGES` | `'accept-ranges'` | +| `http2.constants.HTTP2_HEADER_ACCEPT` | `'accept'` | +| `http2.constants.HTTP2_HEADER_ACCESS_CONTROL_ALLOW_CREDENTIALS` | `'access-control-allow-credentials'` | +| `http2.constants.HTTP2_HEADER_ACCESS_CONTROL_ALLOW_HEADERS` | `'access-control-allow-headers'` | +| `http2.constants.HTTP2_HEADER_ACCESS_CONTROL_ALLOW_METHODS` | `'access-control-allow-methods'` | +| `http2.constants.HTTP2_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN` | `'access-control-allow-origin'` | +| `http2.constants.HTTP2_HEADER_ACCESS_CONTROL_EXPOSE_HEADERS` | `'access-control-expose-headers'` | +| `http2.constants.HTTP2_HEADER_ACCESS_CONTROL_REQUEST_HEADERS` | `'access-control-request-headers'` | +| `http2.constants.HTTP2_HEADER_ACCESS_CONTROL_REQUEST_METHOD` | `'access-control-request-method'` | +| `http2.constants.HTTP2_HEADER_AGE` | `'age'` | +| `http2.constants.HTTP2_HEADER_AUTHORIZATION` | `'authorization'` | +| `http2.constants.HTTP2_HEADER_CACHE_CONTROL` | `'cache-control'` | +| `http2.constants.HTTP2_HEADER_CONNECTION` | `'connection'` | +| `http2.constants.HTTP2_HEADER_CONTENT_DISPOSITION` | `'content-disposition'` | +| `http2.constants.HTTP2_HEADER_CONTENT_ENCODING` | `'content-encoding'` | +| `http2.constants.HTTP2_HEADER_CONTENT_LENGTH` | `'content-length'` | +| `http2.constants.HTTP2_HEADER_CONTENT_TYPE` | `'content-type'` | +| `http2.constants.HTTP2_HEADER_COOKIE` | `'cookie'` | +| `http2.constants.HTTP2_HEADER_DATE` | `'date'` | +| `http2.constants.HTTP2_HEADER_ETAG` | `'etag'` | +| `http2.constants.HTTP2_HEADER_FORWARDED` | `'forwarded'` | +| `http2.constants.HTTP2_HEADER_HOST` | `'host'` | +| `http2.constants.HTTP2_HEADER_IF_MODIFIED_SINCE` | `'if-modified-since'` | +| `http2.constants.HTTP2_HEADER_IF_NONE_MATCH` | `'if-none-match'` | +| `http2.constants.HTTP2_HEADER_IF_RANGE` | `'if-range'` | +| `http2.constants.HTTP2_HEADER_LAST_MODIFIED` | `'last-modified'` | +| `http2.constants.HTTP2_HEADER_LINK` | `'link'` | +| `http2.constants.HTTP2_HEADER_LOCATION` | `'location'` | +| `http2.constants.HTTP2_HEADER_RANGE` | `'range'` | +| `http2.constants.HTTP2_HEADER_REFERER` | `'referer'` | +| `http2.constants.HTTP2_HEADER_SERVER` | `'server'` | +| `http2.constants.HTTP2_HEADER_SET_COOKIE` | `'set-cookie'` | +| `http2.constants.HTTP2_HEADER_STRICT_TRANSPORT_SECURITY` | `'strict-transport-security'` | +| `http2.constants.HTTP2_HEADER_TRANSFER_ENCODING` | `'transfer-encoding'` | +| `http2.constants.HTTP2_HEADER_TE` | `'te'` | +| `http2.constants.HTTP2_HEADER_UPGRADE_INSECURE_REQUESTS` | `'upgrade-insecure-requests'` | +| `http2.constants.HTTP2_HEADER_UPGRADE` | `'upgrade'` | +| `http2.constants.HTTP2_HEADER_USER_AGENT` | `'user-agent'` | +| `http2.constants.HTTP2_HEADER_VARY` | `'vary'` | +| `http2.constants.HTTP2_HEADER_X_CONTENT_TYPE_OPTIONS` | `'x-content-type-options'` | +| `http2.constants.HTTP2_HEADER_X_FRAME_OPTIONS` | `'x-frame-options'` | +| `http2.constants.HTTP2_HEADER_KEEP_ALIVE` | `'keep-alive'` | +| `http2.constants.HTTP2_HEADER_PROXY_CONNECTION` | `'proxy-connection'` | +| `http2.constants.HTTP2_HEADER_X_XSS_PROTECTION` | `'x-xss-protection'` | +| `http2.constants.HTTP2_HEADER_ALT_SVC` | `'alt-svc'` | +| `http2.constants.HTTP2_HEADER_CONTENT_SECURITY_POLICY` | `'content-security-policy'` | +| `http2.constants.HTTP2_HEADER_EARLY_DATA` | `'early-data'` | +| `http2.constants.HTTP2_HEADER_EXPECT_CT` | `'expect-ct'` | +| `http2.constants.HTTP2_HEADER_ORIGIN` | `'origin'` | +| `http2.constants.HTTP2_HEADER_PURPOSE` | `'purpose'` | +| `http2.constants.HTTP2_HEADER_TIMING_ALLOW_ORIGIN` | `'timing-allow-origin'` | +| `http2.constants.HTTP2_HEADER_X_FORWARDED_FOR` | `'x-forwarded-for'` | +| `http2.constants.HTTP2_HEADER_PRIORITY` | `'priority'` | +| `http2.constants.HTTP2_HEADER_ACCEPT_CHARSET` | `'accept-charset'` | +| `http2.constants.HTTP2_HEADER_ACCESS_CONTROL_MAX_AGE` | `'access-control-max-age'` | +| `http2.constants.HTTP2_HEADER_ALLOW` | `'allow'` | +| `http2.constants.HTTP2_HEADER_CONTENT_LANGUAGE` | `'content-language'` | +| `http2.constants.HTTP2_HEADER_CONTENT_LOCATION` | `'content-location'` | +| `http2.constants.HTTP2_HEADER_CONTENT_MD5` | `'content-md5'` | +| `http2.constants.HTTP2_HEADER_CONTENT_RANGE` | `'content-range'` | +| `http2.constants.HTTP2_HEADER_DNT` | `'dnt'` | +| `http2.constants.HTTP2_HEADER_EXPECT` | `'expect'` | +| `http2.constants.HTTP2_HEADER_EXPIRES` | `'expires'` | +| `http2.constants.HTTP2_HEADER_FROM` | `'from'` | +| `http2.constants.HTTP2_HEADER_IF_MATCH` | `'if-match'` | +| `http2.constants.HTTP2_HEADER_IF_UNMODIFIED_SINCE` | `'if-unmodified-since'` | +| `http2.constants.HTTP2_HEADER_MAX_FORWARDS` | `'max-forwards'` | +| `http2.constants.HTTP2_HEADER_PREFER` | `'prefer'` | +| `http2.constants.HTTP2_HEADER_PROXY_AUTHENTICATE` | `'proxy-authenticate'` | +| `http2.constants.HTTP2_HEADER_PROXY_AUTHORIZATION` | `'proxy-authorization'` | +| `http2.constants.HTTP2_HEADER_REFRESH` | `'refresh'` | +| `http2.constants.HTTP2_HEADER_RETRY_AFTER` | `'retry-after'` | +| `http2.constants.HTTP2_HEADER_TRAILER` | `'trailer'` | +| `http2.constants.HTTP2_HEADER_TK` | `'tk'` | +| `http2.constants.HTTP2_HEADER_VIA` | `'via'` | +| `http2.constants.HTTP2_HEADER_WARNING` | `'warning'` | +| `http2.constants.HTTP2_HEADER_WWW_AUTHENTICATE` | `'www-authenticate'` | +| `http2.constants.HTTP2_HEADER_HTTP2_SETTINGS` | `'http2-settings'` | + #### Error codes for `RST_STREAM` and `GOAWAY` | Value | Name | Constant | @@ -3976,9 +4112,10 @@ API: ```mjs import { createServer } from 'node:http2'; const server = createServer((req, res) => { - res.setHeader('Content-Type', 'text/html'); - res.setHeader('X-Foo', 'bar'); - res.writeHead(200, { 'Content-Type': 'text/plain; charset=utf-8' }); + res.writeHead(200, { + 'Content-Type': 'text/plain; charset=utf-8', + 'X-Foo': 'bar', + }); res.end('ok'); }); ``` @@ -3986,9 +4123,10 @@ const server = createServer((req, res) => { ```cjs const http2 = require('node:http2'); const server = http2.createServer((req, res) => { - res.setHeader('Content-Type', 'text/html'); - res.setHeader('X-Foo', 'bar'); - res.writeHead(200, { 'Content-Type': 'text/plain; charset=utf-8' }); + res.writeHead(200, { + 'Content-Type': 'text/plain; charset=utf-8', + 'X-Foo': 'bar', + }); res.end('ok'); }); ``` @@ -5087,6 +5225,7 @@ you need to implement any fall-back behavior yourself. [HTTP/2 Settings Object]: #settings-object [HTTP/2 Unencrypted]: https://http2.github.io/faq/#does-http2-require-encryption [HTTPS]: https.md +[Invalid character handling in header names and values]: #invalid-character-handling-in-header-names-and-values [Performance Observer]: perf_hooks.md [RFC 7838]: https://tools.ietf.org/html/rfc7838 [RFC 8336]: https://tools.ietf.org/html/rfc8336 diff --git a/doc/contributing/strategic-initiatives.md b/doc/contributing/strategic-initiatives.md index c56876238ef0..ac154c0e2bb7 100644 --- a/doc/contributing/strategic-initiatives.md +++ b/doc/contributing/strategic-initiatives.md @@ -8,7 +8,8 @@ agenda to ensure they are active and have the support they need. | Initiative | Champion | Links | | ---------------------- | -------------------------------- | --------------------------------------------- | -| QUIC / HTTP3 | [James M Snell][jasnell] | | +| QUIC / HTTP3 | [James M Snell][jasnell] | | +| Unified HTTP API | [James M Snell][jasnell] | | | Shadow Realm | [Chengzhong Wu][legendecas] | | | V8 Currency | [Michaël Zasso][targos] | | | Next-10 | [Jacob Smith][JakobJingleheimer] | | diff --git a/lib/internal/zip/archive.js b/lib/internal/zip/archive.js index 7693ce1297f2..b2aef250d5ad 100644 --- a/lib/internal/zip/archive.js +++ b/lib/internal/zip/archive.js @@ -133,12 +133,15 @@ async function* fileEntries(files, followSymlinks) { // would let a symlink swapped in after the classification above redirect // the read - a TOCTOU that defeats followSymlinks:false. O_NOFOLLOW makes // a final-component symlink fail the open outright when not following; - // the fstat then confirms a regular file, never a FIFO, device, or - // socket (which as a stream source could block or emit unbounded data). + // O_NONBLOCK keeps the open itself from blocking on a FIFO or a slow + // device (it has no effect on a regular file's reads), so the fstat can + // then reject anything that is not a regular file - a FIFO, device, or + // socket, which as a stream source could block or emit unbounded data. // Metadata comes from that same fstat so it describes the inode we read. - const flags = followSymlinks ? + const flags = (followSymlinks ? fs.constants.O_RDONLY : - fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0); + fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0)) | + (fs.constants.O_NONBLOCK || 0); const handle = await fs.promises.open(sourcePath, flags); let ownsHandle = true; try { diff --git a/lib/internal/zip/entry.js b/lib/internal/zip/entry.js index c9fb1cac62cd..566f5ba58a51 100644 --- a/lib/internal/zip/entry.js +++ b/lib/internal/zip/entry.js @@ -76,6 +76,7 @@ const { const { CentralFileHeader, LocalFileHeader, + assertConsistentLocalHeader, findArchiveEnd, } = require('internal/zip/headers'); const { @@ -174,6 +175,17 @@ function createEntryMeta(filename, options) { }; } +// Release one in-flight read on a shared ZipFile descriptor handle. When the +// last read settles, wake a close() that is waiting to release the fd (see +// ZipFile.close() in file.js), so a read never lands on a closed/reused fd. +function endHandleRead(handle) { + if (--handle.reads === 0 && handle.drain !== null) { + const drain = handle.drain; + handle.drain = null; + drain(); + } +} + /** * A single file or directory inside a ZIP archive: reading, writing, and * (de)serializing one archive member. @@ -399,6 +411,9 @@ class ZipEntry { await readFdFully(fd, full.subarray(30), this.#localOffset + 30); } this.#local = new LocalFileHeader(full, 0); + if (this.#central !== null) { + assertConsistentLocalHeader(this.#local, this.#central); + } this.#contentOffset = this.#localOffset + length; return this.#local; } @@ -419,6 +434,9 @@ class ZipEntry { readFdFullySync(fd, full.subarray(30), this.#localOffset + 30); } this.#local = new LocalFileHeader(full, 0); + if (this.#central !== null) { + assertConsistentLocalHeader(this.#local, this.#central); + } this.#contentOffset = this.#localOffset + length; return this.#local; } @@ -483,17 +501,30 @@ class ZipEntry { // The entry's raw compressed bytes as a bounded-memory chunk stream, read // straight from disk (file-backed entries only). Nothing is retained. async *#rawChunks() { - this.#liveDescriptor(); - let pos = await this.#resolveContentOffset(); - let remaining = this.compressedSize; - while (remaining > 0) { - const take = MathMin(READ_CHUNK_SIZE, remaining); - const chunk = Buffer.allocUnsafe(take); - // Re-check per chunk: the ZipFile may be closed mid-stream. - await readFdFully(this.#liveDescriptor(), chunk, pos); - pos += take; - remaining -= take; - yield chunk; + // Count the whole stream as one in-flight read so a concurrent close() + // waits for it (the finally runs when the consumer stops iterating); a + // stream begun after close() was requested is rejected up front. + const handle = this.#fd; + if (handle.closing) { + throw new ERR_INVALID_STATE( + 'cannot read a ZipEntry after its backing ZipFile has been closed'); + } + handle.reads++; + try { + this.#liveDescriptor(); + let pos = await this.#resolveContentOffset(); + let remaining = this.compressedSize; + while (remaining > 0) { + const take = MathMin(READ_CHUNK_SIZE, remaining); + const chunk = Buffer.allocUnsafe(take); + // Re-check per chunk: the ZipFile may be closed mid-stream. + await readFdFully(this.#liveDescriptor(), chunk, pos); + pos += take; + remaining -= take; + yield chunk; + } + } finally { + endHandleRead(handle); } } // Sync counterpart of #rawChunks(). @@ -526,18 +557,33 @@ class ZipEntry { `entry ${JSONStringify(this.name)} declares ${declared} bytes, ` + `exceeding the ${maxSize} byte limit`); } - const compressed = await this.#compressedBytes(); - const data = await decodeMemberAsync(compressed, { - name: this.name, - flags: this.flags, - method: this.method, - crc32: this.crc32, - uncompressedSize: declared, - }, { verify: options?.verify, maxSize }); - // `data === compressed` only on the store path; copy the in-memory case - // (the entry's retained buffer, see #compressedBytes()) so the result is - // caller-owned on every path. - return data === compressed && this.#fd === null ? Buffer.from(data) : data; + // Count this read on the shared handle so a concurrent close() waits for it + // rather than releasing the fd mid-read; a read begun after close() was + // requested is rejected up front (in-memory entries have no fd). + const handle = this.#fd; + if (handle !== null) { + if (handle.closing) { + throw new ERR_INVALID_STATE( + 'cannot read a ZipEntry after its backing ZipFile has been closed'); + } + handle.reads++; + } + try { + const compressed = await this.#compressedBytes(); + const data = await decodeMemberAsync(compressed, { + name: this.name, + flags: this.flags, + method: this.method, + crc32: this.crc32, + uncompressedSize: declared, + }, { verify: options?.verify, maxSize }); + // `data === compressed` only on the store path; copy the in-memory case + // (the entry's retained buffer, see #compressedBytes()) so the result is + // caller-owned on every path. + return data === compressed && this.#fd === null ? Buffer.from(data) : data; + } finally { + if (handle !== null) endHandleRead(handle); + } } /** @@ -930,6 +976,7 @@ function* readArchiveEntries(buf, end) { } const localOffset = central.localFileHeaderOffset + end.prefix; const local = new LocalFileHeader(buf, localOffset); + assertConsistentLocalHeader(local, central); const dataStart = localOffset + local.byteLength; const length = central.compressedSize; validateArchiveRange(buf, dataStart, length, 'entry data'); diff --git a/lib/internal/zip/file.js b/lib/internal/zip/file.js index b43c091e8630..bae71ed485d5 100644 --- a/lib/internal/zip/file.js +++ b/lib/internal/zip/file.js @@ -19,6 +19,7 @@ const { MapPrototypeKeys, MapPrototypeSet, MathMin, + Promise, PromisePrototypeThen, PromiseResolve, SymbolAsyncDispose, @@ -218,12 +219,14 @@ function checkMemberOverlap(members, centralDirectoryOffset) { * corrupt the archive. */ class ZipFile { - // Shared descriptor handle ({ fd, closed }) handed to every ZipEntry this - // archive produces, so close() can invalidate them all at once. `#closing` - // is set synchronously the moment close()/closeSync() is called, gating any - // further public call; `handle.closed` is set once the fd is actually gone, - // gating reads through already-handed-out entries. Neither read ever falls - // through to a bare (possibly OS-reused) descriptor number. + // Shared descriptor handle ({ fd, closed, reads, drain }) handed to every + // ZipEntry this archive produces, so close() can invalidate them all at once. + // `#closing` is set synchronously the moment close()/closeSync() is called, + // gating any further public call; `handle.closed` then stops new reads, and + // `handle.reads` counts in-flight reads through already-handed-out entries so + // close() can wait for them to finish before releasing the fd. Neither a new + // nor an in-flight read ever falls through to a closed (possibly OS-reused) + // descriptor number. #handle; #closing = false; #closePromise = null; @@ -240,7 +243,7 @@ class ZipFile { * @private */ constructor(fd, centralHeaders, prefix, centralDirectoryOffset, comment, writable) { - this.#handle = { fd, closed: false }; + this.#handle = { fd, closed: false, closing: false, reads: 0, drain: null }; this.#writable = writable; this.#comment = comment; this.#centralDirectoryOffset = centralDirectoryOffset; @@ -384,10 +387,11 @@ class ZipFile { return this.#enqueue(async () => this.#doAdd(await ZipEntry.create(filename, data, options))); } - // Append the entry's bytes where the central directory currently starts, - // then rewrite the directory to include it. On write failure, restore the - // original directory (the partial write may have clobbered it) and rethrow; - // on success, promote a spent stream entry to its on-disk copy. + // Append the entry's bytes where the central directory currently starts, then + // rewrite the directory to include it. If the member write or the directory + // rewrite fails, restore the original directory and drop the half-adopted + // entry so the archive is left exactly as it was, then rethrow; on success, + // promote a spent stream entry to its on-disk copy. async #doAdd(entry) { const localOffset = this.#centralDirectoryOffset; let written = 0; @@ -410,9 +414,30 @@ class ZipFile { } throw err; } + const previousEntry = MapPrototypeGet(this.#entries, entry.name); this.#centralDirectoryOffset = localOffset + written; MapPrototypeSet(this.#entries, entry.name, { central: null, entry, localOffset }); - await this.#rewriteCentralDirectory(); + try { + await this.#rewriteCentralDirectory(); + } catch (err) { + // The directory rewrite failed after the member bytes landed. Undo the + // in-memory adoption and rewrite the original directory back at its old + // offset (where the failed member bytes started), leaving the archive and + // this handle exactly as before the call. + this.#centralDirectoryOffset = localOffset; + if (previousEntry === undefined) { + MapPrototypeDelete(this.#entries, entry.name); + } else { + MapPrototypeSet(this.#entries, entry.name, previousEntry); + } + try { + await this.#rewriteCentralDirectory(); + } catch { + // Restoring failed too (the device is likely full or gone); the + // original error is the actionable one. + } + throw err; + } // The entry now has a stable home in this archive; if it was a spent // streaming entry, rebind it to that on-disk copy so it stays readable. entry[kPromote](this.#handle, localOffset); @@ -450,9 +475,27 @@ class ZipFile { } throw err; } + const previousEntry = MapPrototypeGet(this.#entries, entry.name); this.#centralDirectoryOffset = localOffset + written; MapPrototypeSet(this.#entries, entry.name, { central: null, entry, localOffset }); - this.#rewriteCentralDirectorySync(); + try { + this.#rewriteCentralDirectorySync(); + } catch (err) { + // See #doAdd(): undo the in-memory adoption and restore the original + // directory so a failed rewrite leaves the archive exactly as it was. + this.#centralDirectoryOffset = localOffset; + if (previousEntry === undefined) { + MapPrototypeDelete(this.#entries, entry.name); + } else { + MapPrototypeSet(this.#entries, entry.name, previousEntry); + } + try { + this.#rewriteCentralDirectorySync(); + } catch { + // Restoring failed too; the original error is the actionable one. + } + throw err; + } entry[kPromote](this.#handle, localOffset); return entry; } @@ -655,7 +698,16 @@ class ZipFile { close() { if (this.#closing) return this.#closePromise ?? PromiseResolve(); this.#closing = true; + // Block *new* reads immediately; reads already in flight keep the still-open + // fd until they finish (handle.closed, set below, is what stops mid-read). + this.#handle.closing = true; this.#closePromise = this.#enqueue(async () => { + // Let reads already in flight through handed-out entries finish on the + // open fd before releasing it; endHandleRead() (entry.js) resolves this + // once the last one settles. + if (this.#handle.reads > 0) { + await new Promise((resolve) => { this.#handle.drain = resolve; }); + } this.#handle.closed = true; MapPrototypeClear(this.#entries); await fsCloseAsync(this.#handle.fd); @@ -669,7 +721,15 @@ class ZipFile { closeSync() { if (this.#closing) return; this.#assertNotBusy(); + // A synchronous close cannot wait for an async read the way close() does, + // so refuse rather than pull the fd out from under one still in flight. + if (this.#handle.reads > 0) { + throw new ERR_INVALID_STATE( + 'cannot synchronously close a ZipFile while an asynchronous read is ' + + 'still in flight'); + } this.#closing = true; + this.#handle.closing = true; this.#handle.closed = true; MapPrototypeClear(this.#entries); fs.closeSync(this.#handle.fd); diff --git a/lib/internal/zip/headers.js b/lib/internal/zip/headers.js index f1f028cb511b..bf6f7901516a 100644 --- a/lib/internal/zip/headers.js +++ b/lib/internal/zip/headers.js @@ -25,6 +25,8 @@ const { SIG_ZIP64_EOCD_LOCATOR, SIG_EOCD, MADE_BY_UNIX, + FLAG_ENCRYPTED, + FLAG_DATA_DESCRIPTOR, SENTINEL16, SENTINEL32, TAIL_LENGTH, @@ -267,6 +269,7 @@ class CentralFileHeader { class LocalFileHeader { #buffer; #offset; + #zip64 = null; constructor(buffer, offset = 0) { validateArchiveRange(buffer, offset, 30, 'local file header'); if (buffer.readUInt32LE(offset) !== SIG_LOCAL_FILE_HEADER) { @@ -280,9 +283,31 @@ class LocalFileHeader { } get byteLength() { return 30 + this.fileNameLength + this.extraFieldLength; } get flags() { return this.#buffer.readUInt16LE(this.#offset + 6); } - // Spec field (sec. 4.4.5); the central directory's method is authoritative, - // so the local copy is not consumed today. - // get compressionMethod() { return this.#buffer.readUInt16LE(this.#offset + 8); } + // The central directory is authoritative, but these local fields are read to + // reject an archive whose local header contradicts it (assertConsistentLocalHeader). + get compressionMethod() { return this.#buffer.readUInt16LE(this.#offset + 8); } + get crc32() { return this.#buffer.readUInt32LE(this.#offset + 14); } + // The Zip64 extra supplies the true 64-bit sizes for whichever classic size + // field holds an overflow sentinel; a local header has no offset/disk field. + #resolveZip64() { + if (this.#zip64 === null) { + this.#zip64 = parseZip64Extra(this.extraField, { + uncompressedSize: this.#buffer.readUInt32LE(this.#offset + 22) === SENTINEL32, + compressedSize: this.#buffer.readUInt32LE(this.#offset + 18) === SENTINEL32, + localFileHeaderOffset: false, + diskNumber: false, + }); + } + return this.#zip64; + } + get compressedSize() { + const value = this.#buffer.readUInt32LE(this.#offset + 18); + return value === SENTINEL32 ? this.#resolveZip64().compressedSize : value; + } + get uncompressedSize() { + const value = this.#buffer.readUInt32LE(this.#offset + 22); + return value === SENTINEL32 ? this.#resolveZip64().uncompressedSize : value; + } get fileNameLength() { return this.#buffer.readUInt16LE(this.#offset + 26); } get extraFieldLength() { return this.#buffer.readUInt16LE(this.#offset + 28); } get extraField() { @@ -311,6 +336,35 @@ class LocalFileHeader { } } +// Reject an archive whose local file header contradicts the central directory +// on a field that decides which bytes a member yields. This reader treats the +// central directory as authoritative, but a divergent local header lets a +// local-header-based extractor (Info-ZIP unzip, and others) read a different +// member, method, or size from the same archive - a parser-confusion split that +// defeats inspect-then-consume pipelines. Reject rather than silently diverge. +function assertConsistentLocalHeader(local, central) { + if (local.compressionMethod !== central.compressionMethod) { + throw new ERR_ZIP_INVALID_ARCHIVE( + 'local and central compression methods disagree'); + } + if ((local.flags & FLAG_ENCRYPTED) !== (central.flags & FLAG_ENCRYPTED)) { + throw new ERR_ZIP_INVALID_ARCHIVE( + 'local and central encryption flags disagree'); + } + // A streamed entry (data-descriptor bit) writes zero crc and sizes in the + // local header, deferring the true values to the central directory; only then + // is a difference on those three fields expected. + const deferred = (local.flags & FLAG_DATA_DESCRIPTOR) !== 0 && + local.crc32 === 0 && local.compressedSize === 0 && local.uncompressedSize === 0; + if (!deferred && + (local.crc32 !== central.crc32 || + local.compressedSize !== central.compressedSize || + local.uncompressedSize !== central.uncompressedSize)) { + throw new ERR_ZIP_INVALID_ARCHIVE( + 'local and central header crc-32 or sizes disagree'); + } +} + // Returns whether an EOCD-looking record could describe an archive this // implementation supports. This is deliberately only a cheap preflight: the // selected record still receives the complete Zip64 and central-directory @@ -563,6 +617,7 @@ function readCentralDirectory(buffer, count) { module.exports = { CentralFileHeader, LocalFileHeader, + assertConsistentLocalHeader, findArchiveEnd, readCentralDirectory, }; diff --git a/lib/zlib.js b/lib/zlib.js index 29c61b51fc6f..3e6986bf8127 100644 --- a/lib/zlib.js +++ b/lib/zlib.js @@ -30,7 +30,6 @@ const { ObjectEntries, ObjectFreeze, ObjectKeys, - ObjectSetPrototypeOf, ReflectApply, Symbol, SymbolHasInstance, @@ -52,7 +51,6 @@ const { const { Transform, finished } = require('stream'); const { assignFunctionName, - deprecateInstantiation, emitExperimentalWarning, } = require('internal/util'); const { @@ -219,124 +217,6 @@ const FLUSH_BOUND_IDX_NORMAL = 0; const FLUSH_BOUND_IDX_BROTLI = 1; const FLUSH_BOUND_IDX_ZSTD = 2; -/** - * The base class for all Zlib-style streams. - * @class - */ -function ZlibBase(opts, mode, handle, { flush, finishFlush, fullFlush }) { - let chunkSize = Z_DEFAULT_CHUNK; - let maxOutputLength = kMaxLength; - // The ZlibBase class is not exported to user land, the mode should only be - // passed in by us. - assert(typeof mode === 'number'); - assert(mode >= DEFLATE && mode <= ZSTD_DECOMPRESS); - - let flushBoundIdx; - if (mode === BROTLI_ENCODE || mode === BROTLI_DECODE) { - flushBoundIdx = FLUSH_BOUND_IDX_BROTLI; - } else if (mode === ZSTD_COMPRESS || mode === ZSTD_DECOMPRESS) { - flushBoundIdx = FLUSH_BOUND_IDX_ZSTD; - } else { - flushBoundIdx = FLUSH_BOUND_IDX_NORMAL; - } - - if (opts) { - chunkSize = opts.chunkSize; - if (!validateFiniteNumber(chunkSize, 'options.chunkSize')) { - chunkSize = Z_DEFAULT_CHUNK; - } else if (chunkSize < Z_MIN_CHUNK) { - throw new ERR_OUT_OF_RANGE('options.chunkSize', - `>= ${Z_MIN_CHUNK}`, chunkSize); - } - - flush = checkRangesOrGetDefault( - opts.flush, 'options.flush', - FLUSH_BOUND[flushBoundIdx][0], FLUSH_BOUND[flushBoundIdx][1], flush); - - finishFlush = checkRangesOrGetDefault( - opts.finishFlush, 'options.finishFlush', - FLUSH_BOUND[flushBoundIdx][0], FLUSH_BOUND[flushBoundIdx][1], - finishFlush); - - maxOutputLength = checkRangesOrGetDefault( - opts.maxOutputLength, 'options.maxOutputLength', - 1, kMaxLength, kMaxLength); - - if (opts.rejectGarbageAfterEnd !== undefined) { - validateBoolean( - opts.rejectGarbageAfterEnd, - 'options.rejectGarbageAfterEnd', - ); - } - - if (opts.encoding || opts.objectMode || opts.writableObjectMode) { - opts = { ...opts }; - opts.encoding = null; - opts.objectMode = false; - opts.writableObjectMode = false; - } - } - - Transform.call(this, { autoDestroy: true, ...opts }); - this[kError] = null; - this.bytesWritten = 0; - this._handle = handle; - handle[owner_symbol] = this; - // Used by processCallback() and zlibOnError() - handle.onerror = zlibOnError; - this._outBuffer = Buffer.allocUnsafe(chunkSize); - this._outOffset = 0; - - this._chunkSize = chunkSize; - this._defaultFlushFlag = flush; - this._finishFlushFlag = finishFlush; - this._defaultFullFlushFlag = fullFlush; - this._flushBoundIdx = flushBoundIdx; - this._info = opts?.info; - this._maxOutputLength = maxOutputLength; - - this._rejectGarbageAfterEnd = opts?.rejectGarbageAfterEnd === true; -} -ObjectSetPrototypeOf(ZlibBase.prototype, Transform.prototype); -ObjectSetPrototypeOf(ZlibBase, Transform); - -ObjectDefineProperty(ZlibBase.prototype, '_closed', { - __proto__: null, - configurable: true, - enumerable: true, - get() { - return !this._handle; - }, -}); - -/** - * @this {ZlibBase} - * @returns {void} - */ -ZlibBase.prototype.reset = function() { - assert(this._handle, 'zlib binding closed'); - return this._handle.reset(); -}; - -/** - * This is the _flush function called by the transform class, - * internally, when the last chunk has been written. - * @returns {void} - * @this {ZlibBase} - */ -ZlibBase.prototype._flush = function(callback) { - this._transform(new FastBuffer(), '', callback); -}; - -/** - * Force Transform compat behavior. - * @returns {void} - * @this {ZlibBase} - */ -ZlibBase.prototype._final = function(callback) { - callback(); -}; - // If a flush is scheduled while another flush is still pending, a way to figure // out which one is the "stronger" flush is needed. // This is currently only used to figure out which flush flag to use for the @@ -367,64 +247,169 @@ const kFlushBuffers = []; } } -ZlibBase.prototype.flush = function(kind, callback) { - if (typeof kind === 'function' || (kind === undefined && !callback)) { - callback = kind; - kind = this._defaultFullFlushFlag; +/** + * The base class for all Zlib-style streams. + */ +class ZlibBase extends Transform { + constructor(opts, mode, handle, { flush, finishFlush, fullFlush }) { + let chunkSize = Z_DEFAULT_CHUNK; + let maxOutputLength = kMaxLength; + // The ZlibBase class is not exported to user land, the mode should only be + // passed in by us. + assert(typeof mode === 'number'); + assert(mode >= DEFLATE && mode <= ZSTD_DECOMPRESS); + + let flushBoundIdx; + if (mode === BROTLI_ENCODE || mode === BROTLI_DECODE) { + flushBoundIdx = FLUSH_BOUND_IDX_BROTLI; + } else if (mode === ZSTD_COMPRESS || mode === ZSTD_DECOMPRESS) { + flushBoundIdx = FLUSH_BOUND_IDX_ZSTD; + } else { + flushBoundIdx = FLUSH_BOUND_IDX_NORMAL; + } + + if (opts) { + chunkSize = opts.chunkSize; + if (!validateFiniteNumber(chunkSize, 'options.chunkSize')) { + chunkSize = Z_DEFAULT_CHUNK; + } else if (chunkSize < Z_MIN_CHUNK) { + throw new ERR_OUT_OF_RANGE('options.chunkSize', + `>= ${Z_MIN_CHUNK}`, chunkSize); + } + + flush = checkRangesOrGetDefault( + opts.flush, 'options.flush', + FLUSH_BOUND[flushBoundIdx][0], FLUSH_BOUND[flushBoundIdx][1], flush); + + finishFlush = checkRangesOrGetDefault( + opts.finishFlush, 'options.finishFlush', + FLUSH_BOUND[flushBoundIdx][0], FLUSH_BOUND[flushBoundIdx][1], + finishFlush); + + maxOutputLength = checkRangesOrGetDefault( + opts.maxOutputLength, 'options.maxOutputLength', + 1, kMaxLength, kMaxLength); + + if (opts.rejectGarbageAfterEnd !== undefined) { + validateBoolean( + opts.rejectGarbageAfterEnd, + 'options.rejectGarbageAfterEnd', + ); + } + + if (opts.encoding || opts.objectMode || opts.writableObjectMode) { + opts = { ...opts }; + opts.encoding = null; + opts.objectMode = false; + opts.writableObjectMode = false; + } + } + + super({ autoDestroy: true, ...opts }); + this[kError] = null; + this.bytesWritten = 0; + this._handle = handle; + handle[owner_symbol] = this; + // Used by processCallback() and zlibOnError() + handle.onerror = zlibOnError; + this._outBuffer = Buffer.allocUnsafe(chunkSize); + this._outOffset = 0; + + this._chunkSize = chunkSize; + this._defaultFlushFlag = flush; + this._finishFlushFlag = finishFlush; + this._defaultFullFlushFlag = fullFlush; + this._flushBoundIdx = flushBoundIdx; + this._info = opts?.info; + this._maxOutputLength = maxOutputLength; + + this._rejectGarbageAfterEnd = opts?.rejectGarbageAfterEnd === true; + } + + get _closed() { + return !this._handle; } - kind = checkRangesOrGetDefault( - kind, 'kind', - FLUSH_BOUND[this._flushBoundIdx][0], FLUSH_BOUND[this._flushBoundIdx][1], - this._defaultFullFlushFlag); + reset() { + assert(this._handle, 'zlib binding closed'); + return this._handle.reset(); + } - if (this.writableFinished) { - if (callback) - process.nextTick(callback); - } else if (this.writableEnded) { - if (callback) - this.once('end', callback); - } else { - this.write(kFlushBuffers[kind], '', callback); + /** + * This is the _flush function called by the transform class, + * internally, when the last chunk has been written. + * @returns {void} + */ + _flush(callback) { + this._transform(new FastBuffer(), '', callback); } -}; -/** - * @this {import('stream').Transform} - * @param {(err?: Error) => any} [callback] - */ -ZlibBase.prototype.close = function(callback) { - if (callback) finished(this, callback); - this.destroy(); -}; + /** + * Force Transform compat behavior. + * @returns {void} + */ + _final(callback) { + callback(); + } -ZlibBase.prototype._destroy = function(err, callback) { - _close(this); - callback(err); -}; + flush(kind, callback) { + if (typeof kind === 'function' || (kind === undefined && !callback)) { + callback = kind; + kind = this._defaultFullFlushFlag; + } -ZlibBase.prototype._transform = function(chunk, encoding, cb) { - let flushFlag = this._defaultFlushFlag; - // We use a 'fake' zero-length chunk to carry information about flushes from - // the public API to the actual stream implementation. - if (typeof chunk[kFlushFlag] === 'number') { - flushFlag = chunk[kFlushFlag]; + kind = checkRangesOrGetDefault( + kind, 'kind', + FLUSH_BOUND[this._flushBoundIdx][0], FLUSH_BOUND[this._flushBoundIdx][1], + this._defaultFullFlushFlag); + + if (this.writableFinished) { + if (callback) + process.nextTick(callback); + } else if (this.writableEnded) { + if (callback) + this.once('end', callback); + } else { + this.write(kFlushBuffers[kind], '', callback); + } } - // For the last chunk, also apply `_finishFlushFlag`. - if (this.writableEnded && this.writableLength === chunk.byteLength) { - flushFlag = maxFlush(flushFlag, this._finishFlushFlag); + /** + * @param {(err?: Error) => any} [callback] + */ + close(callback) { + if (callback) finished(this, callback); + this.destroy(); } - processChunk(this, chunk, flushFlag, cb); -}; -ZlibBase.prototype._processChunk = function(chunk, flushFlag, cb) { - // _processChunk() is left for backwards compatibility - if (typeof cb === 'function') + _destroy(err, callback) { + _close(this); + callback(err); + } + + _transform(chunk, encoding, cb) { + let flushFlag = this._defaultFlushFlag; + // We use a 'fake' zero-length chunk to carry information about flushes from + // the public API to the actual stream implementation. + if (typeof chunk[kFlushFlag] === 'number') { + flushFlag = chunk[kFlushFlag]; + } + + // For the last chunk, also apply `_finishFlushFlag`. + if (this.writableEnded && this.writableLength === chunk.byteLength) { + flushFlag = maxFlush(flushFlag, this._finishFlushFlag); + } processChunk(this, chunk, flushFlag, cb); - else - return processChunkSync(this, chunk, flushFlag); -}; + } + + _processChunk(chunk, flushFlag, cb) { + // _processChunk() is left for backwards compatibility + if (typeof cb === 'function') + processChunk(this, chunk, flushFlag, cb); + else + return processChunkSync(this, chunk, flushFlag); + } +} function processChunkSync(self, chunk, flushFlag) { let availInBefore = chunk.byteLength; @@ -649,80 +634,6 @@ const zlibDefaultOpts = { finishFlush: Z_FINISH, fullFlush: Z_FULL_FLUSH, }; -// Base class for all streams actually backed by zlib and using zlib-specific -// parameters. -function Zlib(opts, mode) { - let windowBits = Z_DEFAULT_WINDOWBITS; - let level = Z_DEFAULT_COMPRESSION; - let memLevel = Z_DEFAULT_MEMLEVEL; - let strategy = Z_DEFAULT_STRATEGY; - let dictionary; - - if (opts) { - // windowBits is special. On the compression side, 0 is an invalid value. - // But on the decompression side, a value of 0 for windowBits tells zlib - // to use the window size in the zlib header of the compressed stream. - if ((opts.windowBits == null || opts.windowBits === 0) && - (mode === INFLATE || - mode === GUNZIP || - mode === UNZIP)) { - windowBits = 0; - } else { - // `{ windowBits: 8 }` is valid for deflate but not gzip. - const min = Z_MIN_WINDOWBITS + (mode === GZIP ? 1 : 0); - windowBits = checkRangesOrGetDefault( - opts.windowBits, 'options.windowBits', - min, Z_MAX_WINDOWBITS, Z_DEFAULT_WINDOWBITS); - } - - level = checkRangesOrGetDefault( - opts.level, 'options.level', - Z_MIN_LEVEL, Z_MAX_LEVEL, Z_DEFAULT_COMPRESSION); - - memLevel = checkRangesOrGetDefault( - opts.memLevel, 'options.memLevel', - Z_MIN_MEMLEVEL, Z_MAX_MEMLEVEL, Z_DEFAULT_MEMLEVEL); - - strategy = checkRangesOrGetDefault( - opts.strategy, 'options.strategy', - Z_DEFAULT_STRATEGY, Z_FIXED, Z_DEFAULT_STRATEGY); - - dictionary = opts.dictionary; - if (dictionary !== undefined && !isArrayBufferView(dictionary)) { - if (isAnyArrayBuffer(dictionary)) { - dictionary = Buffer.from(dictionary); - } else { - throw new ERR_INVALID_ARG_TYPE( - 'options.dictionary', - ['Buffer', 'TypedArray', 'DataView', 'ArrayBuffer'], - dictionary, - ); - } - } - } - - const handle = new binding.Zlib(mode); - // Ideally, we could let ZlibBase() set up _writeState. I haven't been able - // to come up with a good solution that doesn't break our internal API, - // and with it all supported npm versions at the time of writing. - this._writeState = new Uint32Array(2); - handle.init(windowBits, - level, - memLevel, - strategy, - this._writeState, - processCallback, - dictionary, - opts?.rejectGarbageAfterEnd === true); - - ZlibBase.call(this, opts, mode, handle, zlibDefaultOpts); - - this._level = level; - this._strategy = strategy; - this._mode = mode; -} -ObjectSetPrototypeOf(Zlib.prototype, ZlibBase.prototype); -ObjectSetPrototypeOf(Zlib, ZlibBase); // This callback is used by `.params()` to wait until a full flush happened // before adjusting the parameters. In particular, the call to the native @@ -738,85 +649,140 @@ function paramsAfterFlushCallback(level, strategy, callback) { } } -Zlib.prototype.params = function params(level, strategy, callback) { - checkRangesOrGetDefault(level, 'level', Z_MIN_LEVEL, Z_MAX_LEVEL); - checkRangesOrGetDefault(strategy, 'strategy', Z_DEFAULT_STRATEGY, Z_FIXED); +// Base class for all streams actually backed by zlib and using zlib-specific +// parameters. +class Zlib extends ZlibBase { + constructor(opts, mode) { + let windowBits = Z_DEFAULT_WINDOWBITS; + let level = Z_DEFAULT_COMPRESSION; + let memLevel = Z_DEFAULT_MEMLEVEL; + let strategy = Z_DEFAULT_STRATEGY; + let dictionary; + + if (opts) { + // windowBits is special. On the compression side, 0 is an invalid value. + // But on the decompression side, a value of 0 for windowBits tells zlib + // to use the window size in the zlib header of the compressed stream. + if ((opts.windowBits == null || opts.windowBits === 0) && + (mode === INFLATE || + mode === GUNZIP || + mode === UNZIP)) { + windowBits = 0; + } else { + // `{ windowBits: 8 }` is valid for deflate but not gzip. + const min = Z_MIN_WINDOWBITS + (mode === GZIP ? 1 : 0); + windowBits = checkRangesOrGetDefault( + opts.windowBits, 'options.windowBits', + min, Z_MAX_WINDOWBITS, Z_DEFAULT_WINDOWBITS); + } - if (this._level !== level || this._strategy !== strategy) { - this.flush( - Z_SYNC_FLUSH, - paramsAfterFlushCallback.bind(this, level, strategy, callback), - ); - } else { - process.nextTick(callback); + level = checkRangesOrGetDefault( + opts.level, 'options.level', + Z_MIN_LEVEL, Z_MAX_LEVEL, Z_DEFAULT_COMPRESSION); + + memLevel = checkRangesOrGetDefault( + opts.memLevel, 'options.memLevel', + Z_MIN_MEMLEVEL, Z_MAX_MEMLEVEL, Z_DEFAULT_MEMLEVEL); + + strategy = checkRangesOrGetDefault( + opts.strategy, 'options.strategy', + Z_DEFAULT_STRATEGY, Z_FIXED, Z_DEFAULT_STRATEGY); + + dictionary = opts.dictionary; + if (dictionary !== undefined && !isArrayBufferView(dictionary)) { + if (isAnyArrayBuffer(dictionary)) { + dictionary = Buffer.from(dictionary); + } else { + throw new ERR_INVALID_ARG_TYPE( + 'options.dictionary', + ['Buffer', 'TypedArray', 'DataView', 'ArrayBuffer'], + dictionary, + ); + } + } + } + + const handle = new binding.Zlib(mode); + // Ideally, we could let ZlibBase() set up _writeState. I haven't been able + // to come up with a good solution that doesn't break our internal API, + // and with it all supported npm versions at the time of writing. + const writeState = new Uint32Array(2); + handle.init(windowBits, + level, + memLevel, + strategy, + writeState, + processCallback, + dictionary, + opts?.rejectGarbageAfterEnd === true); + + super(opts, mode, handle, zlibDefaultOpts); + + this._writeState = writeState; + this._level = level; + this._strategy = strategy; + this._mode = mode; } -}; + + params(level, strategy, callback) { + checkRangesOrGetDefault(level, 'level', Z_MIN_LEVEL, Z_MAX_LEVEL); + checkRangesOrGetDefault(strategy, 'strategy', Z_DEFAULT_STRATEGY, Z_FIXED); + + if (this._level !== level || this._strategy !== strategy) { + this.flush( + Z_SYNC_FLUSH, + paramsAfterFlushCallback.bind(this, level, strategy, callback), + ); + } else { + process.nextTick(callback); + } + } +} // generic zlib // minimal 2-byte header -function Deflate(opts) { - if (!(this instanceof Deflate)) { - return deprecateInstantiation(Deflate, 'DEP0184', opts); +class Deflate extends Zlib { + constructor(opts) { + super(opts, DEFLATE); } - Zlib.call(this, opts, DEFLATE); } -ObjectSetPrototypeOf(Deflate.prototype, Zlib.prototype); -ObjectSetPrototypeOf(Deflate, Zlib); -function Inflate(opts) { - if (!(this instanceof Inflate)) { - return deprecateInstantiation(Inflate, 'DEP0184', opts); +class Inflate extends Zlib { + constructor(opts) { + super(opts, INFLATE); } - Zlib.call(this, opts, INFLATE); } -ObjectSetPrototypeOf(Inflate.prototype, Zlib.prototype); -ObjectSetPrototypeOf(Inflate, Zlib); -function Gzip(opts) { - if (!(this instanceof Gzip)) { - return deprecateInstantiation(Gzip, 'DEP0184', opts); +class Gzip extends Zlib { + constructor(opts) { + super(opts, GZIP); } - Zlib.call(this, opts, GZIP); } -ObjectSetPrototypeOf(Gzip.prototype, Zlib.prototype); -ObjectSetPrototypeOf(Gzip, Zlib); -function Gunzip(opts) { - if (!(this instanceof Gunzip)) { - return deprecateInstantiation(Gunzip, 'DEP0184', opts); +class Gunzip extends Zlib { + constructor(opts) { + super(opts, GUNZIP); } - Zlib.call(this, opts, GUNZIP); } -ObjectSetPrototypeOf(Gunzip.prototype, Zlib.prototype); -ObjectSetPrototypeOf(Gunzip, Zlib); -function DeflateRaw(opts) { - if (opts && opts.windowBits === 8) opts.windowBits = 9; - if (!(this instanceof DeflateRaw)) { - return deprecateInstantiation(DeflateRaw, 'DEP0184', opts); +class DeflateRaw extends Zlib { + constructor(opts) { + if (opts && opts.windowBits === 8) opts.windowBits = 9; + super(opts, DEFLATERAW); } - Zlib.call(this, opts, DEFLATERAW); } -ObjectSetPrototypeOf(DeflateRaw.prototype, Zlib.prototype); -ObjectSetPrototypeOf(DeflateRaw, Zlib); -function InflateRaw(opts) { - if (!(this instanceof InflateRaw)) { - return deprecateInstantiation(InflateRaw, 'DEP0184', opts); +class InflateRaw extends Zlib { + constructor(opts) { + super(opts, INFLATERAW); } - Zlib.call(this, opts, INFLATERAW); } -ObjectSetPrototypeOf(InflateRaw.prototype, Zlib.prototype); -ObjectSetPrototypeOf(InflateRaw, Zlib); -function Unzip(opts) { - if (!(this instanceof Unzip)) { - return deprecateInstantiation(Unzip, 'DEP0184', opts); +class Unzip extends Zlib { + constructor(opts) { + super(opts, UNZIP); } - Zlib.call(this, opts, UNZIP); } -ObjectSetPrototypeOf(Unzip.prototype, Zlib.prototype); -ObjectSetPrototypeOf(Unzip, Zlib); function createConvenienceMethod(ctor, sync) { if (sync) { @@ -844,73 +810,69 @@ const brotliDefaultOpts = { finishFlush: BROTLI_OPERATION_FINISH, fullFlush: BROTLI_OPERATION_FLUSH, }; -function Brotli(opts, mode) { - assert(mode === BROTLI_DECODE || mode === BROTLI_ENCODE); - - brotliInitParamsArray.fill(-1); - if (opts?.params) { - ObjectKeys(opts.params).forEach((origKey) => { - const key = +origKey; - if (NumberIsNaN(key) || key < 0 || key > kMaxBrotliParam || - (brotliInitParamsArray[key] | 0) !== -1) { - throw new ERR_BROTLI_INVALID_PARAM(origKey); - } - const value = opts.params[origKey]; - if (typeof value !== 'number' && typeof value !== 'boolean') { - throw new ERR_INVALID_ARG_TYPE('options.params[key]', - 'number', opts.params[origKey]); - } - brotliInitParamsArray[key] = value; - }); - } +class Brotli extends ZlibBase { + constructor(opts, mode) { + assert(mode === BROTLI_DECODE || mode === BROTLI_ENCODE); - let dictionary = opts?.dictionary; - if (dictionary !== undefined && !isArrayBufferView(dictionary)) { - if (isAnyArrayBuffer(dictionary)) { - dictionary = Buffer.from(dictionary); - } else { - throw new ERR_INVALID_ARG_TYPE( - 'options.dictionary', - ['Buffer', 'TypedArray', 'DataView', 'ArrayBuffer'], - dictionary, - ); + brotliInitParamsArray.fill(-1); + if (opts?.params) { + ObjectKeys(opts.params).forEach((origKey) => { + const key = +origKey; + if (NumberIsNaN(key) || key < 0 || key > kMaxBrotliParam || + (brotliInitParamsArray[key] | 0) !== -1) { + throw new ERR_BROTLI_INVALID_PARAM(origKey); + } + + const value = opts.params[origKey]; + if (typeof value !== 'number' && typeof value !== 'boolean') { + throw new ERR_INVALID_ARG_TYPE('options.params[key]', + 'number', opts.params[origKey]); + } + brotliInitParamsArray[key] = value; + }); + } + + let dictionary = opts?.dictionary; + if (dictionary !== undefined && !isArrayBufferView(dictionary)) { + if (isAnyArrayBuffer(dictionary)) { + dictionary = Buffer.from(dictionary); + } else { + throw new ERR_INVALID_ARG_TYPE( + 'options.dictionary', + ['Buffer', 'TypedArray', 'DataView', 'ArrayBuffer'], + dictionary, + ); + } } - } - const handle = mode === BROTLI_DECODE ? - new binding.BrotliDecoder(mode) : new binding.BrotliEncoder(mode); + const handle = mode === BROTLI_DECODE ? + new binding.BrotliDecoder(mode) : new binding.BrotliEncoder(mode); - this._writeState = new Uint32Array(2); - handle.init( - brotliInitParamsArray, - this._writeState, - processCallback, - dictionary, - ); + const writeState = new Uint32Array(2); + handle.init( + brotliInitParamsArray, + writeState, + processCallback, + dictionary, + ); - ZlibBase.call(this, opts, mode, handle, brotliDefaultOpts); + super(opts, mode, handle, brotliDefaultOpts); + this._writeState = writeState; + } } -ObjectSetPrototypeOf(Brotli.prototype, Zlib.prototype); -ObjectSetPrototypeOf(Brotli, Zlib); -function BrotliCompress(opts) { - if (!(this instanceof BrotliCompress)) { - return deprecateInstantiation(BrotliCompress, 'DEP0184', opts); +class BrotliCompress extends Brotli { + constructor(opts) { + super(opts, BROTLI_ENCODE); } - Brotli.call(this, opts, BROTLI_ENCODE); } -ObjectSetPrototypeOf(BrotliCompress.prototype, Brotli.prototype); -ObjectSetPrototypeOf(BrotliCompress, Brotli); -function BrotliDecompress(opts) { - if (!(this instanceof BrotliDecompress)) { - return deprecateInstantiation(BrotliDecompress, 'DEP0184', opts); +class BrotliDecompress extends Brotli { + constructor(opts) { + super(opts, BROTLI_DECODE); } - Brotli.call(this, opts, BROTLI_DECODE); } -ObjectSetPrototypeOf(BrotliDecompress.prototype, Brotli.prototype); -ObjectSetPrototypeOf(BrotliDecompress, Brotli); const zstdDefaultOpts = { diff --git a/test/parallel/test-zlib-deflate-constructors.js b/test/parallel/test-zlib-deflate-constructors.js index 492048d3d0a1..66a505c13e70 100644 --- a/test/parallel/test-zlib-deflate-constructors.js +++ b/test/parallel/test-zlib-deflate-constructors.js @@ -5,13 +5,19 @@ require('../common'); const zlib = require('zlib'); const assert = require('assert'); -// Work with and without `new` keyword -assert.ok(zlib.Deflate() instanceof zlib.Deflate); +// Require the `new` keyword (DEP0184 End-of-Life) assert.ok(new zlib.Deflate() instanceof zlib.Deflate); - -assert.ok(zlib.DeflateRaw() instanceof zlib.DeflateRaw); assert.ok(new zlib.DeflateRaw() instanceof zlib.DeflateRaw); +assert.throws(() => zlib.Deflate(), { + name: 'TypeError', + message: /Class constructor Deflate cannot be invoked without 'new'/, +}); +assert.throws(() => zlib.DeflateRaw(), { + name: 'TypeError', + message: /Class constructor DeflateRaw cannot be invoked without 'new'/, +}); + // Throws if `options.chunkSize` is invalid assert.throws( () => new zlib.Deflate({ chunkSize: 'test' }), diff --git a/test/parallel/test-zlib-deflate-raw-inherits.js b/test/parallel/test-zlib-deflate-raw-inherits.js index 34bf31058a1d..1f780172dbee 100644 --- a/test/parallel/test-zlib-deflate-raw-inherits.js +++ b/test/parallel/test-zlib-deflate-raw-inherits.js @@ -4,15 +4,14 @@ require('../common'); const { DeflateRaw } = require('zlib'); const { Readable } = require('stream'); -// Validates that zlib.DeflateRaw can be inherited -// with Object.setPrototypeOf +// Validates that zlib.DeflateRaw can be subclassed with class syntax. -function NotInitialized(options) { - DeflateRaw.call(this, options); - this.prop = true; +class NotInitialized extends DeflateRaw { + constructor(options) { + super(options); + this.prop = true; + } } -Object.setPrototypeOf(NotInitialized.prototype, DeflateRaw.prototype); -Object.setPrototypeOf(NotInitialized, DeflateRaw); const dest = new NotInitialized(); diff --git a/test/parallel/test-zlib-invalid-arg-value-brotli-compress.js b/test/parallel/test-zlib-invalid-arg-value-brotli-compress.js index 688acddd16a1..38e95cf6b049 100644 --- a/test/parallel/test-zlib-invalid-arg-value-brotli-compress.js +++ b/test/parallel/test-zlib-invalid-arg-value-brotli-compress.js @@ -15,6 +15,6 @@ const opts = { } }; -assert.throws(() => BrotliCompress(opts), { +assert.throws(() => new BrotliCompress(opts), { code: 'ERR_INVALID_ARG_TYPE' }); diff --git a/test/parallel/test-zlib-invalid-input.js b/test/parallel/test-zlib-invalid-input.js index 721badc13f6a..7a0146a5d641 100644 --- a/test/parallel/test-zlib-invalid-input.js +++ b/test/parallel/test-zlib-invalid-input.js @@ -35,11 +35,11 @@ const nonStringInputs = [ // zlib.Unzip classes need to get valid data, or else they'll throw. const unzips = [ - zlib.Unzip(), - zlib.Gunzip(), - zlib.Inflate(), - zlib.InflateRaw(), - zlib.BrotliDecompress(), + new zlib.Unzip(), + new zlib.Gunzip(), + new zlib.Inflate(), + new zlib.InflateRaw(), + new zlib.BrotliDecompress(), new zlib.ZstdDecompress(), ]; diff --git a/test/parallel/test-zlib-zip-coverage.js b/test/parallel/test-zlib-zip-coverage.js index 437630df95cd..74cdab3b431b 100644 --- a/test/parallel/test-zlib-zip-coverage.js +++ b/test/parallel/test-zlib-zip-coverage.js @@ -288,8 +288,9 @@ test('a Zip64 extra field value beyond Number.MAX_SAFE_INTEGER is rejected', asy uncompressedSize: 0xffffffffffffffffn, }); - const [read] = zlib.ZipEntry.read(patched); - assert.throws(() => read.size, { + // The header cross-check resolves the sizes as the archive is read, so a + // malformed Zip64 value is rejected at read time rather than at .size access. + assert.throws(() => [...zlib.ZipEntry.read(patched)], { code: 'ERR_ZIP_INVALID_ARCHIVE', message: /exceeds the safe integer range/, }); @@ -325,8 +326,7 @@ test('a Zip64 extra-field TLV whose declared size overflows the extra field is r tlv.writeUInt16LE(100, 2); // claims 100 bytes of data, but none follow const patched = injectRawZip64Extra(archive, name, content.length, tlv); - const [read] = zlib.ZipEntry.read(patched); - assert.throws(() => read.size, { + assert.throws(() => [...zlib.ZipEntry.read(patched)], { code: 'ERR_ZIP_INVALID_ARCHIVE', message: /extra field is malformed/, }); @@ -345,8 +345,7 @@ test('a Zip64 extra-field TLV too short for the field it claims to carry is reje tlv.writeUInt32LE(123, 4); const patched = injectRawZip64Extra(archive, name, content.length, tlv); - const [read] = zlib.ZipEntry.read(patched); - assert.throws(() => read.size, { + assert.throws(() => [...zlib.ZipEntry.read(patched)], { code: 'ERR_ZIP_INVALID_ARCHIVE', message: /Zip64 extended information extra field is truncated/, }); @@ -500,7 +499,8 @@ test('contentIterator() enforces the same guards as content() and contentSync()' const archive = await buildArchive([entry]); const tampered = Buffer.from(archive); const centralStart = 30 + 'f.txt'.length + 'hello world'.length; - tampered.writeUInt32LE(1, centralStart + 24); + tampered.writeUInt32LE(1, 22); // Local uncompressed size + tampered.writeUInt32LE(1, centralStart + 24); // Central uncompressed size const [read] = zlib.ZipEntry.read(tampered); await assert.rejects(drain(read.contentIterator()), { code: 'ERR_ZIP_ENTRY_CORRUPT' }); } @@ -593,7 +593,8 @@ test('ZipFile getSync().contentSync() enforces the same guards via decodeMemberS const archive = await buildArchive([entry]); const tampered = Buffer.from(archive); const centralStart = 30 + 'f.txt'.length + 'hello world'.length; - tampered.writeUInt32LE(1, centralStart + 24); + tampered.writeUInt32LE(1, 22); // Local uncompressed size + tampered.writeUInt32LE(1, centralStart + 24); // Central uncompressed size const filePath = await writeTempArchive(tampered, 'size-mismatch'); const zf = zlib.ZipFile.openSync(filePath); assert.throws(() => zf.getSync('f.txt').contentSync(), { code: 'ERR_ZIP_ENTRY_CORRUPT' }); @@ -607,7 +608,8 @@ test('contentIterator() rejects an entry that inflates to less than its declared const archive = await buildArchive([entry]); const tampered = Buffer.from(archive); const centralStart = 30 + 'f.txt'.length + 'hi'.length; - tampered.writeUInt32LE(1000, centralStart + 24); // Declared size grown beyond reality + tampered.writeUInt32LE(1000, 22); // Local declared size + tampered.writeUInt32LE(1000, centralStart + 24); // Central declared size grown beyond reality const [read] = zlib.ZipEntry.read(tampered); await assert.rejects(drain(read.contentIterator()), { code: 'ERR_ZIP_ENTRY_CORRUPT', diff --git a/test/parallel/test-zlib-zip-file-lifecycle.js b/test/parallel/test-zlib-zip-file-lifecycle.js new file mode 100644 index 000000000000..85a06f86b75e --- /dev/null +++ b/test/parallel/test-zlib-zip-file-lifecycle.js @@ -0,0 +1,92 @@ +'use strict'; + +// Lifecycle-hardening regression tests for node:zlib ZipFile (the on-disk, +// fd-backed reader/writer). Each asserts the secure behavior, so it fails on +// the pre-fix code and passes once its fix lands. +// +// 1. A read in flight when close() is called must complete on a live +// descriptor - close() must not pull the fd out from under it (which would +// surface as EBADF, or worse read another file once the fd number is +// reused). +// 2. If the central-directory rewrite fails after an add() has written the +// member bytes, the in-memory state and the on-disk archive must be rolled +// back to exactly what they were before the call, not left half-updated. + +require('../common'); +const assert = require('assert'); +const { test } = require('node:test'); +const zlib = require('zlib'); +const fs = require('fs'); +const tmpdir = require('../common/tmpdir'); + +const SIG_CENTRAL = 0x02014b50; +const SIG_EOCD = 0x06054b50; + +function writeArchive(file, entries) { + const chunks = []; + for (const chunk of zlib.createZipArchiveSync(entries)) chunks.push(chunk); + fs.writeFileSync(file, Buffer.concat(chunks)); +} + +// 1. A read that is in flight when the ZipFile is closed still completes. +test('close() waits for an in-flight read instead of closing under it', async () => { + tmpdir.refresh(); + const file = tmpdir.resolve('inflight.zip'); + const payload = Buffer.alloc(8 * 1024 * 1024, 0x5a); + writeArchive(file, [zlib.ZipEntry.createSync('big', payload, { method: 'store' })]); + + const zf = zlib.ZipFile.openSync(file); + const entry = zf.getSync('big'); + const reading = entry.content(); // In flight; do not await yet + await zf.close(); // Must wait for the read, not close the fd under it + const data = await reading; // Must resolve with correct bytes, not reject EBADF + assert.strictEqual(data.length, payload.length); + assert.ok(data.equals(payload)); +}); + +// 2. A failed central-directory rewrite during add() is rolled back. +test('a failed directory rewrite during addEntrySync is rolled back', () => { + tmpdir.refresh(); + const file = tmpdir.resolve('addfail.zip'); + writeArchive(file, [zlib.ZipEntry.createSync('first.txt', Buffer.from('original'), + { method: 'store' })]); + + const zf = zlib.ZipFile.openSync(file, { writable: true }); + const toAdd = zlib.ZipEntry.createSync('second.txt', Buffer.from('added'), { method: 'store' }); + + // Fail only the first central-directory write (its buffer starts with the + // central-header or EOCD signature); the member bytes start with the local + // header signature and pass through, and the rollback rewrite that follows + // succeeds so the on-disk archive is restored. + const realWriteSync = fs.writeSync; + let failNextDirectoryWrite = true; + fs.writeSync = function(fd, buffer, offset, length, position) { + if (failNextDirectoryWrite && Buffer.isBuffer(buffer) && buffer.length - offset >= 4) { + const sig = buffer.readUInt32LE(offset); + if (sig === SIG_CENTRAL || sig === SIG_EOCD) { + failNextDirectoryWrite = false; + const err = new Error('ENOSPC: simulated no space left on device'); + err.code = 'ENOSPC'; + throw err; + } + } + return realWriteSync.call(fs, fd, buffer, offset, length, position); + }; + try { + assert.throws(() => zf.addEntrySync(toAdd), { code: 'ENOSPC' }); + } finally { + fs.writeSync = realWriteSync; + } + + // In-memory: the half-added entry is gone and the original is still readable. + assert.ok(!zf.has('second.txt')); + assert.strictEqual(zf.getSync('first.txt').contentSync().toString(), 'original'); + zf.closeSync(); + + // On disk: reopening shows the original, uncorrupted archive. + const reopened = zlib.ZipFile.openSync(file); + assert.ok(reopened.has('first.txt')); + assert.ok(!reopened.has('second.txt')); + assert.strictEqual(reopened.getSync('first.txt').contentSync().toString(), 'original'); + reopened.closeSync(); +}); diff --git a/test/parallel/test-zlib-zip-hardening.js b/test/parallel/test-zlib-zip-hardening.js index 9427a546c889..3ec0b015a238 100644 --- a/test/parallel/test-zlib-zip-hardening.js +++ b/test/parallel/test-zlib-zip-hardening.js @@ -118,13 +118,14 @@ test('a declared-size mismatch is rejected as corrupt', async () => { const entry = await zlib.ZipEntry.create('f.txt', Buffer.from('hello world'), { method: 'store' }); const archive = await buildArchive([entry]); - // Shrink the *declared* uncompressed size in the central directory record - // without touching the stored bytes themselves, so the amount of data - // produced no longer matches what the header promised. + // Shrink the *declared* uncompressed size in both the local and central + // headers (kept consistent so the header cross-check passes) without touching + // the stored bytes, so the produced amount no longer matches the headers' + // promise and the decode-time size check fires. const tampered = Buffer.from(archive); const centralHeaderStart = 30 + 'f.txt'.length + 'hello world'.length; - const uncompressedSizeOffset = centralHeaderStart + 24; - tampered.writeUInt32LE(1, uncompressedSizeOffset); + tampered.writeUInt32LE(1, 22); // Local uncompressed size + tampered.writeUInt32LE(1, centralHeaderStart + 24); // Central uncompressed size const [tamperedEntry] = zlib.ZipEntry.read(tampered); assert.strictEqual(tamperedEntry.size, 1); @@ -165,7 +166,10 @@ test('a forged small header whose content inflates past its declared size is rej const tampered = Buffer.from(archive); const eocd = tampered.length - 22; // No comment, so EOCD is the last 22 bytes const cdOffset = tampered.readUInt32LE(eocd + 16); - tampered.writeUInt32LE(50, cdOffset + 24); // Forge declared uncompressedSize + // Forge the declared uncompressedSize in both headers (kept consistent so + // the header cross-check passes; the decompressor still catches the lie). + tampered.writeUInt32LE(50, 22); // Local uncompressedSize + tampered.writeUInt32LE(50, cdOffset + 24); // Central uncompressedSize const [e] = zlib.ZipEntry.read(tampered); assert.strictEqual(e.size, 50); // 50 <= maxSize 100 clears the up-front check @@ -413,9 +417,11 @@ test('a member whose data crosses into the central directory is rejected', async const archive = Buffer.from(await buildArchive( [await zlib.ZipEntry.create(name, content, { method: 'store' })])); const centralHeaderStart = 30 + name.length + content.length; - // Lie about the compressed size so the member's data range reaches into - // the central directory (while staying inside the buffer). - archive.writeUInt32LE(content.length + 40, centralHeaderStart + 20); + // Lie about the compressed size in both headers (kept consistent so the + // header cross-check passes) so the member's data range reaches into the + // central directory (while staying inside the buffer). + archive.writeUInt32LE(content.length + 40, 18); // Local compressed size + archive.writeUInt32LE(content.length + 40, centralHeaderStart + 20); // Central assert.throws(() => [...zlib.ZipEntry.read(archive)], { code: 'ERR_ZIP_INVALID_ARCHIVE', message: /possible zip bomb/ }); }); diff --git a/test/parallel/test-zlib-zip-security-hardening.js b/test/parallel/test-zlib-zip-security-hardening.js new file mode 100644 index 000000000000..97d5105a616f --- /dev/null +++ b/test/parallel/test-zlib-zip-security-hardening.js @@ -0,0 +1,120 @@ +'use strict'; + +// Security-hardening regression tests for node:zlib ZIP support. Each test +// describes a distinct issue found by audit and asserts the *secure* behavior, +// so every test fails on the pre-fix code and passes once its fix lands. +// +// 1. Local-vs-central header confusion (parser-confusion / inspect-then-consume +// divergence): the central directory is authoritative in this reader, but a +// local file header that disagrees on method, sizes, CRC, or the encryption +// flag lets another ZIP tool (which extracts from the local header) read a +// different member from the same archive. Such an archive must be rejected. +// 2. The archiver (zipFiles) must not block forever on a FIFO/special source. +// 3. The streaming read path (contentIterator) is hard-bounded by the header's +// declared uncompressed size: a member that inflates past it is rejected +// mid-stream, so entry.size is a ceiling a consumer can trust up front. + +require('../common'); +const assert = require('assert'); +const { test } = require('node:test'); +const zlib = require('zlib'); +const path = require('path'); +const { spawnSync } = require('child_process'); +const tmpdir = require('../common/tmpdir'); + +const SIG_LOCAL = 0x04034b50; +const SIG_CENTRAL = 0x02014b50; + +// A minimal single-member archive; caller patches its headers. +function buildStored(name, content, method = 'store') { + const entry = zlib.ZipEntry.createSync(name, Buffer.from(content), { method }); + const chunks = []; + for (const chunk of zlib.createZipArchiveSync([entry])) chunks.push(chunk); + return Buffer.concat(chunks); +} + +function centralOffset(buf) { + for (let i = buf.length - 22; i >= 0; i--) { + if (buf.readUInt32LE(i) === SIG_CENTRAL) return i; + } + throw new Error('no central directory header found'); +} + +// A ZipBuffer parses the central directory; reading a member resolves and +// checks its local header. Do both so the assertion holds whether the check +// is eager (parse time) or lazy (read time). +function readsThrow(buf, name) { + assert.throws(() => { + const zb = new zlib.ZipBuffer(buf); + zb.get(name).contentSync(); + }, { code: 'ERR_ZIP_INVALID_ARCHIVE' }); +} + +// 1a. Local vs central compressed/uncompressed size disagreement. +test('a local/central size disagreement is rejected', () => { + const buf = buildStored('a.txt', 'hello'); + assert.strictEqual(buf.readUInt32LE(0), SIG_LOCAL); + buf.writeUInt32LE(999, 18); // Local compressed size + buf.writeUInt32LE(999, 22); // Local uncompressed size + readsThrow(buf, 'a.txt'); +}); + +// 1b. Local vs central compression-method disagreement. +test('a local/central method disagreement is rejected', () => { + const buf = buildStored('a.txt', 'hello'); + buf.writeUInt16LE(8, 8); // Local method deflate; central stays store(0) + readsThrow(buf, 'a.txt'); +}); + +// 1c. Central marks the member encrypted while the local header does not: +// a central-directory reader (e.g. python) treats it as opaque/encrypted, so +// Node must not silently decode it either. +test('a local/central encryption-flag disagreement is rejected', () => { + const buf = buildStored('a.txt', 'hello'); + const c = centralOffset(buf); + buf.writeUInt16LE(buf.readUInt16LE(c + 8) | 0x0001, c + 8); // Central encrypted bit + readsThrow(buf, 'a.txt'); +}); + +// 2. zipFiles must reject a FIFO source rather than block on open() forever. +test('zipFiles rejects a FIFO source instead of hanging', () => { + if (process.platform === 'win32') return; // no mkfifo + tmpdir.refresh(); + const fifo = path.join(tmpdir.path, 'evil.fifo'); + if (spawnSync('mkfifo', [fifo]).status !== 0) return; // mkfifo unavailable + const script = + 'const zlib = require("zlib");' + + '(async () => {' + + ' try {' + + ` for await (const _ of zlib.zipFiles([[${JSON.stringify(fifo)}, "x"]], ` + + ' { followSymlinks: false })) {}' + + ' console.log("COMPLETED");' + + ' } catch (e) { console.log("REJECTED:" + e.code); }' + + '})();'; + const res = spawnSync(process.execPath, ['--no-warnings', '-e', script], + { timeout: 5000, encoding: 'utf8' }); + assert.ok(res.signal === null, + 'zipFiles hung on a FIFO source (killed by timeout)'); + assert.match(res.stdout, /REJECTED:ERR_ZIP_UNSUPPORTED_FEATURE/); +}); + +// 3. Streaming is hard-bounded by the declared uncompressed size: a member +// whose data inflates past it is rejected mid-stream, so a consumer can trust +// entry.size as the ceiling before choosing to buffer. +test('contentIterator rejects a member that inflates past its declared size', async () => { + const big = zlib.ZipEntry.createSync( + 'big', Buffer.alloc(64 * 1024), { method: 'deflate' }); + const chunks = []; + for (const chunk of zlib.createZipArchiveSync([big])) chunks.push(chunk); + const buf = Buffer.concat(chunks); + // Shrink the declared uncompressed size in both headers (kept consistent so + // the header cross-check passes) below what the data actually inflates to. + const c = centralOffset(buf); + buf.writeUInt32LE(100, 22); // Local uncompressed size + buf.writeUInt32LE(100, c + 24); // Central uncompressed size + const entry = new zlib.ZipBuffer(buf).get('big'); + await assert.rejects(async () => { + // eslint-disable-next-line no-unused-vars + for await (const _ of entry.contentIterator()) { /* drain */ } + }, { code: 'ERR_ZIP_ENTRY_CORRUPT', message: /inflates beyond its declared size/ }); +}); diff --git a/test/parallel/test-zlib.js b/test/parallel/test-zlib.js index bda63a45b7b1..35fd2c946030 100644 --- a/test/parallel/test-zlib.js +++ b/test/parallel/test-zlib.js @@ -234,8 +234,9 @@ testKeys.forEach(common.mustCall((file) => { }, testKeys.length)); { - // Test instantiation without 'new' - common.expectWarning('DeprecationWarning', `Instantiating Gzip without the 'new' keyword has been deprecated.`, 'DEP0184'); - const gzip = zlib.Gzip(); - assert.ok(gzip instanceof zlib.Gzip); + // Instantiating without `new` is End-of-Life (DEP0184). + assert.throws(() => zlib.Gzip(), { + name: 'TypeError', + message: /Class constructor Gzip cannot be invoked without 'new'/, + }); }