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-1901997: handle invalid BuildID for redacted crash reports #6777

Merged
merged 2 commits into from
Oct 31, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -1213,6 +1213,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 @@ -24,6 +24,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 @@ -109,6 +110,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