fix: ensure safe text slicing boundaries with multi-byte characters #67
Conversation
This file contains 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
|
|
|
@dsherret Could you please review this PR? |
|
I think it would be really great if we could have a unit test for this :) |
dsherret
reviewed
Sep 8, 2024
| let char_indices: Vec<_> = text.char_indices().collect(); | ||
| let start_char_idx = char_indices.iter().position(|&(i, _)| i == start).unwrap_or(0); | ||
| let end_char_idx = char_indices.iter().position(|&(i, _)| i == end).unwrap_or(char_indices.len() - 1); | ||
| &text[char_indices[start_char_idx].0..char_indices[end_char_idx].0] |
There was a problem hiding this comment.
I think this might just be masking a bug. The range provided to this should be valid instead.
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.
This commit resolves a panic that occurs when the text_info function attempts to slice a string containing non-ASCII characters.
The fix addresses the issue reported in the Deno repository: denoland/deno#23875
Use Case:
The bug arises because the range_text method tries to slice a string using byte indices (start and end) that may not align with UTF-8 character boundaries. In Rust, attempting to slice a string at invalid UTF-8 boundaries results in a panic.
Solution:
The solution involves modifying the range_text method to correctly handle non-ASCII characters. The method now uses char_indices() to collect character boundaries and maps the start and end byte indices to these boundaries. The resulting string slice is based on valid UTF-8 character boundaries, ensuring the operation is safe and the encoding remains intact.