Skip to content

warn about extra sanitization on RSS page#6206

Merged
sarah11918 merged 3 commits intowithastro:mainfrom
mayank99:patch-2
Jan 9, 2024
Merged

warn about extra sanitization on RSS page#6206
sarah11918 merged 3 commits intowithastro:mainfrom
mayank99:patch-2

Conversation

@mayank99
Copy link
Copy Markdown
Contributor

@mayank99 mayank99 commented Jan 9, 2024

Description (required)

Discussed beforehand with Sarah, this PR expands the tip about sanitize-html to warn about potentially too much sanitization.

When I first added RSS to my blog, I simply followed the tutorial and it took me quite a bit of time to understand that many parts of the post content were broken because sanitize-html was stripping out elements and attributes beyond just <script> and <style>.

Related issues & labels (optional)

  • Closes #
  • Suggested label: add new content

@vercel
Copy link
Copy Markdown

vercel bot commented Jan 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview Jan 9, 2024 0:59am
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
docs-i18n ⬜️ Ignored (Inspect) Visit Preview Jan 9, 2024 0:59am


:::tip
A package like [`sanitize-html`](https://www.npmjs.com/package/sanitize-html) will make sure that your content is properly sanitized, escaped, and encoded.
A package like [`sanitize-html`](https://www.npmjs.com/package/sanitize-html) will make sure that your content is properly sanitized, escaped, and encoded. In the process, such a package might also remove some harmless elements and attributes, so make sure to verify the output and configure the package according to your needs.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Could be useful to link to something like https://github.com/apostrophecms/sanitize-html#common-use-cases, but I didn't want to make this tip too verbose

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@mayank99 Although not a direct link to the anchor, the npm readme does include the "common use cases" so someone can certainly get there just by reading further down the page.

Let's merge the tip as is for now, which I think sounds great!

Copy link
Copy Markdown
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Thank you so much for picking this up, @mayank99 ! I think it's a great addition!

@sarah11918 sarah11918 merged commit a14c803 into withastro:main Jan 9, 2024
dreyfus92 added a commit that referenced this pull request Jan 13, 2024
* i18n(fr) Update reading-time.mdx

Just for translator tracker because the PR #5766 should have been [ignore]

* Update rss.mdx

With PR #6206

* Update rss.mdx

With PR #6154

---------

Co-authored-by: Paul Valladares <85648028+dreyfus92@users.noreply.github.com>
ematipico pushed a commit that referenced this pull request Jan 26, 2024
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
ematipico pushed a commit that referenced this pull request Jan 26, 2024
* i18n(fr) Update reading-time.mdx

Just for translator tracker because the PR #5766 should have been [ignore]

* Update rss.mdx

With PR #6206

* Update rss.mdx

With PR #6154

---------

Co-authored-by: Paul Valladares <85648028+dreyfus92@users.noreply.github.com>
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