From 9474b600f6e04c2173f6a75a7c80d2982b4c3ff6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Bompard?= Date: Thu, 4 Jul 2024 08:38:21 +0200 Subject: [PATCH] Protect against unexpected needinfo field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Aurélien Bompard --- bugzilla2fedmsg/relay.py | 6 ++---- bugzilla2fedmsg/utils.py | 10 ++++++++++ tests/test_relay.py | 28 +++++++++++++++++++++++++++ tests/test_schemas.py | 41 +++++++++++----------------------------- 4 files changed, 51 insertions(+), 34 deletions(-) diff --git a/bugzilla2fedmsg/relay.py b/bugzilla2fedmsg/relay.py index f5dba0c..d95dc77 100644 --- a/bugzilla2fedmsg/relay.py +++ b/bugzilla2fedmsg/relay.py @@ -8,7 +8,7 @@ from fedora_messaging.exceptions import ConnectionException, PublishReturned from fedora_messaging.message import INFO -from .utils import convert_datetimes, email_to_fas +from .utils import convert_datetimes, email_to_fas, needinfo_email LOGGER = logging.getLogger(__name__) @@ -172,9 +172,7 @@ def _get_all_emails(self, body): emails.add(email) elif change["field"] == "flag.needinfo": # anyone for whom a 'needinfo' flag is set - # this is extracting the email from a value like: - # "? (senrique@redhat.com)" - email = change["added"].split("(", 1)[1].rsplit(")", 1)[0] + email = needinfo_email(change["added"]) if email: emails.add(email) diff --git a/bugzilla2fedmsg/utils.py b/bugzilla2fedmsg/utils.py index 2e27b06..74d9031 100644 --- a/bugzilla2fedmsg/utils.py +++ b/bugzilla2fedmsg/utils.py @@ -37,6 +37,16 @@ def convert_datetimes(obj): return obj +def needinfo_email(flag_content): + # this is extracting the email from a value like: + # "? (senrique@redhat.com)" + try: + return flag_content.split("(", 1)[1].rsplit(")", 1)[0] + except IndexError: + LOGGER.warning("Could not extract email from %r", flag_content) + return None + + def email_to_fas(email, fasjson): """Try to get a FAS username from an email address, return None if no FAS username is found""" if email.endswith("@fedoraproject.org"): diff --git a/tests/test_relay.py b/tests/test_relay.py index d320b0c..5558587 100644 --- a/tests/test_relay.py +++ b/tests/test_relay.py @@ -213,3 +213,31 @@ def test_publish_exception_connectionexception(testrelay, fakepublish, bug_creat assert fakepublish.call_count == 1 # check the logging worked assert "Error sending message" in caplog.text + + +def test_needinfo_removed(testrelay, fakepublish, bug_modify_message_four_changes): + bug_modify_message_four_changes["body"]["event"]["changes"][2] = { + "field": "cc", + "removed": "", + "added": "awilliam@redhat.com", + } + bug_modify_message_four_changes["body"]["event"]["changes"][3]["added"] = "" + testrelay.on_stomp_message( + bug_modify_message_four_changes["body"], bug_modify_message_four_changes["headers"] + ) + assert fakepublish.call_count == 1 + message = fakepublish.call_args[0][0] + assert message.body["agent_name"] is None + assert message.body["usernames"] == ["adamw"] + + +def test_needinfo_bad(testrelay, fakepublish, bug_modify_message_four_changes): + last_change = bug_modify_message_four_changes["body"]["event"]["changes"][3] + last_change["added"] = "some garbage" + try: + testrelay.on_stomp_message( + bug_modify_message_four_changes["body"], bug_modify_message_four_changes["headers"] + ) + except IndexError as e: + pytest.fail(e) + assert fakepublish.call_count == 1 diff --git a/tests/test_schemas.py b/tests/test_schemas.py index 1f3953d..8ea8e41 100644 --- a/tests/test_schemas.py +++ b/tests/test_schemas.py @@ -37,27 +37,25 @@ def test_bug_create_schema(fakepublish, bug_create_message, testrelay): assert message.product_name == "Fedora" assert ( message.summary - == "dgunchev@gmail.com filed a new bug RHBZ#1701391 'SELinux is preventing touch from 'write'...'" + == "dgunchev filed a new bug RHBZ#1701391 'SELinux is preventing touch from 'write'...'" ) assert ( str(message) - == "dgunchev@gmail.com filed a new bug RHBZ#1701391 'SELinux is preventing touch from 'write'...'" + == "dgunchev filed a new bug RHBZ#1701391 'SELinux is preventing touch from 'write'...'" ) assert message.url == "https://bugzilla.redhat.com/show_bug.cgi?id=1701391" assert ( message.app_icon == "https://bugzilla.redhat.com/extensions/RedHat/web/css/favicon.ico?v=0" ) - assert message.agent_name == "Doncho Gunchev" - assert message.app_name == "bugzilla2fedmsg" - # broken till we can do email2fas - assert message.usernames == [] + assert message.agent_name == "dgunchev" + assert message.app_name == "Bugzilla" + assert message.usernames == ["dgunchev", "lv"] assert message.packages == ["selinux-policy"] assert ( message.agent_avatar == "https://seccdn.libravatar.org/avatar/d4bfc5ec5260361c930aad299c8e14fe03af45109ea88e880a191851b8c83e7f?s=64&d=retro" ) assert message._primary_email == "dgunchev@gmail.com" - assert message._all_emails == ["dgunchev@gmail.com", "lvrabec@redhat.com"] def test_bug_modify_schema(fakepublish, bug_modify_message, testrelay): @@ -71,16 +69,7 @@ def test_bug_modify_schema(fakepublish, bug_modify_message, testrelay): message.summary == "mhroncok@redhat.com updated 'cc' on RHBZ#1699203 'python-pyramid-1.10.4 is available'" ) - # here we test both picking up an address from a 'cc' change - # event, and filtering out lists.fedoraproject.org addresses - assert message._all_emails == [ - "awilliam@redhat.com", - "mhroncok@redhat.com", - "upstream-release-monitoring@fedoraproject.org", - ] - # here we test that we can at least derive usernames from - # fedoraproject.org email addresses - assert message.usernames == ["upstream-release-monitoring"] + assert message.usernames == ["adamw", "upstream-release-monitoring"] def test_bug_modify_four_changes_schema(fakepublish, bug_modify_message_four_changes, testrelay): @@ -99,12 +88,7 @@ def test_bug_modify_four_changes_schema(fakepublish, bug_modify_message_four_cha message.summary == "zebob.m@gmail.com updated 'assigned_to', 'bug_status', 'cc', and 'flag.needinfo' on RHBZ#1702701 'Review Request: perl-Class-AutoClass - D...'" ) - # this tests gathering an address from a 'needinfo' change - assert message._all_emails == [ - "ppisar@redhat.com", - "rob@boberts.com", - "zebob.m@gmail.com", - ] + assert message.usernames == [] def test_bug_modify_two_changes_schema(fakepublish, bug_modify_message_four_changes, testrelay): @@ -146,10 +130,7 @@ def test_bug_modify_no_changes_schema(fakepublish, bug_modify_message, testrelay message.summary == "mhroncok@redhat.com updated something unknown on RHBZ#1699203 'python-pyramid-1.10.4 is available'" ) - assert message._all_emails == [ - "mhroncok@redhat.com", - "upstream-release-monitoring@fedoraproject.org", - ] + assert message.usernames == ["upstream-release-monitoring"] def test_comment_create_schema(fakepublish, comment_create_message, testrelay): @@ -176,7 +157,7 @@ def test_attachment_create_schema(fakepublish, attachment_create_message, testre message.validate() assert ( message.summary - == "peter@sonniger-tag.eu added attachment on RHBZ#1701353 '[abrt] gnome-software: gtk_widget_unpare...'" + == "peter added attachment on RHBZ#1701353 '[abrt] gnome-software: gtk_widget_unpare...'" ) @@ -191,7 +172,7 @@ def test_attachment_modify_schema(fakepublish, attachment_modify_message, testre message.validate() assert ( message.summary - == "joequant@gmail.com updated 'isobsolete' for attachment on RHBZ#1701766 'I2C_HID_QUIRK_NO_IRQ_AFTER_RESET caused ...'" + == "joe updated 'isobsolete' for attachment on RHBZ#1701766 'I2C_HID_QUIRK_NO_IRQ_AFTER_RESET caused ...'" ) @@ -213,7 +194,7 @@ def test_attachment_modify_no_changes_schema(fakepublish, attachment_modify_mess message.validate() assert ( message.summary - == "joequant@gmail.com updated something unknown for attachment on RHBZ#1701766 'I2C_HID_QUIRK_NO_IRQ_AFTER_RESET caused ...'" + == "joe updated something unknown for attachment on RHBZ#1701766 'I2C_HID_QUIRK_NO_IRQ_AFTER_RESET caused ...'" )