Skip to content

struct.pack error messages do not indicate which argument was invalid #67766

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

Open
alynn mannequin opened this issue Mar 3, 2015 · 6 comments
Open

struct.pack error messages do not indicate which argument was invalid #67766

alynn mannequin opened this issue Mar 3, 2015 · 6 comments
Labels
3.13 bugs and security fixes easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@alynn
Copy link
Mannequin

alynn mannequin commented Mar 3, 2015

BPO 23578
Nosy @mdickinson, @meadori, @serhiy-storchaka, @mlouielu
PRs
  • bpo-23578: Show which offset raise error when using struct.pack #291
  • 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 = None
    created_at = <Date 2015-03-03.17:03:05.610>
    labels = ['easy', 'type-feature', 'library', '3.11']
    title = 'struct.pack error messages do not indicate which argument was invalid'
    updated_at = <Date 2022-03-24.00:24:53.123>
    user = 'https://bugs.python.org/alynn'

    bugs.python.org fields:

    activity = <Date 2022-03-24.00:24:53.123>
    actor = 'iritkatriel'
    assignee = 'serhiy.storchaka'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2015-03-03.17:03:05.610>
    creator = 'alynn'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 23578
    keywords = ['easy (C)']
    message_count = 4.0
    messages = ['237153', '288565', '288569', '288570']
    nosy_count = 5.0
    nosy_names = ['mark.dickinson', 'meador.inge', 'serhiy.storchaka', 'alynn', 'louielu']
    pr_nums = ['291']
    priority = 'low'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue23578'
    versions = ['Python 3.11']

    Linked PRs

    @alynn
    Copy link
    Mannequin Author

    alynn mannequin commented Mar 3, 2015

    In this example:

      struct.pack('!hhhh', 0x5FFF, 0x6FFF, 0x7FFF, 0x8FFF)

    Python errors that the 'h' format requires -32768 <= number <= 32767, but it does not indicate which of the arguments is at fault. In this contrived example it's clearly the fourth one, but in dealing with large amounts of data it would be useful to indicate which argument was invalid.

    This would hopefully not be a functional change, just a slightly more helpful error message.

    @alynn alynn mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Mar 3, 2015
    @bitdancer bitdancer added easy 3.7 (EOL) end of life labels Oct 8, 2016
    @mlouielu
    Copy link
    Mannequin

    mlouielu mannequin commented Feb 25, 2017

    Adding PyErr_SetString and PyErr_Format wrapper, with a global offset
    variable to handle this.

    struct.pack('!h', 0x8FFFF)
    Traceback (most recent call last):
      File "tests.py", line 5, in <module>
        struct.pack('!h', 0x8FFFF)
    struct.error: Raise at offset 1, 'h' format requires -32768 <= number <= 32767

    @serhiy-storchaka
    Copy link
    Member

    1. Using global variable doesn't look good to me.

    2. "at offset 1" looks confusing to me. What is offset? Is this an offset of packed item in created bytes object? The you shouldn't get the odd number for the '!h' format.

    I think it would be better to report the number of the packed item. struct.pack() already formats similar errors when pass unsuitable number of items.

    >>> struct.pack('<hb', 1)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    struct.error: pack expected 2 items for packing (got 1)
    >>> struct.pack('<hb', 1, 2, 3)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    struct.error: pack expected 2 items for packing (got 3)
    1. It is not safe to use the fixed length array for formatting error message. Once the underlying error message can be changed and will overflow the buffer. The "%zd" format in sprintf() is not portable.

    @mlouielu
    Copy link
    Mannequin

    mlouielu mannequin commented Feb 25, 2017

    1. Using global variable doesn't look good to me.

    That's true, but I'm not sure if there have other methods to do this.

    If not using global variable, we will need to change a bunch of the
    function arguments. Since the arguments didn't contain the
    information about which item is in the process and raise the error.

    1. "at offset 1" looks confusing to me. What is offset?
    Make the change to: 
    >>> struct.pack('hh', , 0x7FFFF, 0x8FFFF)
    struct.error: 'h' format requires -32768 <= number <= 32767, got bad value at item 2

    (or probably, "got bad value at index 2")

    1. It is not safe to use the fixed length array for formatting error message. Once the underlying error message can be changed and will overflow the buffer.

    Change to snprintf.

    The "%zd" format in sprintf() is not portable.

    Change to "%ld"

    @serhiy-storchaka serhiy-storchaka self-assigned this Feb 26, 2017
    @iritkatriel iritkatriel added 3.11 only security fixes and removed 3.7 (EOL) end of life labels Mar 24, 2022
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @jorisgeer
    Copy link

    @serhiy-storchaka Shall I take this over from you ?

    @iritkatriel
    Copy link
    Member

    @serhiy-storchaka Shall I take this over from you ?

    Go for it.

    @iritkatriel iritkatriel added 3.13 bugs and security fixes and removed 3.11 only security fixes labels May 13, 2023
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.13 bugs and security fixes easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    Successfully merging a pull request may close this issue.

    4 participants