Skip to content

Add add_*/rm_* keyword arguments for set-valued sanitizer options#128

Open
gghez wants to merge 3 commits into
messense:mainfrom
gghez:worktree-issue-78-ammonia-builder-options
Open

Add add_*/rm_* keyword arguments for set-valued sanitizer options#128
gghez wants to merge 3 commits into
messense:mainfrom
gghez:worktree-issue-78-ammonia-builder-options

Conversation

@gghez

@gghez gghez commented May 26, 2026

Copy link
Copy Markdown
Contributor

Closes #78.

Summary

Exposes ammonia's incremental builder methods alongside the existing
replacement options on both nh3.clean() and nh3.Cleaner:

  • add_tags / rm_tags
  • add_clean_content_tags / rm_clean_content_tags
  • add_url_schemes / rm_url_schemes
  • add_generic_attribute_prefixes / rm_generic_attribute_prefixes

Each new option applies on top of the corresponding replacement option
(or the ammonia defaults when it is omitted), so a caller can extend the
default whitelist without first copying it:

# Before:
nh3.clean(html, tags=nh3.ALLOWED_TAGS | {"my-tag"})

# After:
nh3.clean(html, add_tags={"my-tag"})

rm_* applies last, so it wins on conflict with add_* — mirroring
the ammonia builder's ordering.

Notes

  • The filter_style_properties option that the issue also requested
    has already shipped (it is in v0.3.x), so this PR covers only the
    remaining add_tag-style request, generalized to the four
    set-valued options that ammonia exposes add_*/rm_* for.
  • The existing clean_content_tags overlap guard (PR Validate clean_content_tags conflict with tags #125) now
    computes the effective tag sets after add_*/rm_* are applied,
    so e.g. add_tags={"p"} plus add_clean_content_tags={"p"} raises
    ValueError rather than panicking ammonia.
  • Mapping-shaped options (attributes, allowed_classes,
    tag_attribute_values, set_tag_attribute_values) keep the
    replacement-only behaviour — designing an ergonomic additive API
    for nested dicts is out of scope here.

Test plan

  • maturin develop --release builds cleanly
  • python -m pytest -v — 23 passed (8 new tests + existing 14 + 1 RST doctest)
  • cargo fmt --check — silent
  • cargo check — exit 0
  • Manually verified the new RST doctest in docs/index.rst runs as part of the suite

gghez and others added 3 commits May 26, 2026 20:26
Documents the eight add_*/rm_* keyword arguments planned for clean()
and Cleaner, plus the rationale for limiting scope to set-valued
options (tags, clean_content_tags, url_schemes,
generic_attribute_prefixes).
Closes messense#78.

Expose ammonia's incremental builder methods alongside the existing
replacement options:

  - add_tags / rm_tags
  - add_clean_content_tags / rm_clean_content_tags
  - add_url_schemes / rm_url_schemes
  - add_generic_attribute_prefixes / rm_generic_attribute_prefixes

These apply on top of the corresponding replacement option (or the
ammonia defaults when it is omitted), so a caller can extend the
default whitelist without copying it:

    nh3.clean("<my-tag>x</my-tag>", add_tags={"my-tag"})

The existing clean_content_tags overlap guard now uses the effective
tag sets (after add_*/rm_* are applied) to keep ammonia from panicking
when overlap is introduced via the new add_* options.

filter_style_properties from the original issue is already shipped.
…tions-design.md

no need to commit the spec file

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR extends nh3’s Python API to support ammonia’s incremental “builder” modifiers (add_* / rm_*) for set-valued sanitizer options, allowing callers to extend or subtract from defaults without copying full whitelists.

Changes:

  • Added add_* / rm_* keyword arguments for tags, clean_content_tags, url_schemes, and generic_attribute_prefixes to both nh3.clean() and nh3.Cleaner.
  • Updated overlap validation between allowed tags and clean-content tags to consider the effective sets after modifiers are applied.
  • Added tests, type stubs, and documentation covering the new options and examples.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/lib.rs Adds new config fields, wires the new keyword args through to ammonia builder methods, and updates overlap validation logic.
tests/test_nh3.py Adds new pytest coverage for add_* / rm_* behaviors and modifier interactions.
nh3.pyi Updates public type signatures to include the new keyword arguments.
docs/index.rst Documents how to extend/remove from defaults using add_* / rm_* options with doctest examples.
Comments suppressed due to low confidence (1)

src/lib.rs:456

  • The ValueError message references only tags and clean_content_tags, but conflicts can now be introduced via add_tags/rm_tags and add_clean_content_tags/rm_clean_content_tags as well. Consider updating the message to describe the effective allowed vs clean-content sets and suggest resolving it by either removing from the clean-content set or removing from the allowed set (including via rm_tags / rm_clean_content_tags), so callers know how to fix conflicts introduced by the new modifiers.
            if let Some(tag) = effective_clean.intersection(&effective_allowed).next() {
                return Err(PyValueError::new_err(format!(
                    "tag \"{}\" cannot appear in both `tags` and `clean_content_tags`; \
                     either remove it from `clean_content_tags` or pass an explicit \
                     `tags` set that excludes it",
                    tag
                )));

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/lib.rs
// applying the add_*/rm_* modifiers, then check they are disjoint.
// Otherwise ammonia would panic at clean-time. When the caller omits
// the replacement option, ammonia falls back to its own defaults.
if clean_content_tags.is_some() || add_clean_content_tags.is_some() {
Comment thread tests/test_nh3.py
Comment on lines +70 to +77
def test_add_tags_extends_defaults():
# "my-tag" is not in the defaults, so it would normally be stripped.
assert nh3.clean("<my-tag>x</my-tag>") == "x"
# add_tags extends the defaults without replacing them, so default tags
# like <b> are still preserved.
assert (
nh3.clean("<b><my-tag>x</my-tag></b>", add_tags={"my-tag"})
== "<b><my-tag>x</my-tag></b>"
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.

More ammonia builder options

3 participants