-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat(browser-reports): Validate both browser report formats #93917
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
Conversation
This validates both the [Working Draft](https://www.w3.org/TR/reporting-1/#concept-reports) and the [Editor's Draft](https://w3c.github.io/reporting/#concept-reports) formats.
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #93917 +/- ##
==========================================
- Coverage 88.04% 86.86% -1.18%
==========================================
Files 10340 10340
Lines 597231 597334 +103
Branches 23208 23208
==========================================
- Hits 525805 518887 -6918
- Misses 70921 77942 +7021
Partials 505 505 |
] | ||
|
||
|
||
@dataclass | ||
class BrowserReport: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would only do static typing but not runtime verification.
"coop", # Cross-Origin-Opener-Policy violations | ||
"document-policy-violation", # Document Policy violations | ||
"permissions-policy", # Permissions Policy violations | ||
BROWSER_REPORT_TYPES = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I know. There may be more.
age = serializers.IntegerField(required=False) | ||
timestamp = serializers.IntegerField(required=False, min_value=0) | ||
|
||
def validate_timestamp(self, value: int) -> int: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two methods validate that we're dealing with the format for one spec or the other.
owner = ApiOwner.ISSUES | ||
|
||
# CSRF exemption and CORS support required for Browser Reporting API | ||
@csrf_exempt | ||
@allow_cors_options | ||
def post(self, request: Request, *args: Any, **kwargs: Any) -> HttpResponse: | ||
def post(self, request: Request, *args: Any, **kwargs: Any) -> Response: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's preferred to use Response over Django's HttpResponse.
This validates both the Working Draft and the Editor's Draft formats.
Fixes ID-730 - Accept current and upcoming data model.