Skip to content

Conversation

@vikhegde
Copy link

@vikhegde vikhegde commented Jun 7, 2017

A bug in the punycode codec raises an IndexError when we attempt to decode the string "xn--w&". The fix is straightforward and obvious one-liner. Bug number: bpo-30566

https://bugs.python.org/issue30566

@zware
Copy link
Member

zware commented Jun 7, 2017

Could you add a test for this?

@vikhegde
Copy link
Author

vikhegde commented Jun 7, 2017

I have added a test case to the punycode Test-suite.

Copy link
Member

@berkerpeksag berkerpeksag left a comment

Choose a reason for hiding this comment

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

Please also add a NEWS file. See https://devguide.python.org/committing/#what-s-new-and-news-entries for details.

# Make sure punycode codec raises the right kind of exception
# when given invalid punycode to decode
def test_decode_exceptions(self):
for byt, exception in punycode_decode_exceptions:
Copy link
Member

Choose a reason for hiding this comment

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

I'd rename byt to puny.

if len(i)!=2:
print(repr(i))

punycode_decode_exceptions = [
Copy link
Member

Choose a reason for hiding this comment

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

I'd rename this to invalid_punycode_testcases.

puny = puny.decode("ascii").encode("ascii")
self.assertEqual(uni, puny.decode("punycode"))

# Make sure punycode codec raises the right kind of exception
Copy link
Member

Choose a reason for hiding this comment

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

Style nit: We can drop this comment.

# when given invalid punycode to decode
def test_decode_exceptions(self):
for byt, exception in punycode_decode_exceptions:
with self.assertRaises(exception):
Copy link
Member

Choose a reason for hiding this comment

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

We couls use self.subTest() to make test failure messages clearer.

@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@serhiy-storchaka serhiy-storchaka changed the title Fix for issue bpo-30566 bpo-30566: Fix IndexError in the punycode codec Mar 26, 2018
@serhiy-storchaka serhiy-storchaka added type-bug An unexpected behavior, bug, or error needs backport to 3.6 labels Mar 26, 2018
@vstinner
Copy link
Member

I removed the " needs backport to 3.6" label, the 3.6 branch no longer accept bugfixes (only security fixes are accepted): https://devguide.python.org/#status-of-python-branches

@zware
Copy link
Member

zware commented Sep 11, 2019

@vikhegde Would you like to move this forward by implementing @berkerpeksag's suggestions?

@csabella
Copy link
Contributor

This pull request seems to have been abandoned, so I'm going to close it. It can reopened if the original author is interested in working on it again or a new pull request can be created to replace it. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting changes type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants