Skip to content

fix: replace unsafe byte indexing in parse_value with .get() + error return#45

Merged
nathanjmcdougall merged 2 commits into
mainfrom
44-bug-unsafe-byte-indexing-in-parse_value-can-panic-instead-of-returning-error
May 25, 2026
Merged

fix: replace unsafe byte indexing in parse_value with .get() + error return#45
nathanjmcdougall merged 2 commits into
mainfrom
44-bug-unsafe-byte-indexing-in-parse_value-can-panic-instead-of-returning-error

Conversation

@nathanjmcdougall
Copy link
Copy Markdown
Collaborator

Direct byte indexing (source[span.0..span.1] and source[..span.0]) panics if yamlpath returns a span misaligned to UTF-8 boundaries, crashing the Python process. Replace with .get() to return a recoverable PyValueError, matching the existing safe pattern in extract().

Closes #44

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR prevents a potential Rust panic (and resulting Python process crash) in PyDocument::parse_value by replacing direct UTF-8 string byte slicing with safe .get(..) range access that returns a recoverable PyValueError when spans are invalid.

Changes:

  • Replaced &source[span.0..span.1] with source.get(span.0..span.1).ok_or_else(...)? to avoid panics on invalid UTF-8 boundaries/out-of-range spans.
  • Replaced source[..span.0] with a safe .get(..span.0)-based prefix extraction before computing line_start.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/document.rs Outdated
Comment thread src/document.rs Outdated
@nathanjmcdougall nathanjmcdougall force-pushed the 44-bug-unsafe-byte-indexing-in-parse_value-can-panic-instead-of-returning-error branch 2 times, most recently from bfc7d81 to dbaec3d Compare May 25, 2026 07:58
@nathanjmcdougall nathanjmcdougall requested a review from Copilot May 25, 2026 07:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

Comment thread src/document.rs
Comment thread src/document.rs
Comment thread src/document.rs
Direct byte indexing (source[span.0..span.1] and source[..span.0]) panics
if yamlpath returns a span misaligned to UTF-8 boundaries, crashing the
Python process. Replace with upfront is_char_boundary + bounds checks that
return a recoverable error, then use direct indexing safely.

Applied consistently to all 5 sites in document.rs: parse_value,
apply_insert_at, and apply_complex_replace.

Closes #44
@nathanjmcdougall nathanjmcdougall force-pushed the 44-bug-unsafe-byte-indexing-in-parse_value-can-panic-instead-of-returning-error branch from dbaec3d to 74cdff7 Compare May 25, 2026 08:22
…dexing-in-parse_value-can-panic-instead-of-returning-error

# Conflicts:
#	src/document.rs
@nathanjmcdougall nathanjmcdougall merged commit d4ad092 into main May 25, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: unsafe byte indexing in parse_value can panic instead of returning error

2 participants