[pull] main from nodejs:main - #1224
Merged
Merged
Conversation
Add regression tests for node:zlib ZIP hardening: - A local file header that disagrees with the central directory on compression method, sizes, CRC, or the encryption flag lets another ZIP reader extract a different member from the same archive; such an archive must be rejected (fixed in a follow-up commit). - zipFiles() must reject a FIFO/special source rather than block forever on open() (fixed in a follow-up commit). - Streaming (contentIterator) is hard-bounded by the header's declared uncompressed size and rejects a member that inflates past it, so entry.size is a ceiling a consumer can trust up front; lock that in. Signed-off-by: Philipp Dunkel <pip@pipobscure.com> PR-URL: #65016 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
The reader treats the central directory as authoritative for a member's method, sizes, CRC, and name, but read the local header only for its signature. An archive whose local file header disagrees with the central directory therefore extracts different bytes here than in a reader that uses the local header (e.g. Info-ZIP unzip), and its encrypted bit was read from the local header while its identity came from the central directory - a parser-confusion split that defeats inspect-then-consume pipelines and can slip an encrypted member past a central-directory scanner. Cross-check the local header against the central entry when a member is read: method, CRC, and both sizes (exempting a data-descriptor entry, whose local crc/sizes are legitimately zero), plus the encryption flag. Reject a mismatch with ERR_ZIP_INVALID_ARCHIVE, consistent with the reject-rather-than-silently-choose stance already taken for ambiguous archive ends. The existing hardening/coverage tests that forged a decode-time size, CRC, or Zip64 lie in the central header alone now trip this earlier check; update them to patch both headers consistently so they still exercise the decode-time guards they target. Signed-off-by: Philipp Dunkel <pip@pipobscure.com> PR-URL: #65016 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
zipFiles() opened each source before fstat-ing it, so open() on a FIFO (or a slow/blocking device) blocked indefinitely - the regular-file guard ran too late to prevent it, and each stuck open pinned a libuv threadpool thread. Open with O_NONBLOCK so the open returns promptly and the fstat can reject anything that is not a regular file; O_NONBLOCK has no effect on a regular file's subsequent reads. Signed-off-by: Philipp Dunkel <pip@pipobscure.com> PR-URL: #65016 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Add regression tests for two node:zlib ZipFile robustness issues: - A read in flight when close() is called must complete on a live descriptor; close() must not release the fd out from under it (which surfaces as EBADF, or an OS-reused-fd cross-file read). - If the central-directory rewrite fails after add() has written the member bytes, both the in-memory state and the on-disk archive must be rolled back, not left half-updated. Signed-off-by: Philipp Dunkel <pip@pipobscure.com> PR-URL: #65016 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
An entry read runs on a file descriptor shared with its ZipFile, but reads did not take part in close()'s lifecycle: close() marked the handle closed and released the fd while a read was still in flight, so the read landed on a closed - or worse, an OS-reused - descriptor (surfacing as EBADF, or a cross-file read once the number was reclaimed), despite the class comment promising otherwise. Track in-flight reads on the shared handle. close() now marks the handle closing (rejecting new reads at once), waits for the in-flight reads to finish on the still-open fd, and only then closes it; closeSync(), which cannot wait, refuses while an asynchronous read is outstanding. Signed-off-by: Philipp Dunkel <pip@pipobscure.com> PR-URL: #65016 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
add()/addEntrySync() advanced the central-directory offset and adopted the new entry into memory before the final directory rewrite. If that rewrite failed (ENOSPC/EIO after the member bytes were already written), the in-memory state and the on-disk archive were left diverged and half-updated, with no restore, corrupting the next add(). Wrap the rewrite: on failure, restore the previous offset and directory entry and rewrite the original directory back, leaving the archive and handle exactly as before the call, then rethrow. Signed-off-by: Philipp Dunkel <pip@pipobscure.com> PR-URL: #65016 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Signed-off-by: harjoth <harjoth.khara@gmail.com> PR-URL: #64548 Reviewed-By: Aviv Keller <me@aviv.sh> Reviewed-By: James M Snell <jasnell@gmail.com>
Convert ZlibBase, Zlib, and the remaining zlib/Brotli stream constructors to ES6 class syntax. Calling these constructors without `new` is no longer supported (DEP0184 End-of-Life); use `new` or the create* factory helpers instead. This is a semver-major change: code that relied on `zlib.Gzip()` (without `new`) will now throw TypeError. The createGzip/createDeflate helpers and all documented `new`-based usage remain unchanged. The wire behavior and public options are preserved. Refs: #55718 Refs: #54708 Assisted-by: Grok Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com> PR-URL: #64849 Reviewed-By: Aviv Keller <me@aviv.sh> Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
HTTP APIs in Node.js have become a bit of a mess. We have separate `node:http`, `node:https`, and `node:http2` modules. We have separate `fetch` implementation. We have `http3` support in development. There are new http features like datagrams and priorities that are entirely unsupported, etc. This strategic initiative will be focused on the development of a unified HTTP API built around the web standard fetch model. Client and Server.. independent of underlying transport and updated to modern HTTP protocol capabilities. Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #65139 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Richard Lau <richard.lau@ibm.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )