Skip to content

Conversation

@copybara-service
Copy link

Optimized and refined bounds checking for delimited fields.

Prior to this CL, whenever we parsed a size we would immediately verify that size against the current EpsCopyInputStream limit. This was correct but was unnecessarily expensive, especially since it was redundant with other checks we perform when parsing sub-messages or strings.

In this CL, we remove the eager check when parsing a size, and we perform more targeted and optimized bounds checks at the points where these checks are actually needed. This brings us more into line with how EpsCopyInputStream works in C++.

In particular:

  • We never need to check against the pushed limit, because IsDone() is already checking to make sure we ended at the correct limit.
  • When reading a string (either to alias or to copy), we only need to check against the buffer end, not the limit. This could result in us reading a string that extends beyond the current limit, but the parse will then fail, and the string fields will still be safe to read (even if nonsense).

Prior to this CL, whenever we parsed a size we would immediately verify that size against the current EpsCopyInputStream limit.  This was correct but was unnecessarily expensive, especially since it was redundant with other checks we perform when parsing sub-messages or strings.

In this CL, we remove the eager check when parsing a size, and we perform more targeted and optimized bounds checks at the points where these checks are actually needed.  This brings us more into line with how EpsCopyInputStream works in C++.

In particular:

- We never need to check against the pushed limit, because `IsDone()` is already checking to make sure we ended at the correct limit.
- When reading a string (either to alias or to copy), we only need to check against the buffer end, not the limit.  This could result in us reading a string that extends beyond the current limit, but the parse will then fail, and the string fields will still be safe to read (even if nonsense).

PiperOrigin-RevId: 826744003
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