Skip to content

test: test against hash/sign/verify collisions on strings - #60384

Open
ChALkeR wants to merge 1 commit into
nodejs:mainfrom
ChALkeR:chalker/crypto/strings/0
Open

test: test against hash/sign/verify collisions on strings#60384
ChALkeR wants to merge 1 commit into
nodejs:mainfrom
ChALkeR:chalker/crypto/strings/0

Conversation

@ChALkeR

@ChALkeR ChALkeR commented Oct 24, 2025

Copy link
Copy Markdown
Member

See #60267

Equal hashes should mean equal input
Passing verify() should mean equal input to what was signed

Warning

This PR does not fix the issue, but provides a testcase against it

Fixing would require checking all string input against String.prototype.isWellFormed() or it's C++ alternative

It would be (at least technically) a semver-major change

I'm not yet sure if callback-style methods should produce a synchronous error or pass it to the callback

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Oct 24, 2025
@ChALkeR ChALkeR changed the title test: test against hash/verify collisions on strings test: test against hash/sign/verify collisions on strings Oct 24, 2025
@ChALkeR
ChALkeR force-pushed the chalker/crypto/strings/0 branch from 730bc66 to cae6903 Compare October 24, 2025 07:07
@codecov

codecov Bot commented Oct 24, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.58%. Comparing base (ddbe136) to head (47db204).
⚠️ Report is 2264 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #60384      +/-   ##
==========================================
- Coverage   88.59%   88.58%   -0.01%     
==========================================
  Files         704      704              
  Lines      207774   207774              
  Branches    40035    40030       -5     
==========================================
- Hits       184068   184058      -10     
- Misses      15748    15753       +5     
- Partials     7958     7963       +5     

see 24 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ChALkeR
ChALkeR force-pushed the chalker/crypto/strings/0 branch 8 times, most recently from 7f9d7ac to 1a73a6c Compare October 24, 2025 17:22
@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been marked as stale due to 90 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.

@github-actions github-actions Bot added the stale label Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. stale test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants