Skip to content

Conversation

legendecas
Copy link
Member

@legendecas legendecas commented Aug 26, 2025

v8::String::WriteOneByteV2 now defaults to NO_NULL_TERMINATION. Call sites expect null termination should set flag String::WriteFlags::kNullTerminate.

String::REPLACE_INVALID_UTF8 is a no-op in WriteOneByteV2.

Fixes: #59555

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/http2
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Aug 26, 2025
Copy link

codecov bot commented Aug 26, 2025

Codecov Report

❌ Patch coverage is 95.65217% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.90%. Comparing base (886e4b3) to head (2249366).
⚠️ Report is 29 commits behind head on main.

Files with missing lines Patch % Lines
src/node_http_common-inl.h 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #59634      +/-   ##
==========================================
+ Coverage   89.85%   89.90%   +0.04%     
==========================================
  Files         667      667              
  Lines      196260   196606     +346     
  Branches    38559    38595      +36     
==========================================
+ Hits       176341   176750     +409     
+ Misses      12368    12313      -55     
+ Partials     7551     7543       -8     
Files with missing lines Coverage Δ
src/node_buffer.cc 68.83% <100.00%> (ø)
src/node_http2.cc 81.94% <100.00%> (+0.09%) ⬆️
src/string_bytes.cc 70.51% <100.00%> (+0.09%) ⬆️
src/node_http_common-inl.h 67.74% <0.00%> (ø)

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

@legendecas legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 26, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 26, 2025
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@Flarna
Copy link
Member

Flarna commented Aug 26, 2025

Looks like some tests crash because of this changes.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

@legendecas
Copy link
Member Author

@Flarna: Looks like some tests crash because of this changes.

Updated call sites that expect loose convertion to onte bytes, and fixed debug checks.

@Flarna Flarna added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 29, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 29, 2025
@nodejs-github-bot nodejs-github-bot merged commit 5af0355 into nodejs:main Aug 29, 2025
56 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 5af0355

@legendecas legendecas deleted the v8-writeontebytev2 branch August 30, 2025 10:49
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++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v8::String::WriteOneByte is deprecated
7 participants