Replace bleach with nh3 for HTML sanitization by hoheinzollern · Pull Request #163 · torchbox/wagtail-markdown

hoheinzollern · 2026-03-14T20:21:58Z

This PR replaces bleach with nh3 for HTML sanitization, providing significant performance improvements while maintaining the same security guarantees.

Key Changes

API Compatibility: All existing configuration options continue to work exactly as before
Performance: nh3 is significantly faster than bleach (benchmarks show ~20x improvement)
Security: Maintains the same security guarantees with automatic attribute handling

Technical Details

Updated constants to use sets instead of lists (nh3 requirement)
Modified configuration functions to work with nh3's API
Removed rel from allowed attributes (nh3 handles this automatically for security)
Updated all tests to work with the new data structures

Testing

All existing tests pass (note the necessary upgrade, nh3 adds the rel attribute to links with no option to remove it)
Basic functionality verified (markdown rendering, HTML sanitization)

Migration Guide

This change is mostly transparent to users. No code changes are required in projects using wagtail-markdown. Visible changes will be improved performance and the loss of rel attribute.

References

Original issue: Replace bleach with nh3 #132
nh3 documentation: https://nh3.readthedocs.io/
Performance comparison: nh3 is ~20x faster than bleach in benchmarks

hoheinzollern · 2026-03-14T21:08:28Z

Currently the CI is failing with weird messages (one wants me to do the opposite of the other, in two instances where dict() and dict comprehensions are used) so I'm unsure how to fix it.

src/wagtailmarkdown/utils.py

zerolab · 2026-03-16T12:25:33Z

Currently the CI is failing with weird messages (one wants me to do the opposite of the other, in two instances where dict() and dict comprehensions are used) so I'm unsure how to fix it.

Ruff fails with https://docs.astral.sh/ruff/rules/unnecessary-generator-dict/ and https://docs.astral.sh/ruff/rules/unnecessary-comprehension/

hoheinzollern · 2026-03-16T17:46:00Z

Ruff fails with https://docs.astral.sh/ruff/rules/unnecessary-generator-dict/ and https://docs.astral.sh/ruff/rules/unnecessary-comprehension/

Ok, I'm not sure precisely how to address this.

I tried a different approach where all the logic that converts lists to sets (seems necessary with the new library) is localized in the function _sanitise_markdown_html.

hoheinzollern · 2026-03-16T18:54:55Z

OK, looks like the CI passes now, seems like it's addressed. One critical bit is the requirement to use sets. I've made the choice to do the conversion, but it's equally adequate to pass on the change of interface to the users of wagtail-markdown.
My reason for sending this patch is that I am relying on puput which depends on this project. My own pipeline was complaining about the outdated bleach package, so I figured it's an easy enough fix to switch to nh3 instead of just upgrading a deprecated package to a newer version.

zerolab · 2026-04-02T15:00:32Z

src/wagtailmarkdown/utils.py

+    nh3_kwargs["tags"] = set(nh3_kwargs["tags"])
+    nh3_kwargs["attributes"] = {
+        key: set(value) for key, value in nh3_kwargs["attributes"].items()
+    }
+    nh3_kwargs["filter_style_properties"] = set(nh3_kwargs["filter_style_properties"])


IMHO, these belongs in _get_nh3_kwargs

I could but it breaks the tests that rely on _get_nhs3_kwargs, shall I update them?
Another option is to remove these conversions altogether and present the breaking changes directly to the users, so those who customize these default settings will have to update their app.

Another option is to remove these conversions altogether and present the breaking changes directly to the users, so those who customize these default settings will have to update their app.

The proper way would be to add a deprecation warning in the next release, then remove in the following.
At the same time, I do like to be a bit more guarded and prevent users from shooting themselves in the foot. So that is either what this code currentlyu does, or add a system check or similar.

I could but it breaks the tests that rely on _get_nhs3_kwargs, shall I update them?

if it is not too much to ask, that would be nice, thank you

