search

my-covid-story / www

We are a group of concerned citizens who could no longer stand by as Ontario is led into a humanitarian crisis. We believe the power of storytelling is an effective means to drive government action.
https://www.mycovidstory.ca
Creative Commons Zero v1.0 Universal
9 stars 5 forks source link

Do not sanitize HTML in new stories to avoid entity references #179

Closed davesteinberg closed 3 years ago

davesteinberg commented 3 years ago

This addresses the issue of entity references (e.g. &amp;) in our stories by simply removing HTML sanitization from the handling of the POST /api/stories API. Since we've decided that we're treating stories as plain text and we're avoiding potential XSS at display time (by placing the content inside <p> elements in a React render, not using dangerouslySetInnerHTML), there's no need to do any sanitization here.

You could make a case for still stripping out HTML, but I think that's a much lower priority than fixing the current mishandling of & and <. Someone would actually have to type or copy HTML elements into a plain text box, which seems unlikely. If that does happen, we could fix it manually.

Since we're now going to emailing story text to politicians automatically, we really should stop mangling them as quickly as possible.

vercel[bot] commented 3 years ago

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/my-covid-story/www/GR8pXHV1rJQMf11cXEWy6bToL7Tk
✅ Preview: https://www-git-dsdesanitize-my-covid-story.vercel.app

davesteinberg commented 3 years ago

I've added another commit to this PR to clean up story titles on submission. I've noticed that people will often leave whitespace at the end of their titles, which can look bad when we add quotes around it (especially if line wraps between the last word and the closing quote). Also, occasionally some people put their own quotes around the whole title, which doesn't look great, either. So, I'm removing those and converting any double quotes within the title to single quotes (since they're going to be inside out double quotes).

This seemed like the kind of thing that's ideal to TDD, so I did (thanks @silent1mezzo for setting that up!). Since it's just a simple function, I wanted to put it in a simple, no-dependency module for easy unit testing. So, I created model/story.ts for it. I imagine a bunch of other simple, model related stuff could move here, and I'm planning to do some of that soon. I'd like to try to tighten things up a bit more in the handling of the POST (since it is an unprotected API), and more TDD would be helpful for that.

This should be good to go now.