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

bug-1819628: reject incoming crash reports by Android_PackageName #962

Merged
merged 1 commit into from
Jan 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
70 changes: 67 additions & 3 deletions antenna/throttler.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ class Config:
doc="Python dotted path to list of supported products",
parser=parse_attribute,
)
product_packagenames = Option(
default="antenna.throttler.MOZILLA_PRODUCT_PACKAGENAMES",
doc="Python dotted path to map of product -> list of supported packagenames",
parser=parse_attribute,
)

def __init__(self, config):
self.config = config.with_options(self)
Expand Down Expand Up @@ -254,10 +259,34 @@ def match_unsupported_product(throttler, data):
"""Match unsupported products."""
products = throttler.config("products")
product_name = safe_get(data, "ProductName")
is_not_supported = product_name not in products
is_not_supported = products and product_name not in products

if is_not_supported:
logger.info("ProductName rejected: %r", product_name)
return True
return False
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cleaned up the logic here a little, but it should do the same thing it did before.



def match_unsupported_android_packagename(throttler, data):
"""Match unsupported Android_PackageName values"""
product_name = safe_get(data, "ProductName")

packagenames = throttler.config("product_packagenames").get(product_name, [])
packagename = safe_get(data, "Android_PackageName", default=None)

if products and is_not_supported:
logger.info("ProductName rejected: %r" % product_name)
is_not_supported = packagenames and packagename not in packagenames

if is_not_supported:
# NOTE(willkg): safe_get converts None to "None", so we need to test for that
# here
if packagename == "None":
logger.info(
"Android_PackageName rejected: %s no Android_PackageName", product_name
)
else:
logger.info(
"Android_PackageName rejected: %s %r", product_name, packagename
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having this logged allows us to later look at the logs when someone asks us, "are we rejecting packagename xyz?" and then we can say "yes, and we got 1 billion of them yesterday!!!!"

)
return True
return False

Expand Down Expand Up @@ -332,6 +361,33 @@ def match_unsupported_windows(throttler, data):
]


#: This accepts crash reports for all product packagenames
ALL_PRODUCT_PACKAGENAMES = {}


# Supported packagenames values; these have to match the Android_PackageName of the
# incoming crash report
MOZILLA_PRODUCT_PACKAGENAMES = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering whether we'd prefer to have this data in a configuration file rather than in the source code, but I guess either way is fine.

"Fenix": [
"org.mozilla.firefox",
"org.mozilla.firefox_beta",
# This is the Nightly version of Firefox Android, not to be confused with
# "org.mozilla.fenix.nightly", a retired version we no longer care about
"org.mozilla.fenix",
],
"Focus": [
"org.mozilla.focus",
"org.mozilla.focus.beta",
"org.mozilla.focus.nightly",
# This is the German version of Focus
"org.mozilla.klar",
],
"ReferenceBrowser": [
"org.mozilla.reference.browser",
],
}


#: Rule set to accept all incoming crashes
ACCEPT_ALL = [
# Accept everything
Expand Down Expand Up @@ -378,6 +434,14 @@ def match_unsupported_windows(throttler, data):
condition=match_unsupported_product,
result=REJECT,
),
# bug #1819628: Reject crash reports for unsupported packagenames; this does nothing
# if the dict of product -> packagename list is empty
Rule(
rule_name="unsupported_packagename",
key="*",
condition=match_unsupported_android_packagename,
result=REJECT,
),
# Accept crash reports submitted through about:crashes
Rule(
rule_name="throttleable_0",
Expand Down
98 changes: 98 additions & 0 deletions tests/unittest/test_throttler.py
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,104 @@ def test_productname_no_unsupported_products(self):
# by any of the rules, so it gets caught up by the last rule
assert throttler.throttle(raw_crash) == (ACCEPT, "accept_everything", 100)

@pytest.mark.parametrize(
"productname, packagename",
[
# Test packagename is accepted
("Fenix", "org.mozilla.firefox"),
("Focus", "org.mozilla.focus"),
("ReferenceBrowser", "org.mozilla.reference.browser"),
# Test a product that has no Android packages
("Thunderbird", "org.example.fork"),
],
)
def test_packagename_accept(self, caplog, productname, packagename):
"""Verify supported packagename is accepted"""
with caplog.at_level(logging.INFO, logger="antenna"):
# Need a throttler with the default configuration which includes product ->
# supported packagenames
throttler = Throttler(ConfigManager.from_dict({}))
raw_crash = {}
raw_crash["ProductName"] = productname
raw_crash["Android_PackageName"] = packagename

assert throttler.throttle(raw_crash) == (ACCEPT, "accept_everything", 100)

@pytest.mark.parametrize(
"productname, packagename",
[
# Test empty packagename is rejected
("Fenix", ""),
# Test supported product / unsupported packagename is rejected
("Fenix", "org.example.fork"),
("Focus", "org.example.fork"),
],
)
def test_packagename_reject(self, caplog, productname, packagename):
"""Verify unsupported packagename is rejected"""
with caplog.at_level(logging.INFO, logger="antenna"):
# Need a throttler with the default configuration which includes product ->
# supported packagenames
throttler = Throttler(ConfigManager.from_dict({}))
raw_crash = {}
raw_crash["ProductName"] = productname
raw_crash["Android_PackageName"] = packagename

assert throttler.throttle(raw_crash) == (
REJECT,
"unsupported_packagename",
100,
)
assert caplog.record_tuples == [
(
"antenna.throttler",
logging.INFO,
f"Android_PackageName rejected: {productname} '{packagename}'",
)
]

def test_packagename_reject_no_packagename(self, caplog):
"""Verify missing packagename for product that requires it is rejected"""
with caplog.at_level(logging.INFO, logger="antenna"):
# Need a throttler with the default configuration which includes product ->
# supported packagenames
throttler = Throttler(ConfigManager.from_dict({}))
raw_crash = {}
raw_crash["ProductName"] = "Fenix"

assert throttler.throttle(raw_crash) == (
REJECT,
"unsupported_packagename",
100,
)
assert caplog.record_tuples == [
(
"antenna.throttler",
logging.INFO,
"Android_PackageName rejected: Fenix no Android_PackageName",
)
]

def test_packagename_no_unsupported_packagenames(self):
"""Verify packagename rule doesn't do anything if using ALL_PRODUCT_PACKAGENAMES"""
throttler = Throttler(
ConfigManager.from_dict(
{"PRODUCT_PACKAGENAMES": "antenna.throttler.ALL_PRODUCT_PACKAGENAMES"}
)
)
# Test supported product with no Android_PackageName, but no
# product_packagenames set
raw_crash = {"ProductName": "Fenix"}
assert throttler.throttle(raw_crash) == (ACCEPT, "accept_everything", 100)

# Test supported product with unsupported Android_PackageName, but no
# product_packagenames set
raw_crash = {
"ProductName": "Fenix",
"Android_PackageName": "org.example.fork",
}
assert throttler.throttle(raw_crash) == (ACCEPT, "accept_everything", 100)

def test_throttleable(self, throttler):
# Throttleable=0 should match
raw_crash = {"ProductName": "Test", "Throttleable": "0"}
Expand Down