refactor(napi/parser, linter/plugins): improve safety of raw transfer interface#22424
Merged
graphite-app[bot] merged 1 commit intoMay 14, 2026
Conversation
This was referenced May 14, 2026
Member
Author
Contributor
There was a problem hiding this comment.
Pull request overview
Adds defensive runtime bounds checking on source_start/source_len (always) and full UTF-8 validation in debug builds for the raw transfer parser entry points in both napi/parser and Oxlint's JS plugin parser, hardening the safety contract of these unsafe fns.
Changes:
- Add
assert!(source_end <= ACTIVE_SIZE)in both raw-transfer parse implementations. - Replace unconditional
from_utf8_uncheckedwithcfg!(debug_assertions)-gated full UTF-8 validation, falling back to unchecked in release. - Hoist
source_start/source_endcomputation to a single place to avoid duplication.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| napi/parser/src/raw_transfer.rs | Adds active-region bounds assertion and debug-only UTF-8 validation around source slice access. |
| apps/oxlint/src/js_plugins/parse.rs | Mirrors the same defensive checks in Oxlint's JS plugin raw-transfer parser. |
62e44cc to
7e21af0
Compare
dd2abb0 to
3c4ebd0
Compare
3c4ebd0 to
fdd60fa
Compare
0f1fb87 to
eebf4df
Compare
fdd60fa to
ba20cd2
Compare
Contributor
Merge activity
|
… interface (#22424) Add a check that the input passed to raw transfer parser implementations is valid - both in `napi/parser` and in the raw transfer implementation used in Oxlint's `RuleTester`. Check that `source_start` and `source_len` which are passed from JS side are within bounds of the active region of the `Arena`. This check is not strictly necessary - it's part of the safety contract, and is enforced on JS side - but it's critical as the setup of the `Arena` on Rust side depends on the accuracy of these values. If they were wrong, it could lead to reading/writing out of bounds. The check is cheap, so it seems worthwhile - safety trumps the tiny perf impact here. The 3rd invariant of these functions is that the bytes in the buffer between `source_start` and `source_start + source_len` represent a valid UTF-8 string. Here a full runtime check isn't appropriate, as it would involve scanning and validating the whole source text string (expensive). But _do_ enable the full check in debug builds (used by raw transfer parser tests, and Oxlint conformance tests).
eebf4df to
c2c8f80
Compare
ba20cd2 to
e3b0d54
Compare
Base automatically changed from
om/05-13-refactor_napi_parser_raw_transfer_store_source_text_at_end_of_buffer
to
main
May 14, 2026 22:36
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a check that the input passed to raw transfer parser implementations is valid - both in
napi/parserand in the raw transfer implementation used in Oxlint'sRuleTester.Check that
source_startandsource_lenwhich are passed from JS side are within bounds of the active region of theArena. This check is not strictly necessary - it's part of the safety contract, and is enforced on JS side - but it's critical as the setup of theArenaon Rust side depends on the accuracy of these values. If they were wrong, it could lead to reading/writing out of bounds. The check is cheap, so it seems worthwhile - safety trumps the tiny perf impact here.The 3rd invariant of these functions is that the bytes in the buffer between
source_startandsource_start + source_lenrepresent a valid UTF-8 string. Here a full runtime check isn't appropriate, as it would involve scanning and validating the whole source text string (expensive). But do enable the full check in debug builds (used by raw transfer parser tests, and Oxlint conformance tests).