Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upload failed (400): Digests do not match on .tar.gz ending with x0d binary code #76485

Closed
llecaroz mannequin opened this issue Dec 13, 2017 · 7 comments
Closed

Upload failed (400): Digests do not match on .tar.gz ending with x0d binary code #76485

llecaroz mannequin opened this issue Dec 13, 2017 · 7 comments
Assignees
Labels
3.7 stdlib type-security

Comments

@llecaroz
Copy link
Mannequin

@llecaroz llecaroz mannequin commented Dec 13, 2017

BPO 32304
Nosy @pitrou, @merwok, @dstufft, @bbayles, @llecaroz
PRs
  • #5264
  • #5330
  • #5331
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/merwok'
    closed_at = <Date 2018-02-23.18:23:31.886>
    created_at = <Date 2017-12-13.15:48:29.389>
    labels = ['type-security', '3.7', 'library']
    title = 'Upload failed (400): Digests do not match on .tar.gz ending with x0d binary code'
    updated_at = <Date 2018-02-23.18:23:31.884>
    user = 'https://github.com/llecaroz'

    bugs.python.org fields:

    activity = <Date 2018-02-23.18:23:31.884>
    actor = 'eric.araujo'
    assignee = 'eric.araujo'
    closed = True
    closed_date = <Date 2018-02-23.18:23:31.886>
    closer = 'eric.araujo'
    components = ['Distutils']
    creation = <Date 2017-12-13.15:48:29.389>
    creator = 'llecaroz'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 32304
    keywords = ['patch']
    message_count = 7.0
    messages = ['308205', '308691', '310720', '310832', '311141', '312624', '312662']
    nosy_count = 5.0
    nosy_names = ['pitrou', 'eric.araujo', 'dstufft', 'bbayles', 'llecaroz']
    pr_nums = ['5264', '5330', '5331']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'security'
    url = 'https://bugs.python.org/issue32304'
    versions = ['Python 2.7', 'Python 3.6', 'Python 3.7']

    @llecaroz
    Copy link
    Mannequin Author

    @llecaroz llecaroz mannequin commented Dec 13, 2017

    Hi,
    .tar.gz files can end with x0d bytes or whatever you want

    When running setup.py sdist upload, depending on the project, the .tar.gz file, as said can sometimes end with x0d. When doing the upload, the line https://github.com/python/cpython/blob/master/Lib/distutils/command/upload.py#L162 (if value and value[-1:] == b'\r') will remove the ending char of the .tar.gz generating a 400 response error from the server like:

    Upload failed (400): Digests do not match, found: 09f23b52764a6802a87dd753009c2d3d, expected: 972b8e9d3dc8cf6ba6b4b1ad5991f013
    error: Upload failed (400): Digests do not match, found: 09f23b52764a6802a87dd753009c2d3d, expected: 972b8e9d3dc8cf6ba6b4b1ad5991f013

    As this line is generic & run on all key/values, I clearly understand that this check was initially written to eliminate certainly some issues on values in text format.

    But the mistake here, is that you are also changing the content of the 'content' key which contains the .tar.gz as value, and because you remove the ending 0D, you change the .tar.gz content to be uploaded. As consequence, the server will return a 400 error about a wrong digest/crc.

    I was able to make the code working with all .tar.gz files by changing this line to:

                    if value and value[-1:] == '\r' and not key=='content':

    With a such fix, the .tar.gz content will not see its ending \r to be removed & the computed CRC from the server will be the same as computed by md5(content).hexdigest() in upload.py

    @llecaroz llecaroz mannequin added 3.8 3.7 stdlib type-security labels Dec 13, 2017
    @pitrou
    Copy link
    Member

    @pitrou pitrou commented Dec 19, 2017

    I agree with the suggested fix. Do you want to submit a PR?

    @pitrou pitrou removed the 3.8 label Dec 19, 2017
    @merwok
    Copy link
    Member

    @merwok merwok commented Jan 26, 2018

    New changeset 2fc98ae by Éric Araujo (Bo Bayles) in branch 'master':
    bpo-32304: Fix distutils upload for sdists ending with \x0d (GH-5264)
    2fc98ae

    @merwok
    Copy link
    Member

    @merwok merwok commented Jan 27, 2018

    New changeset 995c60d by Éric Araujo (Bo Bayles) in branch '3.6':
    [3.6] bpo-32304: Fix distutils upload for tar files ending with b'\r' (GH-5264) (GH-5330)
    995c60d

    @merwok merwok self-assigned this Jan 28, 2018
    @merwok
    Copy link
    Member

    @merwok merwok commented Jan 29, 2018

    New changeset f5a7935 by Éric Araujo (Bo Bayles) in branch '2.7':
    bpo-32304: Fix distutils upload for tar files ending with b'\r' (GH-5264) (GH-5331)
    f5a7935

    @llecaroz
    Copy link
    Mannequin Author

    @llecaroz llecaroz mannequin commented Feb 23, 2018

    Hi,

    First of all, thank you so much for having fixed this bug, I checked in 3.5 & it seems that this fix needs to be also backport in 3.5 branch & certainly others branches (like 3.4) ?

    Thx in advance for your coming feedback
    Best regards
    Louis

    @merwok
    Copy link
    Member

    @merwok merwok commented Feb 23, 2018

    @merwok merwok added 3.8 and removed 3.8 labels Feb 23, 2018
    @merwok merwok closed this as completed Feb 23, 2018
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 stdlib type-security
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants