From df772c5b89e8878a4d3cce720455d5c1df4b4781 Mon Sep 17 00:00:00 2001 From: Armen Zambrano G <44410+armenzg@users.noreply.github.com> Date: Wed, 18 Jun 2025 10:05:25 -0400 Subject: [PATCH] fix(browser-reports): Support the correct media type --- .../endpoints/browser_reporting_collector.py | 19 ++++++++++++++----- .../test_browser_reporting_collector.py | 16 +++++++++++++++- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/sentry/issues/endpoints/browser_reporting_collector.py b/src/sentry/issues/endpoints/browser_reporting_collector.py index 92a634061d9405..a14153e0595937 100644 --- a/src/sentry/issues/endpoints/browser_reporting_collector.py +++ b/src/sentry/issues/endpoints/browser_reporting_collector.py @@ -1,4 +1,5 @@ import logging +from typing import Any from django.http import HttpResponse from django.views.decorators.csrf import csrf_exempt @@ -14,6 +15,15 @@ logger = logging.getLogger(__name__) +class BrowserReportsJSONParser(JSONParser): + """ + Custom parser for browser Reporting API that handles the application/reports+json content type. + This extends JSONParser since the content is still JSON, just with a different media type. + """ + + media_type = "application/reports+json" + + @all_silo_endpoint class BrowserReportingCollectorEndpoint(Endpoint): """ @@ -23,18 +33,17 @@ class BrowserReportingCollectorEndpoint(Endpoint): """ permission_classes = () - # TODO: Do we need to specify this parser? Content type will be `application/reports+json`, so - # it might just work automatically. - parser_classes = [JSONParser] + # Support both standard JSON and browser reporting API content types + parser_classes = [BrowserReportsJSONParser, JSONParser] publish_status = { "POST": ApiPublishStatus.PRIVATE, } owner = ApiOwner.ISSUES - # TODO: It's unclear if either of these decorators is necessary + # CSRF exemption and CORS support required for Browser Reporting API @csrf_exempt @allow_cors_options - def post(self, request: Request, *args, **kwargs) -> HttpResponse: + def post(self, request: Request, *args: Any, **kwargs: Any) -> HttpResponse: if not options.get("issues.browser_reporting.collector_endpoint_enabled"): return HttpResponse(status=404) diff --git a/tests/sentry/api/endpoints/test_browser_reporting_collector.py b/tests/sentry/api/endpoints/test_browser_reporting_collector.py index e169e901ff1bd3..adba2f9bb5c629 100644 --- a/tests/sentry/api/endpoints/test_browser_reporting_collector.py +++ b/tests/sentry/api/endpoints/test_browser_reporting_collector.py @@ -39,7 +39,9 @@ def test_404s_by_default(self): def test_logs_request_data_if_option_enabled( self, mock_logger_info: MagicMock, mock_metrics_incr: MagicMock ): - response = self.client.post(self.url, self.report_data) + response = self.client.post( + self.url, self.report_data, content_type="application/reports+json" + ) assert response.status_code == status.HTTP_200_OK mock_logger_info.assert_any_call( @@ -48,3 +50,15 @@ def test_logs_request_data_if_option_enabled( mock_metrics_incr.assert_any_call( "browser_reporting.raw_report_received", tags={"type": self.report_data["type"]} ) + + @override_options({"issues.browser_reporting.collector_endpoint_enabled": True}) + @patch("sentry.issues.endpoints.browser_reporting_collector.metrics.incr") + def test_rejects_invalid_content_type(self, mock_metrics_incr: MagicMock): + """Test that the endpoint rejects invalid content type and does not call the browser reporting metric""" + response = self.client.post(self.url, self.report_data, content_type="bad/type/json") + + assert response.status_code == status.HTTP_415_UNSUPPORTED_MEDIA_TYPE + # Verify that the browser_reporting.raw_report_received metric was not called + # Check that none of the calls were for the browser_reporting.raw_report_received metric + for call in mock_metrics_incr.call_args_list: + assert call[0][0] != "browser_reporting.raw_report_received"