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

Ensure book title/summary are not escaped twice #38675

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hamishwillee
Copy link
Collaborator

This is the demo side fix to #5320

This displays the data in book detail title & summary unescaped. That's OK because the form only displays data that has already been sanitized. If we don't do this it gets sanitized on display, so we see the sanitization code we added on saving.

@hamishwillee hamishwillee requested a review from a team as a code owner March 17, 2025 03:04
@hamishwillee hamishwillee requested review from pepelsbey and removed request for a team March 17, 2025 03:04
@github-actions github-actions bot added Content:Learn Learning area docs size/s [PR only] 6-50 LoC changed labels Mar 17, 2025
Copy link
Contributor

@Josh-Cena
Copy link
Member

This is the demo side fix to #5320

Is the other side of the fix coming up? We should have at least one PR that automatically closes the issue which usually is the content PR.

Also, does escaping the backing string do more things than preventing HTML injection? Does it also prevent SQL injection, considering we aren't even writing raw SQL? If its only goal is to prevent HTML injection, then that should be done in the templating layer, because as you have realized the current way already causes bugs with re-updating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:Learn Learning area docs size/s [PR only] 6-50 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants