{{ message }}
Repository navigation
Fix the IndexError message of the pure Python UnknownFieldSet - #30445
Open
ManoharPaturi wants to merge 1 commit into
Open
ManoharPaturi wants to merge 1 commit into
ManoharPaturi wants to merge 1 commit into
Conversation
The bounds check in UnknownFieldSet.__getitem__ formatted its message with '...'.index, which evaluates to the bound method str.index instead of the requested index, so out of range lookups surfaced a message like "index <built-in method index of str object at 0x...> out of range". Use % formatting and add a test asserting the rendered message.
sbenzaquen
added
the
🅰️ safe for tests
github-actions
Bot
removed
the
🅰️ safe for tests
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
The bounds check in UnknownFieldSet.getitem in python/google/protobuf/unknown_fields.py built its error message with string attribute access rather than formatting:
Because '.index' evaluates to the bound method str.index of the literal, an out of range lookup surfaced a message such as
instead of naming the index. The fix is the intended % formatting with the index value.
Testing: added testIndexErrorMessages to python/google/protobuf/internal/unknown_fields_test.py, which performs lookups at negative and positive out of range offsets and asserts the rendered message. Under the pure Python implementation the message now reads exactly "index N out of range" with the wrapped value for negative offsets; under the C++ implementation the test only asserts that an IndexError mentioning "out of range" is raised, since that side formats its own message. The full unknown_fields_test.py suite passes under PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=python (23 tests locally).