Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QuickJS: fixed alg may be used uninitialized in SubtleCrypto.import(). #855

Merged
merged 5 commits into from
Feb 19, 2025

Conversation

xeioex
Copy link
Contributor

@xeioex xeioex commented Feb 12, 2025

Collecting coverage for WebCrypto module:

make clean; ./configure  --cc-opt='-fprofile-arcs -ftest-coverage -O0 -g -I/path/to/quickjs' --ld-opt='-L/path/to/quickjs -lgcov' && make njs
test/test262 --binary='build/njs -n QuickJS -m' ./test/webcrypto/

gcov -o build/external/ -b -a external/qjs_webcrypto_module.c
Lines executed:70.19% of 2221
Branches executed:95.50% of 1244
Taken at least once:64.07% of 1244
Calls executed:75.27% of 1100
Creating 'qjs_webcrypto_module.c.gcov'
...

qjs_webcrypto_module.c.gcov # contains line by line coverage in text form
File 'external/qjs_webcrypto_module.c'
-Lines executed:67.90% of 2221
-Branches executed:94.70% of 1246
-Taken at least once:61.32% of 1246
-Calls executed:72.73% of 1100
+Lines executed:70.19% of 2221
+Branches executed:95.50% of 1244
+Taken at least once:64.07% of 1244
+Calls executed:75.27% of 1100

@xeioex xeioex force-pushed the fix_webcrypto_alg branch 5 times, most recently from ea5e7f9 to ef76495 Compare February 13, 2025 08:00
@xeioex xeioex requested a review from hongzhidao February 13, 2025 08:04
@xeioex xeioex marked this pull request as ready for review February 13, 2025 08:07
@xeioex xeioex requested review from VadimZhestikov and removed request for hongzhidao February 15, 2025 00:43
Found by GCC:
In function ‘qjs_import_jwk_oct’,
external/qjs_webcrypto_module.c:3116:13: error: ‘alg.start’ may be used uninitialized [-Werror=maybe-uninitialized]
 3116 |             JS_ThrowTypeError(cx, "key size and \"alg\" value \"%s\" mismatch",

The similar place in the NJS module was also fixed.
When "%*s" is specified, the first integer is interpreted as width.
Width specifies *minimum* number of characters to output. The next
string is expected to be NULL-terminated.

When "%.*s" is specified, the first integer is interpreted as precision.
Precision specifies *maximum* number of characters to output.
Import related tests are consolidated from other WebCrypto tests.
Copy link
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 xeioex merged commit 54f18a1 into nginx:master Feb 19, 2025
1 check passed
@xeioex xeioex deleted the fix_webcrypto_alg branch February 19, 2025 00:30
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.

2 participants