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

While parsing email message id: UnboundLocalError #82879

Closed
NikitaHoffmann mannequin opened this issue Nov 5, 2019 · 10 comments
Closed

While parsing email message id: UnboundLocalError #82879

NikitaHoffmann mannequin opened this issue Nov 5, 2019 · 10 comments
Labels
3.8 only security fixes 3.9 only security fixes topic-email type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@NikitaHoffmann
Copy link
Mannequin

NikitaHoffmann mannequin commented Nov 5, 2019

BPO 38698
Nosy @warsaw, @bitdancer, @maxking, @miss-islington, @tirkarthi
PRs
  • bpo-38698: Prevent UnboundLocalError to pop up in parse_message_id #17277
  • [3.8] bpo-38698: Prevent UnboundLocalError to pop up in parse_message_id (GH-17277) #17476
  • bpo-38698: Add a new InvalidMessageID token to email header parser. #17503
  • [3.8] bpo-38698: Add a new InvalidMessageID token to email header parser. (GH-17503) #17514
    		•
    Files
  • samplescript.py: Sample script
    		•

    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-12-09.02:36:06.916>
created_at = <Date 2019-11-05.13:03:28.325>
labels = ['3.8', 'expert-email', 'type-crash', '3.9']
title = 'While parsing email message id: UnboundLocalError'
updated_at = <Date 2019-12-09.02:36:06.916>
user = 'https://bugs.python.org/NikitaHoffmann'

    bugs.python.org fields:

    activity = <Date 2019-12-09.02:36:06.916>
actor = 'maxking'
assignee = 'none'
closed = True
closed_date = <Date 2019-12-09.02:36:06.916>
closer = 'maxking'
components = ['email']
creation = <Date 2019-11-05.13:03:28.325>
creator = 'Nikita Hoffmann'
dependencies = []
files = ['48696']
hgrepos = []
issue_num = 38698
keywords = ['patch']
message_count = 10.0
messages = ['356030', '356032', '356108', '357405', '357407', '357836', '357874', '358044', '358046', '358049']
nosy_count = 6.0
nosy_names = ['barry', 'r.david.murray', 'maxking', 'miss-islington', 'xtreak', 'Nikita Hoffmann']
pr_nums = ['17277', '17476', '17503', '17514']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue38698'
versions = ['Python 3.8', 'Python 3.9']
    @NikitaHoffmann
    Copy link
    Mannequin Author

    NikitaHoffmann mannequin commented Nov 5, 2019

    Parsing an invalid email message id can throw a header parser error. A bug in parse_message_ still tries to append the unset token to a variable.

    File "/opt/python-3.8.0/lib/python3.8/email/header_value_parser.py", line 2116, in parse_message
    id
    message_id.append(token)
    UnboundLocalError: local variable 'token' referenced before assignment

    Version 3.7 is not affected.

    @NikitaHoffmann NikitaHoffmann mannequin added 3.8 only security fixes topic-email type-crash A hard crash of the interpreter, possibly with a core dump labels Nov 5, 2019
    @tirkarthi
    Copy link
    Member

    Thanks for the report. Can you please attach a sample script to reproduce this error?

    @tirkarthi
    Copy link
    Member

    This was also reported in bpo-38708 with the original code added in bpo-35805. Commenting out the except clause also doesn't raise any error in test suite so I assume the code path was not tested. Maybe the script could be added as part of the test suite.

    $ git diff
diff --git a/Lib/email/_header_value_parser.py b/Lib/email/_header_value_parser.py
index 1668b4a14e..9b6ca3a268 100644
--- a/Lib/email/_header_value_parser.py
+++ b/Lib/email/_header_value_parser.py
@@ -2110,10 +2110,10 @@ def parse_message_id(value):
     message_id = MessageID()
     try:
         token, value = get_msg_id(value)
+        message_id.append(token)
     except errors.HeaderParseError:
         message_id.defects.append(errors.InvalidHeaderDefect(
             "Expected msg-id but found {!r}".format(value)))
-    message_id.append(token)
     return message_id

    @tirkarthi tirkarthi added the 3.9 only security fixes label Nov 6, 2019
    @bitdancer
    Copy link
    Member

    More tests are always good :)

    The "correct" solution here (as far as I remember, its has been a while since I've had time to even looked at the _header_value_parser code) would be to add a new 'invalid-msg-id' token, and do this:

        message_id = MessageID()
    try:
        token, value = get_msg_id(value)
        message_id.append(token)
    except HeaderParseError as ex:
        message_id = InvalidMessageID(value)
        message_id.defects.append(InvalidHeaderDefect(
            f"Invalid msg_id: {ex}"))
    return message_id

    @bitdancer
    Copy link
    Member

    Actually, the success path there should also check that value is empty, and if it is not register a defect for that as well.

    @miss-islington
    Copy link
    Contributor

    New changeset bb81549 by Miss Islington (bot) (Claudiu Popa) in branch 'master':
    bpo-38698: Prevent UnboundLocalError to pop up in parse_message_id (GH-17277)
    bb81549

    @miss-islington
    Copy link
    Contributor

    New changeset e21aa61 by Miss Islington (bot) in branch '3.8':
    bpo-38698: Prevent UnboundLocalError to pop up in parse_message_id (GH-17277)
    e21aa61

    @maxking
    Copy link
    Contributor

    maxking commented Dec 9, 2019

    New changeset 68157da by Abhilash Raj in branch 'master':
    bpo-38698: Add a new InvalidMessageID token to email header parser. (GH-17503)
    68157da

    @miss-islington
    Copy link
    Contributor

    New changeset f66f4a0 by Miss Islington (bot) in branch '3.8':
    bpo-38698: Add a new InvalidMessageID token to email header parser. (GH-17503)
    f66f4a0

    @maxking
    Copy link
    Contributor

    maxking commented Dec 9, 2019

    Closing this as fixed.

    @maxking maxking closed this as completed Dec 9, 2019
    @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-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @bitdancer @maxking @tirkarthi @miss-islington