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

Bytes objects should reject all formatting codes with an error message #72571

Closed
socketpair mannequin opened this issue Oct 7, 2016 · 20 comments
Closed

Bytes objects should reject all formatting codes with an error message #72571

socketpair mannequin opened this issue Oct 7, 2016 · 20 comments
Assignees
Labels
3.7 (EOL) end of life easy interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@socketpair
Copy link
Mannequin

socketpair mannequin commented Oct 7, 2016

BPO 28385
Nosy @vstinner, @ericvsmith, @ned-deily, @bitdancer, @socketpair, @berkerpeksag, @serhiy-storchaka
Files
  • issue28385.diff
  • object-format.patch
  • object-format-2.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/serhiy-storchaka'
    closed_at = <Date 2016-11-03.11:36:21.409>
    created_at = <Date 2016-10-07.14:28:40.046>
    labels = ['interpreter-core', 'easy', 'type-bug', '3.7']
    title = 'Bytes objects should reject all formatting codes with an error message'
    updated_at = <Date 2016-11-03.11:36:21.407>
    user = 'https://github.com/socketpair'

    bugs.python.org fields:

    activity = <Date 2016-11-03.11:36:21.407>
    actor = 'serhiy.storchaka'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2016-11-03.11:36:21.409>
    closer = 'serhiy.storchaka'
    components = ['Interpreter Core']
    creation = <Date 2016-10-07.14:28:40.046>
    creator = 'socketpair'
    dependencies = []
    files = ['45011', '45015', '45127']
    hgrepos = []
    issue_num = 28385
    keywords = ['patch', 'easy (C)']
    message_count = 20.0
    messages = ['278244', '278249', '278250', '278260', '278293', '278301', '278302', '278831', '278845', '278846', '279736', '279739', '279742', '279756', '279762', '279763', '279786', '279787', '279844', '279986']
    nosy_count = 8.0
    nosy_names = ['vstinner', 'eric.smith', 'ned.deily', 'r.david.murray', 'socketpair', 'python-dev', 'berker.peksag', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue28385'
    versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

    @socketpair
    Copy link
    Mannequin Author

    socketpair mannequin commented Oct 7, 2016

    $ python3 -c "'{0:s}'.format(b'qwe')"
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
    TypeError: non-empty format string passed to object.__format__

    Spent many hours to detect bug in my code.

    @socketpair socketpair mannequin added stdlib Python modules in the Lib dir 3.7 (EOL) end of life labels Oct 7, 2016
    @bitdancer
    Copy link
    Member

    Bytes don't support formatting codes, so they are passed through to object, which doesn't support any formatting codes, and produces the error message you see. Since all other built in types reject invalid codes with a message that mentions their type, I think bytes should too. This would change the message to:

    ValueError: Unknown format code 's' for object of type 'bytes'

    Would that have lessened your confusion sufficiently?

    @bitdancer bitdancer changed the title Non-informative exception while formatting strings. Bytes objects should reject all formatting codes with an error message Oct 7, 2016
    @bitdancer bitdancer added the type-bug An unexpected behavior, bug, or error label Oct 7, 2016
    @ericvsmith
    Copy link
    Member

    Yes, I think we should implement this for object.__format__. Marking as easy.

    @ericvsmith ericvsmith added easy interpreter-core (Objects, Python, Grammar, and Parser dirs) and removed stdlib Python modules in the Lib dir labels Oct 7, 2016
    @socketpair
    Copy link
    Mannequin Author

    socketpair mannequin commented Oct 7, 2016

    Yes, that message will be sufficient. Perfectly.

    @berkerpeksag
    Copy link
    Member

    Here's a patch. Changes in test_builtin might be redundant, but I added them anyway.

    @ericvsmith
    Copy link
    Member

    I left a comment on Rietveld, but repeating it here because for me those messages often go to spam:

    I think you want to make this more general, by modifying the message for the TypeError that follows. Any type that doesn't provide it's own __format__ should produce an error when using a non-empty format string.

    For example:

    >>> format({}, 'a')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: non-empty format string passed to object.__format__

    Would be better as:
    TypeError: non-empty format string passed to __format__ for object of type "class <'dict'>"

    (Or some prettier way to print out the type).

    @serhiy-storchaka
    Copy link
    Member

    Sorry, but it looks to me that bpo-28385.diff solves wrong issue. The issue is not in bytes, but in default __format__. object.__format__ should raise an error message that contains the name of the actual type. There are other issues with object.__format__, following patch tries to solve them.

    1. Error message now contains the name of the actual type.

    2. Format spec is checked before converting the value to str. Converting bytes and bytearray to str can raise a BytesWarning, and it is better if the type of this error doesn't depend on the -b option.

    3. Since format spec is always empty, avoid calling PyObject_Format(). In theory PyObject_Str() can raise a subclass of str with non-trivial __format__, but I think we can ignore this subtle behavior difference.

    @ericvsmith
    Copy link
    Member

    LGTM, although I'm not so sure about your #3. Maybe it should be a separate issue and raised on python-dev? But I don't feel strongly enough about it to argue the point.

    Thanks!

    @serhiy-storchaka
    Copy link
    Member

    Removed item 3 from updated patch.

    @serhiy-storchaka
    Copy link
    Member

    From msg214162:

    This is basically the definition of object.__format__:

    def __format__(self, specifier):
    if len(specifier) == 0:
    return str(self)
    raise TypeError('non-empty format string passed to object.__format__')

    This simple algorithm is implemented in object-format.patch.

    But current implementation uses format(str(self), '') instead of just str(self). object-format-2.patch keeps this.

    @serhiy-storchaka serhiy-storchaka self-assigned this Oct 30, 2016
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 30, 2016

    New changeset 92cae79fa5d9 by Serhiy Storchaka in branch '3.5':
    Issue bpo-28385: An error message when non-empty format spec is passed to
    https://hg.python.org/cpython/rev/92cae79fa5d9

    New changeset 0a985f7c6731 by Serhiy Storchaka in branch '3.6':
    Issue bpo-28385: An error message when non-empty format spec is passed to
    https://hg.python.org/cpython/rev/0a985f7c6731

    New changeset 6e8183abcc35 by Serhiy Storchaka in branch 'default':
    Issue bpo-28385: An error message when non-empty format spec is passed to
    https://hg.python.org/cpython/rev/6e8183abcc35

    @ned-deily
    Copy link
    Member

    Buildbots are failing:

    ======================================================================
    FAIL: test_errors (test.test_fstring.TestCase) (str="f'{(lambda: 0):x}'")
    ----------------------------------------------------------------------
    TypeError: unsupported format string passed to function.__format__

    During handling of the above exception, another exception occurred:
    
    Traceback (most recent call last):
      File "/root/buildarea/3.6.angelico-debian-amd64/build/Lib/test/test_fstring.py", line 20, in assertAllRaise
        eval(str)
    AssertionError: "non-empty" does not match "unsupported format string passed to function.__format__"

    ======================================================================
    FAIL: test_errors (test.test_fstring.TestCase) (str="f'{(0,):x}'")
    ----------------------------------------------------------------------
    TypeError: unsupported format string passed to tuple.__format__

    During handling of the above exception, another exception occurred:
    
    Traceback (most recent call last):
      File "/root/buildarea/3.6.angelico-debian-amd64/build/Lib/test/test_fstring.py", line 20, in assertAllRaise
        eval(str)
    AssertionError: "non-empty" does not match "unsupported format string passed to tuple.__format__"

    Ran 48 tests in 1.389s

    @serhiy-storchaka
    Copy link
    Member

    What is better?

    1. Replace "unsupported format string passed to" back to "non-empty format string passed to " in generated error message? Note that full error message is changed in any case, this is the purpose of this issue.

    2. Change the pattern "non-empty" to "unsupported" in f-string tests. Or to other part of new error message. It may be changed again in future.

    @ericvsmith
    Copy link
    Member

    I would change the f-string tests. I realize it's a fragile test, but it's the only way to test the strings in the exception.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 31, 2016

    New changeset f43078ec598c by Serhiy Storchaka in branch '3.6':
    Update the f-string test broken in issue bpo-28385.
    https://hg.python.org/cpython/rev/f43078ec598c

    New changeset 931410a04240 by Serhiy Storchaka in branch 'default':
    Update the f-string test broken in issue bpo-28385.
    https://hg.python.org/cpython/rev/931410a04240

    @serhiy-storchaka
    Copy link
    Member

    Shouldn't the type of error be changed from TypeError to ValueError?

    @ericvsmith
    Copy link
    Member

    TypeError is documented as "Raised when an operation or function is applied to an object of inappropriate type". I think that fits this case: you're applying some format code to a type that doesn't support it.

    @serhiy-storchaka
    Copy link
    Member

    But on other hand, the error depends on the value of format specifier:

    >>> format('abc', '')
    'abc'
    >>> format('abc', 'j')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    ValueError: Unknown format code 'j' for object of type 'str'
    >>> format([1, 2, 3], '')
    '[1, 2, 3]'
    >>> format([1, 2, 3], 'j')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: unsupported format string passed to list.__format__

    If keep TypeError I think it would be better to restore former error message "non-empty format string".

    @ericvsmith
    Copy link
    Member

    I don't have a strong feeling one way or the other. I'd be surprised if anyone is catching these errors.

    @serhiy-storchaka
    Copy link
    Member

    The proposition of making default __format__ equivalent to str() will be addressed in separate issue. Python-Dev thread: https://mail.python.org/pipermail/python-dev/2016-October/146765.html.

    @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 (EOL) end of life easy 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

    5 participants