Update from upstream#3
Open
phpclub wants to merge 74 commits into
Open
Conversation
Removed the incorrect "alg":"RS256" field from RSA JWK test data files. The field is optional per RFC 7517, and the value RS256 (RSASSA-PKCS1-v1_5) was wrong for tests that use RSA-OAEP and RSA-PSS algorithms. Node.js correctly rejects JWK imports when the "alg" field does not match the requested algorithm. The hash mismatch negative test was updated to use an inline JWK with an explicit "alg" field instead of the shared file. This ensures: `test/test262 --binary=node test/webcrypto` pass.
Supports 128, 192, and 256-bit key sizes with generateKey, importKey, and exportKey operations in raw and JWK formats. Also fixed deriveKey to accept 192-bit AES key lengths.
Implemented Ed25519 sign/verify/generateKey/importKey/exportKey Supports raw, PKCS8, SPKI, and JWK (OKP) key formats. Implemented X25519 deriveBits/deriveKey/generateKey/importKey/ exportKey.
Added JWK format support to unwrapKey: decrypted data is parsed as JSON and imported through the shared import path.
Previously, a notice-level log message "js vm init %s: %p" was emitted for each JS engine created during configuration parsing. This message leaked to the compiled-in default error log even when the user explicitly configured logging elsewhere, because config-time logging uses the initial cycle log whose file descriptor is opened before the user's error_log directive takes effect. The log message was used for VM deduplication tests. The tests are updated to verify unique engine identifiers via HTTP and stream responses instead of grepping for log messages. This fixes #1042 issue on Github.
Since 3185ce8 (0.9.7), njs_crypto_module has been registered conditionally in auto/modules under NJS_HAVE_OPENSSL. libnjs.a is built for the nginx module via nginx/config.make with "./configure --no-openssl ...", so libnjs.a's njs_modules[] no longer contains the crypto module. The nginx addon lists (njs_http_js_addon_modules[] and njs_stream_js_addon_modules[], plus the qjs variants) were not updated accordingly, making "import cr from 'crypto'" fail at nginx -t with ENOENT. Added &njs_crypto_module and &qjs_crypto_module to the shared addon list under the existing NJS_HAVE_OPENSSL guard, next to the webcrypto entries. While here, the near-identical addon arrays were factored into shared macros in a new header nginx/ngx_js_modules.h, so adding a future conditional addon needs a single edit instead of four. This closes #1049 issue on Github.
The directive registers a JavaScript handler in the access phase, running after built-in access checkers (allow/deny, auth_basic, auth_request). r.subrequest(), ngx.fetch() and other async operations are supported. The handler defaults to NGX_OK (access granted) on normal completion, matching the behavior of other access phase modules. The r.decline() method allows the handler to return NGX_DECLINED (no opinion), deferring the decision to other access checkers under "satisfy any". The r.return() method can send any HTTP response from the access phase, including 3xx redirects for authentication flows.
Added async methods
- r.readRequestText() as string
- r.readRequestArrayBuffer() as ArrayBuffer
- r.readRequestJSON() as object.
that return Promises resolving with the request body wrapped
as a corresponding type.
The async method parses the client request body as an HTML form and returns a Promise resolving to a form object with get(), getAll(), has(), forEach(), hasFiles() accessors. Supports "application/x-www-form-urlencoded" and "multipart/form-data" content types. File parts are detected but their contents are not exposed. An optional maxKeys option caps the number of fields. File parts are detected but their contents are not exposed. A proper File API with streaming Blob semantics is a significant amount of work and is out of scope.
The destination buffers for the decoded user and password in ngx_js_parse_proxy_url() were a fixed 128 bytes, while the encoded input length was bounded only by the URL length. Since ngx_unescape_uri() writes at most one byte per input byte, raw credentials longer than 128 bytes overflowed the buffer; the length check ran only after the decode. The fix is to size the destination buffer based on the encoded input length. The bug appeared in dea8318 (0.9.4).
This makes the ownership of the request pointer clearer for readers and avoids unnecessary confusion for static analyzers.
Found by Coverity (CID 1692699).
Native frames always have an associated function. Keep the function lookup in the native-frame branch to make the invariant explicit and to avoid confusing static analysis. Found by Coverity (CID 1681309).
Previously, when a js_access handler called r.return(status, body), ngx_http_send_response() sent the response but returned NGX_OK. The access handler then returned NGX_OK to the phase engine, which treated the access check as allowed and continued to the content phase. As a result, a denied request could still reach proxy_pass or another content handler after the response had already been sent. Now the access handler finalizes the request if a response was already sent during js_access execution. This keeps r.return() behavior in js_content unchanged, while making r.return(status, body) terminal in js_access.
Previously, r.internalRedirect() only stored the redirect URI and set ctx->status to NGX_DONE. The stored URI was handled only by the js_content finalization path. As a result, when internalRedirect() was called from js_access, the access handler returned NGX_DONE to the phase engine without starting the redirect, leaving the request unfinished. Now the internal redirect handling is shared by js_access and js_content, so internalRedirect() works consistently in both handlers.
njs_chb_destroy() frees chain nodes through chain->free() and guards against a NULL free callback. njs_chb_drain() and the tail-freeing path of njs_chb_drop() called njs_mp_free() directly, which is wrong for chains initialized with NJS_CHB_CTX_INIT() where chain->free is js_free(), and unsafe for NGX_CHB_CTX_INIT() chains where chain->free is NULL. Both paths now route through chain->free() with the same NULL guard as njs_chb_destroy().
Since fd5e523 (0.9.7), njs has evaluated all call argument expressions before emitting the frame and PUT_ARG instructions. This fixed await expressions in call arguments and preserved argument side effects before callee validation, but left PUT_ARG reading argument values only after later argument expressions had already run. As a result, an earlier argument backed by a mutable variable slot could observe a later mutation of the same slot. For example, f(a, a = 2) passed 2 as both arguments instead of preserving the first value as 1. The same issue applied to later argument effects such as getters. The fix is to preserve affected argument values in temporary registers where appropriate. This fixes #1059 issue on Github.
The method returns names of variables declared with js_var. An optional
string argument filters returned names by prefix.
This lets JavaScript code discover a configuration-defined variable schema
without duplicating every variable name in the script. For example:
js_var $extract_name;
js_var $extract_arguments_workspace;
function handler(r) {
r.jsVarNames("extract_").forEach(function(name) {
r.variables[name] = extract(name.slice(8));
});
}
The same API is available as s.jsVarNames() in stream. Variables created by
js_set or by the core modules are not returned.
The new top-level AGENTS.md (with CLAUDE.md symlinked to it) is the
canonical index for agent and contributor instructions. Detailed
guides live under docs/agent/:
docs/agent/engine-dev.md building, testing, debugging the engine
and the nginx modules.
docs/agent/js-dev.md writing JavaScript for either engine,
with the common nginx API surface and
engine differences.
docs/agent/js-dev-njs.md specifics of the deprecated njs engine
and migration to QuickJS.
Freed the typed array constructor object after reading constructor.name while detecting Float32Array in Buffer.from(). This keeps the exception path from leaking the constructor object. Handled JS_ToCString() failure for constructor.name before comparing the name, avoiding a NULL dereference when the property converts to an exception, such as a Symbol value. Divided the source offset by the element size in qjs_buffer_from() typed-array path so the offset addresses the right element for 2-, 4-, and 8-byte element types (previously the offset was left in byte units while size was already in element units). Added a NULL check on JS_ToCStringLen() in qjs_buffer_encoding(), and moved the JS_FreeCString() call after the JS_ThrowTypeError() so the encoding name remains valid while the error message is formatted. Routed array source errors in qjs_buffer_from_object() through a single fail label so the destination buffer is freed once on every failure path (previously leaked on three of them).
The property definition consumes ownership, so freeing the result object is enough on later error paths. Returned JS_EXCEPTION on property-definition failures instead of the unrelated typed-array lookup result.
Reject control characters and DEL in Fetch Headers values while preserving OWS trimming and obs-text.
Reject empty methods, C0 control bytes, space, and DEL before a fetch request can be serialized. Preserve existing forbidden-method checks, common-method normalization, and valid server-side extension methods.
Reject raw C0 control bytes, space, and DEL in the parsed request target before fetch request serialization. Leave percent-encoded bytes unchanged.
Wrong receiver errors are caused by JS API misuse, not by internal host failures. Report TypeError for TextEncoder, TextDecoder, console, and fs Stats receiver checks.
Preserve exceptions raised by QuickJS conversions and synthesize an out of memory exception only for silent pool allocation failures in ngx_qjs_string() callers. Use JS_GetOpaque() where a class mismatch is a normal miss, avoiding a stale pending TypeError while continuing with non-Headers and non-Request input. Also release the headers init value before throwing for a non-object Headers option.
Report API misuse in Fetch, Request, Response, and Headers as TypeError, and report status bounds violations as RangeError. Keep internal host failures as InternalError and preserve conversion helper exceptions where they already provide the real cause.
Set pending exceptions before returning NJS_ERROR from njs response output paths. Previously sendHeader(), send(), and finish() could fail without an exception being available to the VM.
Report HTTP request API misuse as TypeError and status bounds violations as RangeError. Keep nginx output and request body collection failures as InternalError, since they are host/runtime failures rather than invalid JavaScript arguments.
Reset the variable value validity flag when allocating storage for a stream variable fails. This matches the HTTP variable path and prevents a failed assignment from leaving a half-initialized value marked as valid.
Report stream session API misuse as TypeError, including wrong receiver, unknown or duplicate event handlers, wrong handler phase, and invalid variable access. Report status bounds violations as RangeError and keep stream output pipeline failures as InternalError. Also fix the async send error message spacing.
Report XML API misuse as TypeError and XML parse failures as SyntaxError, aligning njs and QuickJS behavior. The QuickJS message "'this' is not XMLNode or XMLDoc" is changed to "value is not XMLNode or XMLDoc" because qjs_xml_node() is not always validating this.
Align common helper failures with the exception policy used by both engines: invalid numeric conversion is TypeError, and missing external receiver/context for common log helpers is TypeError rather than an internal host failure. The numeric conversion message changes from "is not a number" to "invalid number" to match the QuickJS helper text.
QuickJS property setter and define APIs consume the JSValue argument on both success and failure. This applies to JS_SetProperty(), JS_SetPropertyStr(), JS_SetPropertyUint32(), JS_DefinePropertyValue(), JS_DefinePropertyValueStr(), and JS_DefinePropertyValueUint32(). Do not free values after passing them to these APIs, including failure paths in shared cleanup labels. Target objects and containers are not consumed by these calls and still need normal cleanup unless ownership has been transferred by inserting them into another object. Also fix adjacent cleanup paths that leaked target objects after a setter consumed the value being attached. Reported by R4mbb of KRsecurity.
Previously, njs_chb_t accumulated bytes unbounded through the memory pool until the final string boundary (njs_string_create_chb()) enforced the NJS_STRING_MAX_LENGTH limit. For string-producing chains this let the pool grow to tens of GiB before the final alloc was refused, which on adversarial inputs (such as String.prototype.replace with a regex that matches the full string and a replacement containing thousands of $N references) led to process OOM instead of a catchable RangeError. The fix is to add two fields to njs_chb_t: total_size (O(1) running byte count) and max_size (optional upper bound, 0 means unlimited). Exceeding max_size puts the chain into a sticky NJS_CHB_ERR_OVERFLOW state. As part of the change njs_chb_drain() now returns early on a failed chain, in line with njs_chb_drop().
Overflow now trips during chain growth and surfaces as a catchable
RangeError("invalid string length") instead of continuing to inflate
the VM memory pool until the process OOMs.
Out of scope (non-JS-string chains):
- njs_vm.c AST serialization
- njs_json.c njs_vm_value_dump (console.log value dump)
- njs_function.c new Function() source assembly
- njs_error.c error message composition
- external/ modules on both engines (querystring, xml, zlib)
This fixes OOM in staging/sm/String/replace-math Tests262.
Buffer.prototype.toString(encoding, start, end) on the njs engine clamped start and end independently to the buffer length but never enforced start <= end. With start > end the unsigned subtraction end - start underflowed, the zero-length guard did not fire, and the encoder read past the buffer. Reject the start >= end case before computing the range, returning an empty string to match Node.js semantics. While here, add parallel missing functionality to QuickJS for start and end arguments for Buffer.prototype.toString().
Previously, ngx_js_dict_parse_entry() parsed numeric values with strtod((char *) p, &p), which has no end awareness. The state file loader allocated a buffer sized to the exact file length and passed end = buf + len, so a numeric token whose digits ran to the very end of the allocation (for example a truncated or tampered state file ending in '"value":123') let strtod() read past the buffer into adjacent pool memory. NUL-terminate the loaded buffer so strtod() stops at the file end.
Previously, for the JSON body type without a NUL terminator, ngx_http_qjs_body_to_value() round-tripped the raw body through qjs_string_create() (JS_NewStringLen) and JS_ToCString(), then parsed the result with JS_ParseJSON() using the original body_read_len. JS_NewStringLen() collapses invalid UTF-8 (a run of continuation bytes becomes a single U+FFFD), so the byte length of the C string no longer matches body_read_len. With a body containing such bytes the parser was handed the wrong length and a valid body was truncated and rejected as a SyntaxError. The fix is to use JS_ToCStringLen() to capture the actual C string length and pass it to JS_ParseJSON().
Previously, when dynamic proxy URL evaluation failed in ngx_qjs_fetch(), the error was returned directly, bypassing the fail path which deletes the registered fetch event and releases the promise. The njs counterpart already goes through the fail path. As a side effect, the error is now reported by rejecting the returned promise instead of throwing synchronously, consistently with the other failure paths.
Previously, the values of the buffer_size, max_response_body_size and verify init properties were not released after conversion, leaking heap values such as objects with a valueOf() method.
Previously, when the resolver context allocation failed, both engines returned without deleting the registered fetch event and reported the error with a synchronous throw instead of rejecting the returned promise, unlike all other failure paths.
Previously, ngx_js_http_build_connect_request() emitted the blank line terminating the request headers only when a proxy auth header was present. An HTTPS request through a proxy configured without credentials sent a CONNECT request with no terminating blank line, so the proxy kept waiting for more headers and the fetch timed out.
Previously, the reservation passed to njs_chb_sprintf() was too small for the maximum size_t output, so the header could be silently truncated for very large request bodies.
Reading a request header via r.headersIn[name] could crash the worker with SIGSEGV. The header in question was "Proxy-Connection", but any header that nginx registers without a dedicated ngx_table_elt_t * slot was affected. The getter mapped an arbitrary header name through ngx_hash_find() and treated (char *) &r->headers_in + hh->offset as a pointer to a cache slot. This only holds for headers whose parser stores into such a slot. "Proxy-Connection" is registered in ngx_http_headers_in[] with offset 0 and a handler that stores nothing, so the getter aliased the ngx_list_t at the start of ngx_http_headers_in_t and followed its elts pointer as a bogus ngx_table_elt_t. The fix is to walk the parsed header list by name, as it is always safe. The single-value and semicolon-join flags moved into the per-name header table, and the now-unused slot pointer argument and its branch were dropped from the generic getter. The slot lookup that is removed was only an optimization for the names in ngx_http_headers_in[]; for any other header it missed the hash and walked the list anyway, so dropping it is not a performance regression. This closes #1071 issue on Github.
Previously, njs_sort_indexed_properties() cached the fast array backing store pointer (array->start) once and then iterates. For a hole it performed a generic property lookup that walks the prototype chain and may invoke a user-defined getter. A getter that grows the same array triggers njs_array_expand(), which reallocates the backing store and frees the old buffer, leaving the cached pointer dangling. The remaining iterations then read freed memory, and the stale values are sorted and written back, exposing freed heap contents to script. Reported by Sangsoo Jeong (78ResearchLab).
Previously, since njs subrequests are background subrequests that nginx does not wake on completion, the handlers ngx_http_js_subrequest_done() and ngx_http_qjs_subrequest_done() woke the parent request themselves, synchronously, via ngx_http_run_posted_requests(), from within ngx_http_finalize_request() of the subrequest. When the downstream client had already aborted, that re-entered request processing and freed the request pool shared by all subrequests before finalization unwound; the freed subrequest was then dereferenced in ngx_http_post_action() (heap-use-after-free). This handler is unlike the other event completions. Timers and ngx.fetch() run their callbacks from their own event handlers, where draining posted requests is safe; the subrequest completion handler runs while the subrequest is still being finalized, where it is not. The wake helper is shared, but this caller must be treated differently. Before 0.8.1 the parent was woken by posting the request only; 0ee0184 (0.8.1) added a synchronous ngx_http_run_posted_requests(), which let an inner run free the request while an outer one was still unwinding. d34fcb0 (0.8.5) dropped the synchronous run and posted the connection write event instead, fixing a nested-run use-after-free when a subrequest callback threw; but that lost the wake when the handler runs as a subrequest of another module (lua), whose connection write handler is not ours. 75d6b61 (0.9.5) reverted to posting the request plus the synchronous run to fix the lost wake, reintroducing the use-after-free, now seen on a premature client close. The fix is to post the parent request without running posted requests in place, as it was done before 0.8.1. This closes #1077 issue on Github.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Proposed changes
Update from upstream
Checklist
Before creating a PR, run through this checklist and mark each as complete:
CONTRIBUTINGdocument