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

WebCrypto: HDKF and PBKDF2 return an empty ArrayBuffer when length is zero #49048

Merged

Conversation

javifernandez
Copy link
Contributor

The PR#380 [1] changed the PBKDF2 deriveBits operation to allow zero length and introduced an additional step to return an empty string in that case. It also reversted the PR#275 [2] so that HKDF also handles the zero length in the same way.

This PR updates the tests cases affecting this 2 algorithms on the cases where zero was passed in the length parameter.

[1] w3c/webcrypto#380
[2] w3c/webcrypto#275

@wpt-pr-bot wpt-pr-bot requested a review from twiss November 8, 2024 00:31
@javifernandez javifernandez changed the title HDKF and PBKDF2 returns an empty string when length is zero HDKF and PBKDF2 return an empty string when length is zero Nov 8, 2024
@javifernandez javifernandez force-pushed the hkdf-and-pbkdf2-zero-length branch from 980f750 to 45204d9 Compare November 8, 2024 00:33
The PR#380 [1] changed the PBKDF2 deriveBits operation to allow
zero length and introduced an additional step to return an empty
string in that case. It also reversted the PR#275 [2] so that
HKDF also handles the zero length in the same way.

This PR updates the tests cases affecting this 2 algorithms on
the cases where zero was passed in the length parameter.

[1] w3c/webcrypto#380
[2] w3c/webcrypto#275
@javifernandez javifernandez force-pushed the hkdf-and-pbkdf2-zero-length branch from 45204d9 to 194773e Compare November 8, 2024 10:51
@javifernandez javifernandez requested a review from panva November 8, 2024 10:52
Copy link
Contributor

@panva panva left a comment

Choose a reason for hiding this comment

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

lgtm (this PR, the change upstream is a different story)...

Copy link
Member

@twiss twiss left a comment

Choose a reason for hiding this comment

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

LGTM 👍

However, @javifernandez do you know why the pipelines failed, or who to ask? Seems like installing Safari Technology Preview hit a 403 here: https://dev.azure.com/web-platform-tests/wpt/_build/results?buildId=129776&view=logs&j=0a19af81-f788-555d-f214-a5e35905eaf5&t=84b058e1-3bc1-5dbb-06da-ea0f1018bbe7

@javifernandez
Copy link
Contributor Author

LGTM 👍

However, @javifernandez do you know why the pipelines failed, or who to ask? Seems like installing Safari Technology Preview hit a 403 here: https://dev.azure.com/web-platform-tests/wpt/_build/results?buildId=129776&view=logs&j=0a19af81-f788-555d-f214-a5e35905eaf5&t=84b058e1-3bc1-5dbb-06da-ea0f1018bbe7

I'll try to get some information about it.

@javifernandez
Copy link
Contributor Author

LGTM 👍
However, @javifernandez do you know why the pipelines failed, or who to ask? Seems like installing Safari Technology Preview hit a 403 here: https://dev.azure.com/web-platform-tests/wpt/_build/results?buildId=129776&view=logs&j=0a19af81-f788-555d-f214-a5e35905eaf5&t=84b058e1-3bc1-5dbb-06da-ea0f1018bbe7

I'll try to get some information about it.

I have contacted some folks at Apple involving on the WPT support; it seems there was a problem with the STP downloads, but the infra issue should be solved now.

I think we can merge this now, but let me now if you think we should re-trigger the checks.

@twiss twiss merged commit b818311 into web-platform-tests:master Nov 11, 2024
19 checks passed
@twiss twiss changed the title HDKF and PBKDF2 return an empty string when length is zero WebCrypto: HDKF and PBKDF2 return an empty ArrayBuffer when length is zero Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants