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

Fix #231: report percentage of 100% should always report #236

Merged
merged 1 commit into from
Jul 11, 2024

Conversation

robhudson
Copy link
Member

This also updates the RateLimited CSPMiddleware to remove both report-uri and report-to directives based on report percentage.

This also updates the RateLimited CSPMiddleware to remove both
`report-uri` and `report-to` directives based on report percentage.
@robhudson robhudson requested a review from stevejalim July 10, 2024 19:48
Copy link
Contributor

@stevejalim stevejalim left a comment

Choose a reason for hiding this comment

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

r+wc

include_report_uri = random.randint(0, 100) < report_percentage
if not include_report_uri:
replace["report-uri"] = None
remove_report = random.randint(0, 99) >= report_percentage
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's my brain but I find this way around harder to track in my head. Maybe worth a comment to clarify?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ha, that's interesting. I was trying to remove the if not include and simplify to if remove since the code block inside the conditional removes the reporting. But perhaps I'm overlooking something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants