Fix #230: Make REPORT_PERCENTAGE a float #242
Conversation
django.core.checks.registry.CheckRegistry.register is now typed correctly.
| ``report-uri``. An **integer** between 0 and 100 (0 = no reports at all). | ||
| Ignored if ``report-uri`` isn't set. | ||
| Percentage of requests that should see the ``report-uri`` directive. Use this to throttle the | ||
| number of CSP violation reports made to your ``report-uri``. A **float** between 0.0 and 100.0 |
There was a problem hiding this comment.
A float between 0.0 and 100.0
Won't a float of 100.0 give us a REPORT_PERCENTAGE of 10000.0? Would be good to warn about invalid values maybe?
config["REPORT_PERCENTAGE"] = _REPORT_PERCENTAGE * 100
There was a problem hiding this comment.
That * 100 is only when converting from the old settings config when using CSP_REPORT_PERCENTAGE which was a float between 0 and 1.
There was a problem hiding this comment.
Would it be worthwhile to add a Django check in checks.py to validate some of the settings? Like this value being 0 <= x <= 100 and possibly some others that makes sense and emit a warning?
There was a problem hiding this comment.
I think it'd be nice, to help avoid surprises, but can be a follow-on thing or a ticket for someone in the community to pick up?
| @@ -23,7 +23,8 @@ def get_policy_parts(self, request: HttpRequest, response: HttpResponseBase, rep | |||
| if policy is None: | |||
| return policy_parts | |||
|
|
|||
| remove_report = random.randint(0, 99) >= policy.get("REPORT_PERCENTAGE", 100) | |||
| # `random.random` returns a value in the range [0.0, 1.0) so all values will be < 100.0. | |||
There was a problem hiding this comment.
| # `random.random` returns a value in the range [0.0, 1.0) so all values will be < 100.0. | |
| # `random.random` returns a value in the range [0.0, 1.0] so all values will be < 100.0. |
There was a problem hiding this comment.
That was actually intentional to denote that 1.0 isn't included in the range that random.random() can return.
| ``report-uri``. An **integer** between 0 and 100 (0 = no reports at all). | ||
| Ignored if ``report-uri`` isn't set. | ||
| Percentage of requests that should see the ``report-uri`` directive. Use this to throttle the | ||
| number of CSP violation reports made to your ``report-uri``. A **float** between 0.0 and 100.0 |
There was a problem hiding this comment.
I think it'd be nice, to help avoid surprises, but can be a follow-on thing or a ticket for someone in the community to pick up?
No description provided.