Conversation
…invalid mailbox get_invalid_mailbox() builds a 'misplaced-special' ValueTerminal for every stray special character but leaves its .defects empty, so the only record of the problem is the generic 'invalid address in address-list' (or 'invalid mailbox in mailbox-list') defect on the enclosing token list, which does not say which character was misplaced. Attach an InvalidHeaderDefect naming the character to the terminal itself. It is picked up by all_defects of every enclosing token, so defect lists for malformed address headers gain one entry per misplaced character; the three tests that spell those lists out are updated accordingly.
TokenList.all_defects built its result with sum(), which rebuilds the whole accumulated list once per child token, so gathering the defects of a parse tree costs time quadratic in the number of defects it holds. Until now that was mostly hidden, because the containers that collect many defects are rare; attaching a defect to every misplaced special character makes an ordinary malformed address header hit it. Parsing 'To: ' + '@'*32000 goes from 0.0154 to 1.08 seconds, and the pre-existing worst case, a long run of backslashes in an obs-local-part, already takes 4.45 seconds at the same size on main. Extend one list instead. Terminal.all_defects already returns a fresh copy, so returning a copy from an empty TokenList rather than the .defects list itself also makes the two agree.
Author
|
Closing this myself. @picnixz pointed out on #158152 that I have been opening too many at once and that old issues nobody has revived are the wrong ones to pick up unasked — this is one of those, so it should not be sitting in the queue. The branch stays on my fork if the issue ever comes back to life. |
Author
|
Reopened. Closing this was my own overreaction to being told I had too many open, not something anyone asked for. |
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.
Closes #66889.
Being upfront about one thing first: the original message on this issue did not survive the move from bugs.python.org — what is on GitHub is only the metadata block, so the title is all we have of what @bitdancer meant. I went by the title plus what the parser actually does today.
For
abc@xyz.c:omthe parser does report a defect, but only a generic one on the enclosing list:Nothing says what was wrong or where. The parser already knows: it builds a
misplaced-specialterminal for the stray:and then throws that knowledge away. This attaches anInvalidHeaderDefectto that terminal, so the character is named:One defect per misplaced character, which for a header with several bad addresses is the difference between "something is wrong somewhere" and a list you can point at.
There is a second, separate change in here, and it has its own news entry because it stands alone.
TokenList.all_defectswassum((x.all_defects for x in self), self.defects), which builds a new list at every node, so collecting defects is quadratic in their number. It now appends to one list. Measured on headers of 200, 800 and 3200 malformed addresses:Four times the input costs roughly ten times the collection on main and roughly four times here. The branch is also collecting twice as many defects, since it now reports the specific one too.
./python.exe -m test test_emailis 1,822 passing. RevertingLib/email/_header_value_parser.pywhile keeping the tests fails five of them.If the missing report @bitdancer had in mind was something other than this, say so and I will rework it — the title is genuinely all I had to go on.