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.

Sign up for GitHub

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

Jump to bottom

struct.pack_into check boundary error message ignores offset #73835

Closed
mlouielu mannequin opened this issue Feb 25, 2017 · 11 comments
Closed

struct.pack_into check boundary error message ignores offset #73835

mlouielu mannequin opened this issue Feb 25, 2017 · 11 comments
Assignees
@serhiy-storchaka
Labels
3.7 (EOL) end of life extension-modules C modules in the Modules dir type-feature A feature request or enhancement

Comments

@mlouielu
Copy link
Mannequin

mlouielu mannequin commented Feb 25, 2017

BPO 29649
Nosy @serhiy-storchaka, @andrewnester, @mlouielu
PRs
  • bpo-29649: struct.pack_into check boundary error message didn't respect offset #424
    		•

    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/serhiy-storchaka'
closed_at = <Date 2017-04-04.10:47:37.875>
created_at = <Date 2017-02-25.13:53:04.073>
labels = ['extension-modules', 'type-feature', '3.7']
title = 'struct.pack_into check boundary error message ignores offset'
updated_at = <Date 2017-04-04.10:47:37.874>
user = 'https://github.com/mlouielu'

    bugs.python.org fields:

    activity = <Date 2017-04-04.10:47:37.874>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2017-04-04.10:47:37.875>
closer = 'serhiy.storchaka'
components = ['Extension Modules']
creation = <Date 2017-02-25.13:53:04.073>
creator = 'louielu'
dependencies = []
files = []
hgrepos = []
issue_num = 29649
keywords = []
message_count = 11.0
messages = ['288564', '288882', '288884', '288885', '288886', '288887', '288978', '289021', '290081', '291119', '291120']
nosy_count = 3.0
nosy_names = ['serhiy.storchaka', 'andrewnester', 'louielu']
pr_nums = ['424']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue29649'
versions = ['Python 3.7']
    @mlouielu
    Copy link
    Mannequin Author

    mlouielu mannequin commented Feb 25, 2017

    For this situation, check boundary error message didn't correctly show out.

    >>> import struct
>>> import ctypes
>>> byte_list = ctypes.create_string_buffer(1)
>>> struct.pack_into('b', byte_list, 5, 1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
struct.error: pack_into requires a buffer of at least 1 bytes

    Since offset is setting at 5, it should at least need offset + soself->s_size bytes to store it.

    @mlouielu mlouielu mannequin added 3.7 (EOL) end of life extension-modules C modules in the Modules dir labels Feb 25, 2017
    @andrewnester
    Copy link
    Mannequin

    andrewnester mannequin commented Mar 3, 2017

    Thanks! Just added PR fixing this.

    @serhiy-storchaka
    Copy link
    Member

    I'm not sure the new error message is better. It is not known what is wrong -- the size of the buffer, or the offset?

    I think that the better error message should include three numbers: the offset, the size of the destination buffer, and the size of packed data or the minimal size of the destination buffer.

    @andrewnester
    Copy link
    Mannequin

    andrewnester mannequin commented Mar 3, 2017

    yeah, I also thought about this too.
    Something like 'pack_into requires a buffer of at least 6 bytes (size is 1, offset is 5)'

    @serhiy-storchaka
    Copy link
    Member

    Or "pack_into requires a buffer of at least 6 bytes for packing 1 bytes at offset 5".

    @andrewnester
    Copy link
    Mannequin

    andrewnester mannequin commented Mar 3, 2017

    thanks Serhiy! just implemented your variant in my PR.

    @terryjreedy terryjreedy changed the title struct.pack_into check boundary error message didn't respect offset struct.pack_into check boundary error message ignores offset Mar 3, 2017
    @terryjreedy terryjreedy added the type-feature A feature request or enhancement label Mar 3, 2017
    @serhiy-storchaka
    Copy link
    Member

    Different error messages are needed if original offset < 0. For example packing 4 bytes with offset -2 always fails, not depending of the size of the buffer. Packing into buffer of size 10 with offset -11 always fails, not depending of the size of packed data.

    struct.pack_into('<I', bytearray(10), -2, 123)
    struct.pack_into('<B', bytearray(10), -11, 123)

    @serhiy-storchaka serhiy-storchaka self-assigned this Mar 4, 2017
    @andrewnester
    Copy link
    Mannequin

    andrewnester mannequin commented Mar 5, 2017

    Thanks Serhiy! Just implemented new error messages in my PR for case you mentioned.

    @andrewnester
    Copy link
    Mannequin

    andrewnester mannequin commented Mar 24, 2017

    any updates on this issue? looks like PR is ready to go

    @serhiy-storchaka
    Copy link
    Member

    New changeset f78b119 by Serhiy Storchaka (Andrew Nester) in branch 'master':
    bpo-29649: Improve struct.pack_into() boundary error messages (#424)
    f78b119

    @serhiy-storchaka
    Copy link
    Member

    Thank you for your contribution Andrew.

    @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
    Assignees

    @serhiy-storchaka serhiy-storchaka

    Labels
    3.7 (EOL) end of life extension-modules C modules in the Modules dir type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @serhiy-storchaka @terryjreedy