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

Jump to bottom

IndexError in get_bare_quoted_string #81672

Closed
maxking opened this issue Jul 3, 2019 · 7 comments
Closed

IndexError in get_bare_quoted_string #81672

maxking opened this issue Jul 3, 2019 · 7 comments
Labels
3.8 only security fixes 3.9 only security fixes topic-email type-bug An unexpected behavior, bug, or error

Comments

@maxking
Copy link
Contributor

maxking commented Jul 3, 2019

BPO 37491
Nosy @warsaw, @terryjreedy, @bitdancer, @matrixise, @maxking
PRs
  • bpo-37491: Fix IndexError when parsing unexpectedly ending quoted-string. #14813
  • [3.7] bpo-37491: Fix IndexError when parsing unexpectedly ending quoted-string. (GH-14813) #14820
  • [3.8] bpo-37491: Fix IndexError when parsing unexpectedly ending quoted-string. (GH-14813) #14819
    		•
    Files
  • fix-indexerror.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 = None
closed_at = <Date 2019-08-01.12:28:25.782>
created_at = <Date 2019-07-03.08:19:31.022>
labels = ['type-bug', '3.8', 'expert-email', '3.9']
title = 'IndexError in get_bare_quoted_string'
updated_at = <Date 2019-08-01.12:28:25.781>
user = 'https://github.com/maxking'

    bugs.python.org fields:

    activity = <Date 2019-08-01.12:28:25.781>
actor = 'r.david.murray'
assignee = 'none'
closed = True
closed_date = <Date 2019-08-01.12:28:25.782>
closer = 'r.david.murray'
components = ['email']
creation = <Date 2019-07-03.08:19:31.022>
creator = 'maxking'
dependencies = []
files = ['48462']
hgrepos = []
issue_num = 37491
keywords = ['patch']
message_count = 7.0
messages = ['347190', '347454', '347529', '347565', '347566', '347572', '348842']
nosy_count = 5.0
nosy_names = ['barry', 'terry.reedy', 'r.david.murray', 'matrixise', 'maxking']
pr_nums = ['14813', '14820', '14819']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue37491'
versions = ['Python 3.8', 'Python 3.9']
    @maxking
    Copy link
    Contributor Author

    maxking commented Jul 3, 2019 

    from email.parser import BytesParser, Parser
from email.policy import default

payload = 'Content-Type:x;\x1b*="\'G\'\\"""""'
msg = Parser(policy=default).parsestr(payload)
print(msg.get('content-type'))

    When trying to review PR for bpo-37461, I found another bug where an IndexError
    is raised if there aren't closing quote characters in the input message:

    Suggested patch:

    @@ -1191,7 +1192,7 @@ def get_bare_quoted_string(value):
             "expected '\"' but found '{}'".format(value))
     bare_quoted_string = BareQuotedString()
     value = value[1:]
-    if value[0] == '"':
+    if value and value[0] == '"':
         token, value = get_qcontent(value)
         bare_quoted_string.append(token)
     while value and value[0] != '"':

    @maxking maxking added type-crash A hard crash of the interpreter, possibly with a core dump 3.8 only security fixes 3.9 only security fixes topic-email labels Jul 3, 2019
    @terryjreedy
    Copy link
    Member

    An Python exception is not a crash; a crash is the program stopping without an exception and proper cleanup.

    If s is a string (rather than, for instance, None),
    s and (s[0] == char) is equivalent to s[0:1] == char

    @terryjreedy terryjreedy added type-bug An unexpected behavior, bug, or error and removed type-crash A hard crash of the interpreter, possibly with a core dump labels Jul 6, 2019
    @maxking
    Copy link
    Contributor Author

    maxking commented Jul 9, 2019

    Thanks for the explanation Terry!

    In this case, value becomes None (I think), which causes the IndexError.

    @terryjreedy
    Copy link
    Member

    To avoid such questions, bug reports should contain exception messages and usually at least some of the tracebacks.

    >>> ''[0]
Traceback (most recent call last):
  File "<pyshell#0>", line 1, in <module>
    ''[0]
