Skip to content

Modules: fixed loading of the built-in "crypto" module.#1050

Merged
xeioex merged 3 commits intonginx:masterfrom
xeioex:fix_crypto_module
Apr 23, 2026
Merged

Modules: fixed loading of the built-in "crypto" module.#1050
xeioex merged 3 commits intonginx:masterfrom
xeioex:fix_crypto_module

Conversation

@xeioex
Copy link
Copy Markdown
Contributor

@xeioex xeioex commented Apr 22, 2026

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.

@xeioex xeioex requested a review from VadimZhestikov April 22, 2026 22:46
@xeioex xeioex force-pushed the fix_crypto_module branch from 4aec16a to 7195af7 Compare April 22, 2026 22:57
@VadimZhestikov
Copy link
Copy Markdown
Contributor

QJS addon order (fetch_module <--> shared_dict_module) swap. Is it intentional?

Neither test covers the QJS (qjs_import) path. Given the fix also adds qjs_crypto_module, a QJS-variant test (or a note explaining it's already covered elsewhere) would be reassuring.

@xeioex
Copy link
Copy Markdown
Contributor Author

xeioex commented Apr 23, 2026

@VadimZhestikov

Neither test covers the QJS (qjs_import) path. Given the fix also adds qjs_crypto_module, a QJS-variant test (or a note explaining it's already covered elsewhere) would be reassuring.

js_crypto.t passes for both engines. or what do you mean?

rm -fr /tmp/nginx-test*;  TEST_NGINX_GLOBALS_HTTP="js_engine qjs;" TEST_NGINX_LEAVE=1 TEST_NGINX_BINARY=/home/xeioex/workspace/nginx/nginx/objs/nginx prove -I /home/xeioex/workspace/nginx/nginx-tests/lib/ nginx/t/js_crypto.t
nginx/t/js_crypto.t .. ok

xeioex added 2 commits April 22, 2026 17:18
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 nginx#1049 issue on Github.
@xeioex xeioex force-pushed the fix_crypto_module branch from 7195af7 to f8e88e6 Compare April 23, 2026 00:18
Copy link
Copy Markdown
Contributor

@VadimZhestikov VadimZhestikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@xeioex
Copy link
Copy Markdown
Contributor Author

xeioex commented Apr 23, 2026

QJS addon order (fetch_module <--> shared_dict_module) swap. Is it intentional?

I restored it back for consistency.

@xeioex xeioex changed the title Fix crypto module Modules: fixed loading of the built-in "crypto" module. Apr 23, 2026
@xeioex xeioex merged commit ab6f49e into nginx:master Apr 23, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[0.9.7] Crypto no longer working in nginx

2 participants