#37131 closed Cleanup/optimization (fixed)
Improvements to the security topic
| Reported by: | blighj | Owned by: | VIZZARD-X |
|---|---|---|---|
| Component: | Documentation | Version: | 6.0 |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
#28592 Had an original PR from Daniele, that improved the CSRF documentation, this has since been broken down and merged in smaller PRs. The remaining changes from the original PR do not relate to CSRF, they consist of a new note at the top and rewording of the XSS documentation. These changes can be considered on their own merit, rather than as part of the old ticket.
I've extracted the changes into the attached patch.
Attachments (1)
Change History (19)
by , 3 months ago
| Attachment: | security_xss.patch added |
|---|
comment:1 by , 3 months ago
| Summary: | Improvements to XSS section of security topic → Improvements to the security topic |
|---|
comment:2 by , 3 months ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:3 by , 3 months ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
comment:4 by , 3 months ago
| Owner: | removed |
|---|---|
| Status: | assigned → new |
comment:5 by , 3 months ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
comment:6 by , 3 months ago
| Owner: | removed |
|---|---|
| Status: | assigned → new |
comment:7 by , 3 months ago
comment:8 by , 3 months ago
The ticket is not assigned, if you wanted you could take it on, create a PR out of the patch?
comment:9 by , 3 months ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
Thank you for the patch @blighj. I'm open to take this, will be opening a PR for this shortly.
comment:10 by , 2 months ago
The PR is up and ready for review: PR
I've created the pull request based on the attached security_xss.patch, thanks to James (@blighj) for the initial patch and for breaking the documentation down so clearly and for others for their reviews.
comment:11 by , 2 months ago
| Patch needs improvement: | set |
|---|
comment:12 by , 2 months ago
| Patch needs improvement: | unset |
|---|
comment:13 by , 2 months ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
Provided some feedback which has been incorporated. Touches on the main mechanisms of XSS and where gotchas might exist when working with Django.
comment:14 by , 2 months ago
| Patch needs improvement: | set |
|---|---|
| Triage Stage: | Ready for checkin → Accepted |
comment:15 by , 2 months ago
| Patch needs improvement: | unset |
|---|
All the changes are implemented as per the reviews.
Thank you Daniele Procida and James Bligh for the initial patches.
Thanks James Beard and Jacob Walls for the reviews
comment:16 by , 2 months ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
Hi,
I reviewed this ticket and the attached
security_xss.patchas a new contributor looking for documentation-related tasks.From a reader's perspective, the proposed changes seem useful because they make the XSS section easier to follow. In particular, separating the explanation into shorter paragraphs and listing common XSS scenarios makes the documentation clearer than the current single-paragraph explanation.
I also found the added clarification about Django templates' autoescaping and its limitations helpful, especially the example showing why leaving an HTML attribute unquoted can still be risky.
I don't have enough experience with Django's security documentation to mark this as ready for check-in, but the wording in the patch seems understandable and useful from a new contributor/user perspective.
I hope this review helps with the triage process.