Skip to content

Conversation

@bbayles
Copy link
Contributor

@bbayles bbayles commented Jan 26, 2018

This PR brings #5264, which @merwok merged, to version 2.7.

https://bugs.python.org/issue32304

@merwok
Copy link
Member

merwok commented Jan 26, 2018

Thanks for the backport. Could you also port the what’s new entry?


# bpo-32304: archives whose last byte was b'\r' were corrupted due to
# normalization intended for Mac OS 9.
def test_upload_correct_cr(self):
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I see what's going on - the cmd.show_response isn't the same in this version. There are some other differences in the test I'll smooth out shortly.

@merwok merwok self-assigned this Jan 26, 2018

headers = dict(self.last_open.req.headers)
self.assertEqual(headers['Content-length'], '2170')
self.assertIn(b'long description\r', self.last_open.req.data)
Copy link
Member

Choose a reason for hiding this comment

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

This test would not catch a regression, i.e. fail if the request data contains b'long description\r\n'.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The size check earlier would signal the error, but I've added a new line to make it explicit.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks!

@merwok merwok merged commit f5a7935 into python:2.7 Jan 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants