bpo-32304: Fix distutils upload for tar files ending with b'\r' #5264
Conversation
Lib/distutils/command/upload.py
Outdated
| @@ -159,7 +159,7 @@ def upload_file(self, command, pyversion, filename): | |||
| body.write(title.encode('utf-8')) | |||
| body.write(b"\r\n\r\n") | |||
| body.write(value) | |||
| if value and value[-1:] == b'\r': | |||
| if value and (value[-1:] == b'\r') and (key != 'content'): | |||
| body.write(b'\n') # write an extra newline (lurve Macs) | |||
There was a problem hiding this comment.
I’m wondering if this if block is even needed. Only Mac OS 9 used \n as end of line character, right?
We could delete both of these lines.
There was a problem hiding this comment.
Ah, I agree! (I alluded to this above)
Shall I change this PR to do that? It would save having to add the test case.
There was a problem hiding this comment.
OK, I've zapped the newline correction. The test is still there, but a little odd - "make sure we don't append '\n' to anything," so I added an explanatory comment.
merwok
added
type-bug
Lib/distutils/tests/test_upload.py
Outdated
| cmd.run() | ||
|
|
||
| # an extra character should have been added to the description, | ||
| # but not to the content |
There was a problem hiding this comment.
Whoops, good catch.
Lib/distutils/tests/test_upload.py
Outdated
| dist_files = [(command, pyversion, filename)] | ||
| self.write_file(self.rc, PYPIRC_LONG_PASSWORD) | ||
|
|
||
| # other fields that end with \r should not be modified. |
There was a problem hiding this comment.
Update comment: other fields that ended with \r used to be modified, now are preserved
|
Thanks a lot! I will merge tomorrow and this will be in the next beta. |
| The ``upload`` command now longer tries to change CR end-of-line characters | ||
| to CRLF. This fixes a corruption issue with sdists that ended with a byte | ||
| equivalent to CR. | ||
| (Contributed by Bo Bayles in :issue:`32304`.) |
There was a problem hiding this comment.
@bbayles I used the name found in your github profile, is that ok?
There was a problem hiding this comment.
Yes, that's fine. Thanks!
|
Sorry, @bbayles and @merwok, I could not cleanly backport this to |
|
Sorry, @bbayles and @merwok, I could not cleanly backport this to |
…thonGH-5264) Patch by Bo Bayles.. (cherry picked from commit 2fc98ae)
|
GH-5330 is a backport of this pull request to the 3.6 branch. |
…thonGH-5264) Patch by Bo Bayles.. (cherry picked from commit 2fc98ae)
|
GH-5331 is a backport of this pull request to the 2.7 branch. |
) (GH-5331) Patch by Bo Bayles.
This PR fixes issue 32304, in which
distutils's upload command corrupts tar files that end withb'\r'by appendingb'\n'to them (here).The original report describes a simple fix, which is endorsed. I've provided that fix here, plus a test that ensures that (1) the "content" doesn't get the newline appended, (2) the other keys do.
Of course, one wonders if fixing
'\r'without'\n'is so important now that no major OSs use it?https://bugs.python.org/issue32304