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

Increase buffer token size as needed #1230

Merged
merged 4 commits into from
Mar 25, 2025
Merged

Conversation

denisonbarbosa
Copy link
Member

Some users had issues with GPOs with large entries overflowing the buffer and causing the update to fail. This fixes that by progressively increasing the expected readable token size until the maximum we can (the bufio package limits this).

UDENG-6434
Closes #1191

@denisonbarbosa denisonbarbosa marked this pull request as ready for review March 21, 2025 10:42
@denisonbarbosa denisonbarbosa requested a review from a team as a code owner March 21, 2025 10:42
@denisonbarbosa denisonbarbosa force-pushed the increase-gpo-buffer branch 3 times, most recently from ecb1af8 to b55492b Compare March 24, 2025 13:17
@denisonbarbosa denisonbarbosa requested a review from adombeck March 24, 2025 13:47
@denisonbarbosa denisonbarbosa force-pushed the increase-gpo-buffer branch 2 times, most recently from 23b954d to 293c7fe Compare March 25, 2025 11:13
@denisonbarbosa denisonbarbosa requested a review from adombeck March 25, 2025 11:28
@adombeck
Copy link
Contributor

I've been running the fuzz test on my machine for 30 minutes now and it didn't crash yet, so it seems like your fix worked.

Copy link
Member

@didrocks didrocks left a comment

Choose a reason for hiding this comment

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

Nice work! We are now capable of handling more use cases and the implementation is elegant. Thanks you both for the suggestions and keeping the fuzz test!

In order to be able to resize the buffer later, we need to have control
over the file cursor otherwise it would move forward and we would not be
able to retry reading the large entry that failed due to lack of buffer
size.
Although the token size is already quite large by default, we might
end up in a situation where it's still not big enough, so we need to
address this by increasing its size if we find one that's too large.
@denisonbarbosa denisonbarbosa merged commit ccc9b83 into main Mar 25, 2025
5 checks passed
@denisonbarbosa denisonbarbosa deleted the increase-gpo-buffer branch March 25, 2025 14:32
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.

Issue: go bufio.scanner GPO size too large
3 participants