GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-34480: fix bug where match variable is used prior to being defined by jkamdjou · Pull Request #17643 · python/cpython

Conversation

@jkamdjou

@jkamdjou jkamdjou commented Dec 17, 2019 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

https://bugs.python.org/issue34480

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@jkamdjou
fix bug where match variable is used prior to being defined
6375163
@jkamdjou
jkamdjou requested a review from ezio-melotti as a code owner December 17, 2019 19:23
@the-knights-who-say-ni the-knights-who-say-ni added the CLA not signed label Dec 17, 2019
@the-knights-who-say-ni

the-knights-who-say-ni commented Dec 17, 2019

Copy link
Copy Markdown

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 username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@jkamdjou

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!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added the awaiting review label Dec 17, 2019
@jkamdjou jkamdjou closed this Dec 17, 2019
@jkamdjou jkamdjou reopened this Dec 17, 2019
@tirkarthi

tirkarthi commented Dec 18, 2019

Copy link
Copy Markdown
Member

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@csabella

csabella commented Dec 31, 2019

Copy link
Copy Markdown
Contributor

@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!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@csabella csabella added awaiting changes and removed awaiting review labels Dec 31, 2019
@jkamdjou jkamdjou changed the title fix bug where match variable is used prior to being defined bpo-34480: fix bug where match variable is used prior to being defined Jan 4, 2020
@jkamdjou

jkamdjou commented Jan 4, 2020

Copy link
Copy Markdown
Author

Title updated with issue # (didn't see the issue, my apologies). Responded to the issue, awaiting core dev feedback. Thanks!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@csabella csabella removed the CLA not signed label Jan 5, 2020
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Jan 5, 2020
@csabella

csabella commented Jan 12, 2020

Copy link
Copy Markdown
Contributor

@jkamdjou, thank you for updating the PR. Based on the discussion on the pull request, please add tests here. Ezio said that both pull requests (this one and PR 8562) are needed, so one doesn't supercede the other. Thank you!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
berkerpeksag
berkerpeksag reviewed Feb 24, 2020
View reviewed changes
Comment thread Lib/_markupbase.py
# look for MS Office ]> ending
match= _msmarkedsectionclose.search(rawdata, i+3)
else:
match = None

@berkerpeksag berkerpeksag Feb 24, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@iritkatriel iritkatriel May 21, 2021

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

8562 has been merged, so I think this can be closed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@jkamdjou

jkamdjou commented Nov 22, 2021

Copy link
Copy Markdown
Author

Closing as it looks like this is no longer needed after merging #8562. thanks @berkerpeksag @iritkatriel
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jkamdjou jkamdjou closed this Nov 22, 2021
@mareksuscak

mareksuscak commented Mar 27, 2022

Copy link
Copy Markdown

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+?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@iritkatriel

iritkatriel commented Mar 27, 2022

Copy link
Copy Markdown
Member

3.8 is only getting security fixes now.
AlexWaygood and mareksuscak reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mareksuscak

mareksuscak commented Mar 27, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

@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?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@iritkatriel

iritkatriel commented Mar 27, 2022

Copy link
Copy Markdown
Member

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.
mareksuscak reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mareksuscak mareksuscak mentioned this pull request Apr 2, 2022
[3.9] bpo-34480: fix bug where match variable is used prior to being defined (GH-17643) #32256
Merged
ambv added a commit that referenced this pull request May 16, 2022
@mareksuscak @ezio-melotti
@blurb-it @ambv
[3.9] bpo-34480: fix bug where match variable is used prior to being …
518b238 
…defined (GH-17643) (GH-32256)

Co-authored-by: Ezio Melotti <[email protected]>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Łukasz Langa <[email protected]>
hello-adam pushed a commit to hello-adam/cpython that referenced this pull request Jun 2, 2022
@mareksuscak @ezio-melotti
@blurb-it @ambv @hello-adam
[3.9] bpo-34480: fix bug where match variable is used prior to being …
180748b 
…defined (pythonGH-17643) (pythonGH-32256)

Co-authored-by: Ezio Melotti <[email protected]>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Łukasz Langa <[email protected]>
@kodial kodial mannequin mentioned this pull request Sep 9, 2022
_markupbase.py fails with UnboundLocalError on invalid keyword in marked section #78661
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@berkerpeksag berkerpeksag berkerpeksag left review comments
@iritkatriel iritkatriel iritkatriel left review comments
@ezio-melotti ezio-melotti Awaiting requested review from ezio-melotti ezio-melotti is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

8 participants

@jkamdjou @the-knights-who-say-ni @tirkarthi @csabella @mareksuscak @iritkatriel @berkerpeksag @bedevere-bot
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.