Skip to content

[2.7] bpo-32304: Fix distutils upload for tar files ending with b'\r' (GH-5264)#5331

Merged
merwok merged 4 commits into
python:2.7from
bbayles:backport-2fc98ae-2.7
Jan 29, 2018
Merged

[2.7] bpo-32304: Fix distutils upload for tar files ending with b'\r' (GH-5264)#5331
merwok merged 4 commits into
python:2.7from
bbayles:backport-2fc98ae-2.7

Conversation

@bbayles

@bbayles bbayles commented Jan 26, 2018

Copy link
Copy Markdown
Contributor

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

https://bugs.python.org/issue32304

@merwok

merwok commented Jan 26, 2018

Copy link
Copy Markdown
Member

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
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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