IndexError: string index out of range
>>> x=None
>>> x[0]
Traceback (most recent call last):
  File "<pyshell#3>", line 1, in <module>
    x[0]
TypeError: 'NoneType' object is not subscriptable

    IndexError should mean object was indexable.
    Operations on None should give TypeError.

    @terryjreedy
    Copy link
    Member

    Also, quotes should be attributed to a file and patch snipped should indicate the target.

    As for the bug, the author(s) of the expressions "value[1:]" and "value[0]" presumably *expected* value to initially have length 2 so that it would be be non-empty after clipping. In the absence of additional information, it is possible that the bug is in the unquoted code that produced value. This is potentially true whenever a function or expession raises.

    @maxking
    Copy link
    Contributor Author

    maxking commented Jul 9, 2019

    I just wanted to report before I forgot and hence missed some details, turns out the bug report was slightly wrong too. The testcase I provided wasn't right.

    Here is the right test case to reproduce the exception with master:

    # bpo_37491.py
from email.parser import BytesParser, Parser
from email.policy import default

payload = 'Content-Type:"'
msg = Parser(policy=default).parsestr(payload)
print(msg.get('content-type'))
    $ ./python bpo_37491.py                                                                                                                                                              
Traceback (most recent call last):
  File "bpo_37491.py", line 5, in <module>
    msg = Parser(policy=default).parsestr(payload)
  File "/home/maxking/Documents/cpython/Lib/email/parser.py", line 68, in parsestr
    return self.parse(StringIO(text), headersonly=headersonly)
  File "/home/maxking/Documents/cpython/Lib/email/parser.py", line 58, in parse
    return feedparser.close()
  File "/home/maxking/Documents/cpython/Lib/email/feedparser.py", line 187, in close
    self._call_parse()
  File "/home/maxking/Documents/cpython/Lib/email/feedparser.py", line 180, in _call_parse
    self._parse()
  File "/home/maxking/Documents/cpython/Lib/email/feedparser.py", line 256, in _parsegen
    if self._cur.get_content_type() == 'message/delivery-status':
  File "/home/maxking/Documents/cpython/Lib/email/message.py", line 578, in get_content_type
    value = self.get('content-type', missing)
  File "/home/maxking/Documents/cpython/Lib/email/message.py", line 471, in get
    return self.policy.header_fetch_parse(k, v)
  File "/home/maxking/Documents/cpython/Lib/email/policy.py", line 163, in header_fetch_parse
    return self.header_factory(name, value)
  File "/home/maxking/Documents/cpython/Lib/email/headerregistry.py", line 602, in __call__
    return self[name](name, value)
  File "/home/maxking/Documents/cpython/Lib/email/headerregistry.py", line 197, in __new__
    cls.parse(value, kwds)
  File "/home/maxking/Documents/cpython/Lib/email/headerregistry.py", line 447, in parse
    kwds['decoded'] = str(parse_tree)
  File "/home/maxking/Documents/cpython/Lib/email/_header_value_parser.py", line 126, in __str__
    return ''.join(str(x) for x in self)
  File "/home/maxking/Documents/cpython/Lib/email/_header_value_parser.py", line 126, in <genexpr>
    return ''.join(str(x) for x in self)
  File "/home/maxking/Documents/cpython/Lib/email/_header_value_parser.py", line 796, in __str__
    for name, value in self.params:
  File "/home/maxking/Documents/cpython/Lib/email/_header_value_parser.py", line 770, in params
    value = param.param_value
  File "/home/maxking/Documents/cpython/Lib/email/_header_value_parser.py", line 679, in param_value
    return token.stripped_value
  File "/home/maxking/Documents/cpython/Lib/email/_header_value_parser.py", line 710, in stripped_value
    token = self[0]
IndexError: list index out of range

    This time I attached a correct patch instead of copy-pasting only the diff :)

    About IndexError, I agree, it must be an empty string value and not a None value.

    @matrixise
    Copy link
    Member

    @barry

    the 3 PR have been merged, do you think we could close this issue?

    Thank you

    @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
    No one assigned
    Labels
    3.8 only security fixes 3.9 only security fixes topic-email type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @matrixise @bitdancer @maxking @terryjreedy