Fix out-of-bounds panic in Path::path_with_height when next_i == self.path.len() #124
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.
Summary
This PR fixes a potential out-of-bounds panic in
Path::path_with_heightthat can occur when the path tightly follows polygon edges or corners. In certain cases, thenext_iindex can become equal toself.path.len()after the last waypoint is processed, but before the method exits. The next line then tries to accessself.path[next_i], causing an index-out-of-bounds panic. I came across with problem while usingpolyanyafor a game navigation system.Details
The issue stems from this condition:
When
next_i == self.path.len(), the comparison is false (len() < next_i), so execution continues andself.path[next_i]is accessed, which is invalid.Changing the check to
next_i >= self.path.len()ensures we break out safely once all waypoints have been consumed.Fix
Impact
Prevents runtime panics when computing heighted paths.
No API or behavioral changes except improved stability.
Verified in downstream projects using polyanya for Recast navmesh height sampling (previously mentioned game navigation).
Additional Notes
The existing comment
// TODO: shouldn't happensuggests the author already anticipated this case; this patch simply codifies the intended safeguard.