GeezerVison

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

GeezerVison is showing github.com — gh-123378: fix a crash in `UnicodeError.__str__` by picnixz · Pull Request #124935 · python/cpython

Conversation

picnixz commented Oct 3, 2024 •

edited by bedevere-app Bot

Loading

Uh oh!

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

Copy link

Copy Markdown

Member

cc @encukou @serhiy-storchaka @vstinner

Issue: Incorrect handling of negative start values on PyUnicodeErrorObject #123378

Sorry, something went wrong.

Uh oh!

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

All reactions

picnixz added 3 commits

October 3, 2024 14:40


          fix OOB in UnicodeError.__str__

a9b1c0c


          blurb


          add tests

43141c0

picnixz requested a review from iritkatriel as a code owner

October 3, 2024 12:51

bedevere-app Bot added the awaiting review label

Oct 3, 2024

bedevere-app Bot mentioned this pull request

Oct 3, 2024

Incorrect handling of negative start values on PyUnicodeErrorObject #123378

Closed


          small fixup

0c61375

picnixz mentioned this pull request

Oct 3, 2024

gh-123378: fix some corner cases of start and end values in PyUnicodeErrorObject #123380

Merged


          Fix Windows compilation

80287f6

encukou reviewed

Oct 4, 2024

View reviewed changes

Comment thread

Objects/exceptions.c Resolved

Uh oh!

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

picnixz added 3 commits

October 4, 2024 14:10


          improve test coverage

7bbc0f5


          fix UnicodeTranslateError

2313cd1


          improve test coverage

a27a88b

picnixz changed the title ~~gh-123378: fix a crash in Unicode{Encode,Decode}Error.__str__~~ gh-123378: fix a crash in UnicodeError.__str__

Oct 4, 2024


          revert cosmetic changes

360c1a5

picnixz force-pushed the fix/unicode-error-str-123378 branch from 5ca7f92 to 360c1a5 Compare

October 5, 2024 10:28

picnixz requested a review from encukou

October 5, 2024 10:37

encukou approved these changes

Oct 7, 2024

View reviewed changes

encukou left a comment

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.

This looks good, thank you!

Sorry, something went wrong.

Uh oh!

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

All reactions

bedevere-app Bot added awaiting merge and removed awaiting review labels

Oct 7, 2024


          add back PEP-7 braces

85199f8

picnixz requested a review from encukou

October 7, 2024 14:40

vstinner reviewed

Oct 7, 2024

View reviewed changes

Comment thread

Lib/test/test_exceptions.py Outdated


		def test_unicode_error_str_gh_123378(self):
		for formatter, start, end, obj in product(
		(str, repr),

vstinner Oct 7, 2024

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.

Testing repr() if we already test str() sounds redundant to me. Is it really worth it?

Sorry, something went wrong.

Uh oh!

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

All reactions

picnixz Oct 8, 2024

Copy link

Copy Markdown

Member Author

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.

No you're right. I'll remove it.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

Lib/test/test_exceptions.py Outdated

		for klass in klasses:
		self.assertEqual(str(klass.__new__(klass)), "")

		def test_unicode_error_str_gh_123378(self):

vstinner Oct 7, 2024

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.

The purpose of the test is unclear to me, it just calls str(). Please add a comment to explain that you check that str() doesn't crash with a reference to the issue.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

Lib/test/test_exceptions.py Outdated

		(str, repr),
		range(-5, 5),
		range(-5, 5),
		('', 'a', '123', '1234', '12345', 'abc123'),

vstinner Oct 7, 2024

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.

Suggested change

	('', 'a', '123', '1234', '12345', 'abc123'),
	('', 'a', 'abc', 'abcde', 'abcdef'),

Sorry, something went wrong.

Uh oh!

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

picnixz reacted with thumbs up emoji

All reactions

👍 1 reaction


          address Victor's review

44dfaeb

picnixz requested a review from vstinner

October 8, 2024 09:07

vstinner merged commit ba14dfa into python:main

Oct 8, 2024

bedevere-app Bot removed the awaiting merge label

Oct 8, 2024

vstinner added needs backport to 3.12

only security fixes

needs backport to 3.13 only security fixes labels

Oct 8, 2024

miss-islington-app Bot commented Oct 8, 2024

Copy link

Copy Markdown

Thanks @picnixz for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

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

miss-islington-app Bot commented Oct 8, 2024

Copy link

Copy Markdown

Thanks @picnixz for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request

Oct 8, 2024


          pythongh-123378: fix a crash in UnicodeError.__str__ (pythonGH-124935)

f1a3817

(cherry picked from commit ba14dfa)

Co-authored-by: Bénédikt Tran <[email protected]>

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request

Oct 8, 2024


          pythongh-123378: fix a crash in UnicodeError.__str__ (pythonGH-124935)

b928a8b

(cherry picked from commit ba14dfa)

Co-authored-by: Bénédikt Tran <[email protected]>

bedevere-app Bot commented Oct 8, 2024

Copy link

Copy Markdown

GH-125098 is a backport of this pull request to the 3.12 branch.

All reactions

Sorry, something went wrong.

Uh oh!

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

bedevere-app Bot removed the needs backport to 3.12 only security fixes label

Oct 8, 2024

bedevere-app Bot commented Oct 8, 2024

Copy link

Copy Markdown

GH-125099 is a backport of this pull request to the 3.13 branch.

All reactions

Sorry, something went wrong.

Uh oh!

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

bedevere-app Bot removed the needs backport to 3.13 only security fixes label

Oct 8, 2024

picnixz deleted the fix/unicode-error-str-123378 branch

October 8, 2024 11:50

vstinner pushed a commit that referenced this pull request

Oct 8, 2024


          [3.12] gh-123378: fix a crash in UnicodeError.__str__ (GH-124935) (#…

d3b437c

…125098)

gh-123378: fix a crash in `UnicodeError.__str__` (GH-124935)
(cherry picked from commit ba14dfa)

Co-authored-by: Bénédikt Tran <[email protected]>

vstinner pushed a commit that referenced this pull request

Oct 8, 2024


          [3.13] gh-123378: fix a crash in UnicodeError.__str__ (GH-124935) (#…

…125099)

gh-123378: fix a crash in `UnicodeError.__str__` (GH-124935)
(cherry picked from commit ba14dfa)

Co-authored-by: Bénédikt Tran <[email protected]>

efimov-mikhail pushed a commit to efimov-mikhail/cpython that referenced this pull request

Oct 9, 2024


          pythongh-123378: fix a crash in UnicodeError.__str__ (python#124935)

Reviewers

iritkatriel

Awaiting requested review from iritkatriel

iritkatriel is a code owner

encukou

Awaiting requested review from encukou

vstinner

Awaiting requested review from vstinner

Uh oh!

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

Assignees

No one assigned

Labels

None yet

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.

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.

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.

GeezerVison

Conversation

picnixz commented Oct 3, 2024 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

encukou left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner Oct 7, 2024

Choose a reason for hiding this comment

Uh oh!

picnixz Oct 8, 2024

Choose a reason for hiding this comment

Uh oh!

vstinner Oct 7, 2024

Choose a reason for hiding this comment

Uh oh!

vstinner Oct 7, 2024

Choose a reason for hiding this comment

Uh oh!

miss-islington-app Bot commented Oct 8, 2024

Uh oh!

miss-islington-app Bot commented Oct 8, 2024

Uh oh!

bedevere-app Bot commented Oct 8, 2024

Uh oh!

bedevere-app Bot commented Oct 8, 2024

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

picnixz commented Oct 3, 2024 •

edited by bedevere-app Bot

Loading

Uh oh!

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