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

python2 file __repr__ does not escape filename #58369

Closed
RonnyPfannschmidt mannequin opened this issue Feb 29, 2012 · 12 comments
Closed

python2 file __repr__ does not escape filename #58369

RonnyPfannschmidt mannequin opened this issue Feb 29, 2012 · 12 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@RonnyPfannschmidt
Copy link
Mannequin

RonnyPfannschmidt mannequin commented Feb 29, 2012

BPO 14161
Nosy @pitrou, @pjenvey, @ezio-melotti, @merwok, @RonnyPfannschmidt
Files
  • issue14161.diff: Patch against 2.7.
  • 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/ezio-melotti'
    closed_at = <Date 2012-10-01.01:38:27.506>
    created_at = <Date 2012-02-29.16:43:29.986>
    labels = ['interpreter-core', 'type-bug']
    title = 'python2 file __repr__ does not escape filename'
    updated_at = <Date 2012-10-02.17:10:08.198>
    user = 'https://github.com/RonnyPfannschmidt'

    bugs.python.org fields:

    activity = <Date 2012-10-02.17:10:08.198>
    actor = 'Ronny.Pfannschmidt'
    assignee = 'ezio.melotti'
    closed = True
    closed_date = <Date 2012-10-01.01:38:27.506>
    closer = 'ezio.melotti'
    components = ['Interpreter Core']
    creation = <Date 2012-02-29.16:43:29.986>
    creator = 'Ronny.Pfannschmidt'
    dependencies = []
    files = ['24694']
    hgrepos = []
    issue_num = 14161
    keywords = ['patch']
    message_count = 12.0
    messages = ['154649', '154674', '154675', '154679', '154702', '154810', '155418', '155420', '155422', '171688', '171817', '171818']
    nosy_count = 6.0
    nosy_names = ['pitrou', 'pjenvey', 'ezio.melotti', 'eric.araujo', 'python-dev', 'Ronny.Pfannschmidt']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue14161'
    versions = ['Python 2.7']

    @RonnyPfannschmidt
    Copy link
    Mannequin Author

    RonnyPfannschmidt mannequin commented Feb 29, 2012

    behaviour:
    >>> name = 'woo\raa'
    >>> open(name, 'w')
    aa', mode 'w' at 0x295a8a0>
    
    expected:
    >>> name = 'woo\raa'
    >>> open(name, 'w')
    <open file 'woo\raa', mode 'w' at 0x295a8a0>

    note:
    don't ask why i tried this chunk of code in the first place

    @RonnyPfannschmidt RonnyPfannschmidt mannequin added the type-bug An unexpected behavior, bug, or error label Feb 29, 2012
    @ezio-melotti ezio-melotti added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Feb 29, 2012
    @merwok
    Copy link
    Member

    merwok commented Mar 1, 2012

    Funny one :D Reproduced on linux:

      >>> open('/tmp/t\nest', 'w')
      <open file '/tmp/t
      est', mode 'w' at 0x7f11268a19c0>

    file_repr in Objects/fileobject.c calls http://docs.python.org/c-api/unicode#PyUnicode_AsUnicodeEscapeString, equivalent to encode('unicode-escape'), so backslashes should be escaped.

    @merwok
    Copy link
    Member

    merwok commented Mar 1, 2012

    Duh, obviously that code branch is used only for unicode paths:

      >>> open('/tmp/t\nest', 'w')
      <open file '/tmp/t
      est', mode 'w' at 0x7f6f0f3dd9c0>
      >>> open(u'/tmp/t\nest', 'w')
      <open file u'/tmp/t\nest', mode 'w' at 0x7f6f0f3dda50>

    There does not seem to be something similar in http://docs.python.org/c-api/string, so I guess one would have to create intermediary objects to decode str to unicode, transform with unicode-escape and convert back to str.

    @ezio-melotti
    Copy link
    Member

    The attached patch seems to do the trick (not sure if it's the best way to fix the issue though):
    >>> open('woo\raa')
    <open file 'woo\raa', mode 'r' at 0xb77c2aa8>
    >>> open('woo\ra\'a', 'w')
    <open file "woo\ra'a", mode 'w' at 0xb77c2b88>
    >>> open('woo\ra\'a"', 'w')
    <open file 'woo\ra\'a"', mode 'w' at 0xb77c2b18>
    >>> 

    It's more or less equivalent to:

    • return "<open file '%s', mode '%s' at %p>" % (fname, mode, addr)
      + return "<open file %s, mode '%s' at %p>" % (repr(fname), mode, addr)

    @pitrou
    Copy link
    Member

    pitrou commented Mar 1, 2012

    1. PyObject_Repr() should IMO be preferred (it's the abstract, high-level function).
    2. You must check the result for NULL before calling PyString_AsString() on it.

    @pjenvey
    Copy link
    Member

    pjenvey commented Mar 2, 2012

    I think you want to decref the result of PyObject_Repr after the fact, too

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 11, 2012

    New changeset 6c1964dee98b by Ezio Melotti in branch '2.7':
    bpo-14161: fix the __repr__ of file objects to escape the file name.
    http://hg.python.org/cpython/rev/6c1964dee98b

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 11, 2012

    New changeset 86c749151660 by Ezio Melotti in branch '2.7':
    bpo-14161: fix compile error under Windows.
    http://hg.python.org/cpython/rev/86c749151660

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 12, 2012

    New changeset 6b1fad34d893 by Ezio Melotti in branch '2.7':
    bpo-14161: fix test failures on Windows.
    http://hg.python.org/cpython/rev/6b1fad34d893

    @ezio-melotti
    Copy link
    Member

    Not sure why this is still open -- probably I was just waiting for the buildbots and forgot it open. The issue seems fixed, so I'm going to close it now.

    @ezio-melotti ezio-melotti self-assigned this Oct 1, 2012
    @RonnyPfannschmidt
    Copy link
    Mannequin Author

    RonnyPfannschmidt mannequin commented Oct 2, 2012

    wtf? you made it possible to return NULL in some case

    @RonnyPfannschmidt
    Copy link
    Mannequin Author

    RonnyPfannschmidt mannequin commented Oct 2, 2012

    sorry for the buzz, i got myself up to date on the c api now

    why is there still the unicode case?
    the PyObject_Repr variant should work fine in both cases

    @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

    4 participants