bpo-34480: fix bug where match variable is used prior to being defined #17643
Conversation
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA). Recognized GitHub usernameWe couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames: This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. You can check yourself to see if the CLA has been received. Thanks again for the contribution, we look forward to reviewing it! |
|
The issue is reported under https://bugs.python.org/issue34480. You might want to prepend the issue number as per the required format to link this PR. The issue discusses the usage of self.error to raise an exception. This needs a test case. Something like creating a custom subclass of HTMLParser and feed it with "<![nonexistent>" that triggers this and the fix needs to be approved by core dev in the linked issue discussion. Thanks. |
|
@jkamdjou, thank you for your contribution. Please address @tirkarthi's comments from 2 weeks ago. If you're unable to update this PR, then we will need to close it. Thank you! |
|
Title updated with issue # (didn't see the issue, my apologies). Responded to the issue, awaiting core dev feedback. Thanks! |
| @@ -156,6 +156,7 @@ def parse_marked_section(self, i, report=1): | |||
| # look for MS Office ]> ending | |||
| match= _msmarkedsectionclose.search(rawdata, i+3) | |||
| else: | |||
| match = None | |||
There was a problem hiding this comment.
I've changed this branch to raise an AssertionError in #8562. Depending on @ezio-melotti's feedback, I can also change it to return -1. In both cases, this PR may not be needed so I'd suggest waiting for #8562 to be resolved first.
I think it would be nice to have a test case that hits to this branch.
There was a problem hiding this comment.
8562 has been merged, so I think this can be closed.
|
Closing as it looks like this is no longer needed after merging #8562. thanks @berkerpeksag @iritkatriel |
It doesn't seem to be fixed for me running Python 3.8.12 and beautifulsoup==4.10.0. Can you please clarify if the patch has only been applied on Python 3.9.0+? |
|
3.8 is only getting security fixes now. |
|
@iritkatriel got it, thanks for pointing that out. Just tested on Python 3.9.12 and the same error occurs, though. The changes in this PR suggest the issue was fixed in Python 3.9 but it's only fixed in Python 3.10 according to the changelog. We're unfortunately unable to upgrade since Apache Beam has only recently introduced support for Python 3.9. Any chance this bugfix could be backported to 3.9? |
|
Please comment on https://bugs.python.org/issue34480 to explain the current state of the issue. It will be a up to the 3.9 release manager (@ambv) to decide how to proceed. |
…defined (pythonGH-17643) (pythonGH-32256) Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com> Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Łukasz Langa <lukasz@langa.pl>
https://bugs.python.org/issue34480