gh-148518 fix index error in local part attribute#148522
Open
bitdancer wants to merge 5 commits intopython:mainfrom
Open
gh-148518 fix index error in local part attribute#148522bitdancer wants to merge 5 commits intopython:mainfrom
bitdancer wants to merge 5 commits intopython:mainfrom
Conversation
As part of fixing bpo-27931 code was introduced to get_bar_quoted_string that added an empty Terminal if the quoted string was empty. This isn't the best answer in terms of the parse tree, we really want the token list to be empty in that case. But having it be empty would result in local_part raising the index error. Which is the same bug we find if we try to parse an address consisting of a single dquote. So by fixing local_part to not raise in that case, we can have the bare_quoted_string code correctly return an empty token list for the empty string cases (two dquotes or a single dquote as the entire addrespec, at the end of a line). After this commit there will be two test failures instead of just one, and we'll fix both with the fix to local_part.
I'm not sure this is worth a news item, though.
bitdancer
changed the title
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
1 participant
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.
Add failing test.
Correct imperfect fix for variation of this bug.
As part of fixing bpo-27931 code was introduced to get_bar_quoted_string
that added an empty Terminal if the quoted string was empty. This isn't
the best answer in terms of the parse tree, we really want the token
list to be empty in that case. But having it be empty would result in
local_part raising the index error. Which is the same bug we find if we
try to parse an address consisting of a single dquote. So by fixing
local_part to not raise in that case, we can have the bare_quoted_string
code correctly return an empty token list for the empty string cases
(two dquotes or a single dquote as the entire addrespec, at the end of a
line).
After this commit there will be two test failures instead of just one,
and we'll fix both with the fix to local_part.
Fix the bug.
Add news item.
I'm not sure this is worth a news item, though.