Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix panic in DecodeStream::step due to incorrect index usage #1699

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

n0gu-furiosa
Copy link

When calling DecodeStream::step multiple times, it eventually panics with attempt to subtract with overflow in the following lines of code:

let new_prefix_index = ids.len() - *prefix_index;
*ids = ids.drain(*read_index..).collect();

The panic can be easily reproduced, and I have added a test case to demonstrate the issue.

Upon inspecting the code, I found that the shrinking of the token buffer references read_index instead of prefix_index. This PR corrects the issue by using the correct index.

However, this change makes read_index unused, so I am not entirely certain if it aligns with the intended logic of the original implementation. Please let me know if further adjustments or clarifications are needed, or if there is additional context regarding the intended use of read_index.

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.

1 participant