I moved the coercion outside as discussed, but I will leave the deprecation warnings etc. to you, I hope that's OK.

Maybe worth mentioning that styles has been renamed to filter_style_properties in nh3, and this is also a user-facing breaking change.

zerolab · 2026-04-02T15:31:57Z

tests/testapp/tests/test_templatetags.py

-    "<a>anchor tag</a> &lt;script&gt;alert('boom!')&lt;/script&gt;</p>"
+    'text with a <a href="https://example.com" rel="noopener noreferrer">link</a>\n'
+    "and some disallowed tag and attributes: italic, "
+    '<a rel="noopener noreferrer">anchor tag</a> </p>'


While I agree to noopener noreferrer, not everyone will. I think we should leave this to the user, but default to the nh3 defaults.

This is just the default nh3 behaviour, I've just updated the tests to match the output. Should we change that? A quick search revealed this related issue: messense/nh3#8

Good search-fu! Let's do that as it preserves the current behaviour.

We could follow it up to make it configurable, and that gives that choice to end-users

Done and documented, should be ready for review.

This commit replaces the bleach library with nh3 for HTML sanitization, providing significant performance improvements while maintaining the same security guarantees. Key changes: - Updated dependency from bleach to nh3 - Modified constants to use sets instead of lists (nh3 requirement) - Updated configuration functions to work with nh3's API - Adjusted tests to work with the new data structures - Removed 'rel' from allowed attributes (nh3 handles this automatically) - Updated documentation to reflect the change The migration is transparent to users - all existing configuration options continue to work exactly as before, but with improved performance.

- from _sanitise_markdown_html to _coerce_nh3_kwargs_types, called by _get_nh3_kwargs - updated tests to match

- updated tests to match

hoheinzollern force-pushed the main branch from ab92712 to f696b04 Compare March 14, 2026 21:04

zerolab reviewed Mar 16, 2026

View reviewed changes

src/wagtailmarkdown/utils.py Outdated Show resolved Hide resolved

hoheinzollern force-pushed the main branch from d9e0bac to 195000f Compare March 16, 2026 17:43

zerolab reviewed Apr 2, 2026

View reviewed changes

hoheinzollern added 11 commits April 9, 2026 13:34

fix torchbox issues

d1034ae

alternative use of set / list

c98a6b1

ruff check

6256e8d

ruff check

540334f

ruff format

f3ac6f9

moved type coercion

14ca60c

- from _sanitise_markdown_html to _coerce_nh3_kwargs_types, called by _get_nh3_kwargs - updated tests to match

added link_rel=None option to default settings

a0c0942

- updated tests to match

exposing link_rel option to the WAGTAILMARKDOWN settings

b7fc894

readme

8dd9b86

fix tests

a18573e

hoheinzollern force-pushed the main branch from 57a2c83 to a18573e Compare April 9, 2026 11:41

hoheinzollern added 4 commits April 9, 2026 13:43

pleasing ruff

c9233f0

more pleasing ruff

fc3f019

fix test_settings.py

cc60f35

fix test_settings.py

c489399

hoheinzollern mentioned this pull request Apr 13, 2026

feat: upgrade to Wagtail 7.x support APSL/puput#304

Closed

Conversation

hoheinzollern commented Mar 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Key Changes

Technical Details

Testing

Migration Guide

References

Uh oh!

hoheinzollern commented Mar 14, 2026

Uh oh!

Uh oh!

zerolab commented Mar 16, 2026

Uh oh!

hoheinzollern commented Mar 16, 2026

Uh oh!

hoheinzollern commented Mar 16, 2026

Uh oh!

zerolab Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hoheinzollern Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

zerolab Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

hoheinzollern Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hoheinzollern Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

zerolab Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

hoheinzollern Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

zerolab Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

hoheinzollern Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hoheinzollern commented Mar 14, 2026 •

edited

Loading

zerolab Apr 2, 2026 •

edited

Loading

hoheinzollern Apr 9, 2026 •

edited

Loading