Skip to content

Commit

Permalink
bug-1901997: handle invalid BuildID for redacted crash reports (#6777)
Browse files Browse the repository at this point in the history
* Add support for --pause to process_crashes.sh

This makes it possible to adjust a crash report for testing a specific
processing scenario.

* bug-1901997: handle invalid BuildID for redacted crash reports

If the user viewing the crash report does not have protected data access
and the crash report has a crash annotation value that doesn't match our
schema, then the redaction process can fail. In this scenario, Crash
Stats can't recover and redact the document properly.

In bug 1901997, we were seeing this scenario when BuildID=null. Because
crash annotation data comes from the crash reporter which might be
running on a machine that has bad hardware, this scenario can happen
with other fields.

These changes handle that scenario and shows a page indicating the crash
report is malformed and providing a link the user can use to report the
malformed crash report.
  • Loading branch information
willkg authored Oct 31, 2024
1 parent f60fb36 commit 165fd98
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 3 deletions.
31 changes: 28 additions & 3 deletions bin/process_crashes.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,23 @@
#
# Usage: ./bin/process_crashes.sh
#
# This should be run inside the app container.
#
# You can use it with fetch_crashids. For example:
#
# socorro-cmd fetch_crashids --num=1 | ./bin/process_crashes.sh
#
# Make sure to run the processor to do the actual processing.
# You can pause after fetching crash ids and before uploading them to the cloud
# storage bucket. This lets you adjust the crash reports.
#
# ./bin/process_crashes.sh --pause afd59adc-1cfd-4aa3-af2f-6b9ed0241029
#
# Note: This should be called from inside a container.
# Make sure to run the processor to do the actual processing.

set -euo pipefail

DATADIR=./crashdata_tryit_tmp
PAUSE=0

function cleanup {
# Cleans up files generated by the script
Expand All @@ -29,10 +35,24 @@ function cleanup {
# Set up cleanup function to run on script exit
trap cleanup EXIT

# If there's at least one argument, check for options
if [[ $# -ne 0 ]]; then
if [[ $1 == "--pause" ]]; then
PAUSE=1
shift
fi
fi

# Check for crash ids
if [[ $# -eq 0 ]]; then
if [ -t 0 ]; then
# If stdin is a terminal, then there's no input
echo "Usage: process_crashes.sh CRASHID [CRASHID ...]"
echo "Usage: process_crashes.sh [--pause] CRASHID [CRASHID ...]"
exit 1
fi

if [[ "${PAUSE}" == "1" ]]; then
echo "Can't use PAUSE and process crash ids from stdin. Exiting."
exit 1
fi

Expand All @@ -45,6 +65,11 @@ mkdir "${DATADIR}" || echo "${DATADIR} already exists."
# Pull down the data for all crashes
./socorro-cmd fetch_crash_data "${DATADIR}" $@

# Pause (if requested) to let user make changes to crash data
if [[ "${PAUSE}" == "1" ]]; then
read -r -n 1 -p "Pausing... Any key to continue." pauseinput
fi

# Make the bucket and sync contents
./socorro-cmd gcs create "${CRASHSTORAGE_GCS_BUCKET}"
./socorro-cmd gcs upload "${DATADIR}" "${CRASHSTORAGE_GCS_BUCKET}"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{% extends "crashstats_base.html" %}

{% block content %}
<div id="mainbody">
<div class="page-heading">
<h2>Crash Report Malformed</h2>
</div>
<div class="panel">
<div class="body">
<p>
The crash report you requested is malformed in some way such that it
cannot be shown.
</p>
<p>
If you need to see this crash report, please
<a href="https://bugzilla.mozilla.org/enter_bug.cgi?{{ make_query_string(product='Socorro', component='Webapp', bug_file_loc=request.build_absolute_uri()) }}">submit a bug</a>
describing what happened, and please include the URL for this page.
</p>
</div>
</div>
</div>
{% endblock %}
28 changes: 28 additions & 0 deletions webapp/crashstats/crashstats/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1230,6 +1230,34 @@ def test_raw_crash_not_found(self, client, db, storage_helper):
assert response.status_code == 404
assert "Crash Report Not Found" in smart_str(response.content)

def test_raw_crash_malformed(self, client, db, storage_helper):
crash_id, raw_crash, processed_crash = build_crash_data()

# If the BuildID is None, that will cause the reducer to raise an
# InvalidDocumentError when someone who doesn't have protected data access
# tries to view it. Bug #1901997.
raw_crash["BuildID"] = None

bucket = storage_helper.get_crashstorage_bucket()
raw_key = build_keys("raw_crash", crash_id)[0]
storage_helper.upload(
bucket_name=bucket, key=raw_key, data=dict_to_str(raw_crash).encode("utf-8")
)

validate_instance(processed_crash, PROCESSED_CRASH_SCHEMA)
processed_key = build_keys("processed_crash", crash_id)[0]
storage_helper.upload(
bucket_name=bucket,
key=processed_key,
data=dict_to_str(processed_crash).encode("utf-8"),
)

url = reverse("crashstats:report_index", args=[crash_id])
response = client.get(url)

assert response.status_code == 500
assert "Crash Report Malformed" in smart_str(response.content)

def test_processed_crash_not_found(self, client, db, storage_helper, queue_helper):
crash_id, raw_crash, processed_crash = build_crash_data()

Expand Down
14 changes: 14 additions & 0 deletions webapp/crashstats/crashstats/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from crashstats.crashstats.decorators import track_view, pass_default_context
from crashstats.supersearch.models import SuperSearchFields
from socorro.external.crashstorage_base import CrashIDNotFound
from socorro.lib.libsocorrodataschema import InvalidDocumentError
from socorro.signature.generator import SignatureGenerator
from socorro.signature.utils import convert_to_crash_data

Expand Down Expand Up @@ -110,6 +111,19 @@ def report_index(request, crash_id, default_context=None):
raw_api.api_user = request.user
try:
context["raw"] = raw_api.get(crash_id=crash_id)
except InvalidDocumentError:
# If the user does not have protected data access, then the document is
# redacted. However, if it's invalid because the world is a terrible place and
# things working right is not the norm, then we want to display a "This crash
# report is broken in some way. Please report a bug." message instead
# of throwing an HTTP 500 error. We definitely can't gloss over this because
# the brokenness prevents the document from being redacted correctly.
return render(
request,
"crashstats/report_index_malformed_raw_crash.html",
context,
status=500,
)
except CrashIDNotFound:
# If the raw crash can't be found, we can't do much.
return render(
Expand Down

0 comments on commit 165fd98

Please sign in to comment.