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

struct.pack('L', -1) #48478

Closed
arigo mannequin opened this issue Oct 29, 2008 · 10 comments
Closed

struct.pack('L', -1) #48478

arigo mannequin opened this issue Oct 29, 2008 · 10 comments

Comments

@arigo
Copy link
Mannequin

arigo mannequin commented Oct 29, 2008

BPO 4228
Nosy @loewis, @arigo, @birkenfeld, @mdickinson
Files
  • struct-2.5-fix.diff
  • 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 = None
    closed_at = <Date 2009-01-01.12:15:37.475>
    created_at = <Date 2008-10-29.15:41:01.649>
    labels = ['release-blocker']
    title = "struct.pack('L', -1)"
    updated_at = <Date 2009-03-18.13:26:02.121>
    user = 'https://github.com/arigo'

    bugs.python.org fields:

    activity = <Date 2009-03-18.13:26:02.121>
    actor = 'mark.dickinson'
    assignee = 'none'
    closed = True
    closed_date = <Date 2009-01-01.12:15:37.475>
    closer = 'georg.brandl'
    components = []
    creation = <Date 2008-10-29.15:41:01.649>
    creator = 'arigo'
    dependencies = []
    files = ['12326']
    hgrepos = []
    issue_num = 4228
    keywords = ['patch']
    message_count = 10.0
    messages = ['75319', '75320', '77497', '77613', '77615', '77734', '78702', '83586', '83743', '83745']
    nosy_count = 5.0
    nosy_names = ['loewis', 'arigo', 'georg.brandl', 'mark.dickinson', 'andreas.schawo']
    pr_nums = []
    priority = 'release blocker'
    resolution = 'accepted'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue4228'
    versions = ['Python 2.6', 'Python 2.7']

    @arigo
    Copy link
    Mannequin Author

    arigo mannequin commented Oct 29, 2008

    struct.pack('L', -1) raises a DeprecationWarning since Python 2.5, as it
    should. However, it also returns a different (and nonsensical) result
    than Python <= 2.4 used to: it returns '\x00\x00\x00\x00' instead of
    '\xff\xff\xff\xff'.

    This might lead the zipfile module of release25-maint (the version >=
    2.5.2) to produce buggy zip files. The -1 value can come as the
    header_offset field, which will then be packed as an all-0 string
    instead of an all-ff string in the zip file headers.

    Given the DeprecationWarning I would classify this as low priority.
    However, given that the stdlib module zipfile relies on this feature in
    release25-maint, it should probably really be fixed.

    @arigo
    Copy link
    Mannequin Author

    arigo mannequin commented Oct 29, 2008

    Ah, I should also mention that a fix of zipfile for 2.5 to no longer use
    the deprecated feature (and thus no longer cause DeprecationWarnings)
    also sounds like a good idea, in addition to the fix to the struct module.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Dec 10, 2008

    Can you provide a fix within the next two days? Otherwise, I see little
    chance that this gets fixed in 2.5.

    @arigo
    Copy link
    Mannequin Author

    arigo mannequin commented Dec 11, 2008

    FWIW, struct.pack("I", "whatever") produces "\x00\x00\x00\x00" too.

    @arigo
    Copy link
    Mannequin Author

    arigo mannequin commented Dec 11, 2008

    Attached struct-2.5-fix.diff. The tests still pass (both 32- and 64-bits).

    @loewis loewis mannequin added the release-blocker label Dec 11, 2008
    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Dec 13, 2008

    Thanks for the patch. Committed (along with a test case) as r67733, in
    the 2.5 branch.

    Porting to the other branches still needs to happen. Armin, if you want
    to make these changes, please go ahead.

    @birkenfeld
    Copy link
    Member

    Committed to trunk in r68120.

    @mdickinson
    Copy link
    Member

    Was zipfile ever fixed to avoid this deprecated behaviour? If not, is the
    fix fairly trivial?

    It would be nice to be able to finally turn these struct deprecation
    warnings into errors in Python 3.1 and/or Python 2.7.

    @andreasschawo
    Copy link
    Mannequin

    andreasschawo mannequin commented Mar 18, 2009

    As I understand actually the zipfile module possibly creates damaged zip
    files after version 2.4 because of '\x00\x00\x00\x00' instead of
    '\xff\xff\xff\xff' as header offset. But without any error.

    I think the _struct.c should be cleaned in any case. Because we only get
    errors in zipfile module when damaged zip files are created. An error
    would be appriciated instead of a silenty damaged zip file.

    But, why don't boundary check the header offset in zipfile module in a
    short private function and returning '\xff\xff\xff\xff' in case of
    overflow? Maybe all longs should be boundary checked if this seems
    necassery.

    @mdickinson
    Copy link
    Member

    I don't know the zipfile module very well (i.e., at all),
    but as far as I can tell from looking at the source,
    there's no use of struct.pack('L', -1) in 2.6 onwards:
    it's only potentially a problem in 2.5 (and that isn't
    going to change, now that 2.5 is in security-fix only
    mode).

    Can anyone who understands the zipfile module better than
    me confirm this?

    @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
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants