Skip to content

Fix panic in unit parsing with non-UTF8 code#16355

Merged
sholderbach merged 2 commits intonushell:mainfrom
sholderbach:fix-literal-parse-panic
Aug 5, 2025
Merged

Fix panic in unit parsing with non-UTF8 code#16355
sholderbach merged 2 commits intonushell:mainfrom
sholderbach:fix-literal-parse-panic

Conversation

@sholderbach
Copy link
Copy Markdown
Member

Description

Trying to parse non-UTF8 data as a value with unit (part of every
literal parse) introduced a replacement character which shifted the span
information so the indices where incorrect and triggered
a panic.

This has been resolved by removing a bad String::from_utf8_lossy

User-Facing Changes

One less possible panic

Tests + Formatting

Added a test with the original reproducer from fuzzing:

File with 0\xffB where the \xff represents the non utf-8 char FF
run as a script to trigger

Trying to parse non-UTF8 data as a value with unit (part of every
literal parse) introduced a replacement character which shifted the span
information so the indices where incorrect and triggered
a panic.

This has been resolved by removing a bad `String::from_utf8_lossy`
@github-actions github-actions bot added the A:parser Issues related to parsing label Aug 5, 2025
@sholderbach sholderbach added the notes:fixes Include the release notes summary in the "Bug fixes" section label Aug 5, 2025
@sholderbach sholderbach merged commit 3a82c6c into nushell:main Aug 5, 2025
16 checks passed
@sholderbach sholderbach deleted the fix-literal-parse-panic branch August 5, 2025 20:08
@github-actions github-actions bot added this to the v0.107.0 milestone Aug 5, 2025
@WindSoilder WindSoilder added notes:mention Include the release notes summary in the "Hall of Fame" section notes:ready The "Release notes summary" section of this PR is ready to be included in our release notes. and removed notes:fixes Include the release notes summary in the "Bug fixes" section labels Aug 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A:parser Issues related to parsing notes:mention Include the release notes summary in the "Hall of Fame" section notes:ready The "Release notes summary" section of this PR is ready to be included in our release notes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants