Skip to content

Conversation

@mcollina
Copy link
Member

Add missing input validation in FastCRC32 to match the slow path.

Refs: #60538

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib subsystem. labels Dec 25, 2025
@mcollina mcollina marked this pull request as ready for review December 25, 2025 23:36
@codecov
Copy link

codecov bot commented Dec 26, 2025

Codecov Report

❌ Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.51%. Comparing base (4f24aff) to head (958ba9c).
⚠️ Report is 54 commits behind head on main.

Files with missing lines Patch % Lines
src/node_zlib.cc 0.00% 4 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #61175      +/-   ##
==========================================
- Coverage   88.53%   88.51%   -0.02%     
==========================================
  Files         703      703              
  Lines      208546   208590      +44     
  Branches    40217    40226       +9     
==========================================
- Hits       184634   184632       -2     
- Misses      15926    15963      +37     
- Partials     7986     7995       +9     
Files with missing lines Coverage Δ
src/node_zlib.cc 77.50% <0.00%> (-0.37%) ⬇️

... and 57 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.

v8::HandleScope handle_scope(options.isolate);
if (!data->IsArrayBufferView() && !data->IsString()) {
TRACK_V8_FAST_API_CALL("zlib.crc32.error");
THROW_ERR_INVALID_ARG_TYPE(
Copy link
Member

Choose a reason for hiding this comment

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

This is already validated in the JS layer:

node/lib/zlib.js

Lines 949 to 955 in 42d0e13

function crc32(data, value = 0) {
if (typeof data !== 'string' && !isArrayBufferView(data)) {
throw new ERR_INVALID_ARG_TYPE('data', ['Buffer', 'TypedArray', 'DataView', 'string'], data);
}
validateUint32(value, 'value');
return crc32Native(data, value);
}

This can just be a CHECK, as the slow callback does already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants