Skip to content

test: cover ECDH deriveBits length boundaries - #249

Open
harrshita123 wants to merge 4 commits into
google:masterfrom
harrshita123:issue-130-ecdh-derivebits-docs
Open

test: cover ECDH deriveBits length boundaries#249
harrshita123 wants to merge 4 commits into
google:masterfrom
harrshita123:issue-130-ecdh-derivebits-docs

Conversation

@harrshita123

@harrshita123 harrshita123 commented Feb 7, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #289.

This change adds explicit ECDH deriveBits boundary coverage for each supported curve:

  • exercises the exact maximum length in generated cases;
  • verifies the documented maximum succeeds; and
  • verifies the first invalid bit length is rejected.

The tested limits are 256 bits for P-256, 384 bits for P-384, and 528 bits for P-521.

Comment thread lib/src/webcrypto/webcrypto.ecdh.dart Outdated
Comment thread test/crypto_subtle_test.dart Outdated
@jonasfj

jonasfj commented Apr 24, 2026

Copy link
Copy Markdown
Member

Please rebase, I think CI mostly works on master branch now.

@harrshita123

Copy link
Copy Markdown
Contributor Author

Please rebase, I think CI mostly works on master branch now.

I have rebased the branch onto the latest master so the CI tests can run properly.

Comment thread test/crypto_subtle_test.dart Outdated
@harrshita123
harrshita123 force-pushed the issue-130-ecdh-derivebits-docs branch from 3797510 to 0c84b6e Compare June 30, 2026 18:33
@harrshita123

harrshita123 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

I updated the branch based on the feedback:

  • Rebased the PR onto the latest master, which resolved the merge conflict.
  • Kept the ECDH deriveBits documentation change focused on the maximum length for each curve.
  • Moved the new ECDH deriveBits boundary tests out of crypto_subtle_test.dart and into the package test runner under lib/src/testing/ecdh/derive_bits.dart.
  • Simplified the tests to use shared curve cases for P-256, P-384, and P-521.
  • Left the JSDomException behavior out of this PR since it was called out as a separate issue.

I also cleaned up the branch history into a single commit and verified it locally with dart format, dart analyze, and dart test test/webcrypto_test.dart -p vm.

@HamdaanAliQuatil HamdaanAliQuatil left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

these tests are still useful even though #130 was closed by #289. the generated cases added there don’t actually hit the documented limits: maxDeriveLength is a bit count, but those calls pass 32/48/66, and the random range excludes its upper bound.

could we correct those calls here by setting both minDeriveLength and maxDeriveLength to 256/384/528 respectively?

since #130 is already closed, I’d also reframe this as a follow-up to #289, replace Fixes #130, and perhaps rename the PR to test: cover ECDH deriveBits length boundaries.

Comment thread lib/src/testing/ecdh/derive_bits.dart Outdated
@harrshita123 harrshita123 changed the title Document and test maximum deriveBits length for ECDH curves test: cover ECDH deriveBits length boundaries Jul 24, 2026
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.

3 participants