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

builtin __format__ methods cannot fill with \x00 char #56755

Closed
GavinAndresen mannequin opened this issue Jul 13, 2011 · 14 comments
Closed

builtin __format__ methods cannot fill with \x00 char #56755

GavinAndresen mannequin opened this issue Jul 13, 2011 · 14 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@GavinAndresen
Copy link
Mannequin

GavinAndresen mannequin commented Jul 13, 2011

BPO 12546
Nosy @vstinner, @ericvsmith, @ezio-melotti, @florentx, @sorcio
Files
  • format00.patch
  • 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/ericvsmith'
    closed_at = <Date 2014-04-14.16:09:44.041>
    created_at = <Date 2011-07-13.06:58:06.161>
    labels = ['interpreter-core', 'type-bug']
    title = 'builtin __format__ methods cannot fill with \\x00 char'
    updated_at = <Date 2014-05-19.15:15:04.784>
    user = 'https://bugs.python.org/GavinAndresen'

    bugs.python.org fields:

    activity = <Date 2014-05-19.15:15:04.784>
    actor = 'eric.smith'
    assignee = 'eric.smith'
    closed = True
    closed_date = <Date 2014-04-14.16:09:44.041>
    closer = 'eric.smith'
    components = ['Interpreter Core']
    creation = <Date 2011-07-13.06:58:06.161>
    creator = 'Gavin.Andresen'
    dependencies = []
    files = ['22640']
    hgrepos = []
    issue_num = 12546
    keywords = ['patch']
    message_count = 14.0
    messages = ['140225', '140231', '140233', '140235', '140238', '140242', '140654', '215457', '215793', '216093', '216105', '216106', '218782', '218797']
    nosy_count = 7.0
    nosy_names = ['vstinner', 'eric.smith', 'ezio.melotti', 'flox', 'davide.rizzo', 'python-dev', 'Gavin.Andresen']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue12546'
    versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

    @GavinAndresen
    Copy link
    Mannequin Author

    GavinAndresen mannequin commented Jul 13, 2011

    This gives me "foo " instead of expected "foo\x00\x00\x00" :

    "{0:\x00<6}".format('foo')

    @GavinAndresen GavinAndresen mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Jul 13, 2011
    @ericvsmith
    Copy link
    Member

    \x00 is used as a flag internally meaning "use the default fill character". That's clearly a bug. I'll look at fixing it.

    I think there are other places in the built in __format__ functions where special values are used instead of flags. I'll review those as well.

    Thanks for the report!

    @ericvsmith ericvsmith added interpreter-core (Objects, Python, Grammar, and Parser dirs) and removed stdlib Python modules in the Lib dir labels Jul 13, 2011
    @ericvsmith ericvsmith self-assigned this Jul 13, 2011
    @sorcio
    Copy link
    Mannequin

    sorcio mannequin commented Jul 13, 2011

    This patch removes the special meaning for \x00 and defines the default padding character (' ') in parse_internal_render_format_spec. Test included. Maybe the default padding character should be defined elsewhere?

    @sorcio
    Copy link
    Mannequin

    sorcio mannequin commented Jul 13, 2011

    Oops, sorry. Above patch was overly buggy. Please just ignore it.

    @sorcio
    Copy link
    Mannequin

    sorcio mannequin commented Jul 13, 2011

    Here's the patch. Same rationale as above (removed the special meaning of '\x00', default specified in parse_internal_render_format_spec). Sorry about the mess again!

    @ericvsmith
    Copy link
    Member

    Patch looks good at first glance. I'll review it some more today and commit it. Thanks!

    @ericvsmith
    Copy link
    Member

    I finally got around to reviewing the patch. A couple of comments:

    1. There should be some tests for str.__format__, not just str.format. This is really a bug with str.__format__, after all. I can add those.

    2. The bigger issue is that the other built in formatters have this same problem.

    >>> format(3, '\x00<6')
    '3     '
    >>> format(3., '\x00<6')
    '3.0   '
    >>> format('3', '\x00<6')
    '3\x00\x00\x00\x00\x00'
    >>> format(3+1j, '\x00<6')
    '(3+1j)'
    [38654 refs]
    >>> format(3+1j, '\x00<10')
    '(3+1j)    '

    I think the fix is basically the same as str.__format__ (but in format_int_or_long_internal, format_float_internal, and format_complex_internal). I tried that and it worked, but I haven't had time to write tests for them. If you (Davide) can do that, great. Otherwise I'll try and grab some time this week.

    Changing the subject to match the wider scope of the problem.

    @ericvsmith ericvsmith changed the title str.format cannot fill with \x00 char builtin __format__ methods cannot fill with \x00 char Jul 19, 2011
    @vstinner
    Copy link
    Member

    vstinner commented Apr 3, 2014

    bpo-17705 has been closed as a duplicate of this issue.

    @vstinner
    Copy link
    Member

    vstinner commented Apr 9, 2014

    The patch looks good to me.

    @vstinner vstinner added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Apr 9, 2014
    @ericvsmith ericvsmith added type-bug An unexpected behavior, bug, or error and removed type-feature A feature request or enhancement labels Apr 14, 2014
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 14, 2014

    New changeset 520ce42ba2b8 by Eric V. Smith in branch '2.7':
    Issue bpo-12546: Allow \x00 as a fill character for builtin type __format__ methods.
    http://hg.python.org/cpython/rev/520ce42ba2b8

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 14, 2014

    New changeset 7c484551bce1 by Eric V. Smith in branch '3.4':
    Issue bpo-12546: Allow \x00 as a fill character for builtin type __format__ methods.
    http://hg.python.org/cpython/rev/7c484551bce1

    New changeset bd90e68dc81f by Eric V. Smith in branch 'default':
    Closes issue bpo-12546: Allow \x00 as a fill character for builtin type __format__ methods.
    http://hg.python.org/cpython/rev/bd90e68dc81f

    @ericvsmith
    Copy link
    Member

    Fixed in 2.7, 3.4, 3.5.

    @vstinner
    Copy link
    Member

    I don't understand why it works with "<", "=" or ">":

    >>> "{0:\x00<6d}".format(123)
    '123\x00\x00\x00'

    But not without:

    >>> "{0:\x006d}".format(123)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    ValueError: Invalid format specifier

    Compare it to:

    >>> "{0:6d}".format(123)
    '   123'
    >>> "{0:06d}".format(123)
    '000123'

    @ericvsmith
    Copy link
    Member

    For int, the spec is:
    [[fill]align][sign][#][0][width][,][.precision][type]

    So, for "06d", "0" is matched as the literal 0, "6" is matched as width, and "d" is matched as type.

    For "\x00<6d", "\x00" is matched as fill, "<" as align, "6" as width, and "d" as type.

    For "\x006d", there's no align. So "\x00" cannot match as fill. "\x00" doesn't match anything else, so it's an invalid format specifier, thus the exception.

    @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
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants