8000 [FrameworkBundle] Integrate the HtmlSanitizer component by tgalopin · Pull Request #44798 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[FrameworkBundle] Integrate the HtmlSanitizer component #44798

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 15, 2022
Merged

[FrameworkBundle] Integrate the HtmlSanitizer component #44798

merged 2 commits into from
Apr 15, 2022

Conversation

tgalopin
Copy link
Contributor
Q A
Branch? 6.1
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR -

This PR adds the integration if the HtmlSanitizer component in the FrameworkBundle. See #44681 for details about the component.

The configuration for this integration is the following:

framework:
    # This configuration is not required: as soon as you install the component, a default
    # "html_sanitizer" service is created, with the safe configuration, to be used directly.
    #
    # This configuration allows to set custom behaviors, in addition or instead of the default.
    html_sanitizer:
        # Default sanitizer (optional)
        # When not provided, the native "html_sanitizer" service is wired as default.
        default: my.sanitizer

        # Custom sanitizers (optional)
        sanitizers:

            # Each sanitizer defines its own service (no prefix/suffix) to ease understanding
            # Each sanitizer also defines a named autowiring alias to ease injection using variable name.

            # Here, this sanitizer is available as a service "my.sanitizer" or using autowiring
            # as "HtmlSanitizerInterface $mySanitizer".
            my.sanitizer:
                allow_safe_elements: true
                allow_elements:
                    iframe: ['src']
                    custom-tag: ['data-attr']
                    custom-tag-2: '*'
                block_elements:
                    - section
                drop_elements:
                    - video
                allow_attributes:
                    src: ['iframe']
                    data-attr: '*'
                drop_attributes:
                    data-attr: '*'
                force_attributes:
                    a:
                        rel: noopener noreferrer
                    h1:
                        class: bp4-heading
                force_https_urls: true
                allowed_link_schemes: ['http', 'https', 'mailto']
                allowed_link_hosts: ['symfony.com']
                allow_relative_links: true
 
8000
               allowed_media_schemes: ['http', 'https', 'data']
                allowed_media_hosts: ['symfony.com']
                allow_relative_medias: true

            # "all.sanitizer" / "HtmlSanitizerInterface $allSanitizer"
            all.sanitizer:
                allow_all_static_elements: true
                allow_elements:
                    custom-tag: ['data-attr']

This PR is still WIP (esp tests) but I wanted to gather feedback regarding the configuration and DX as soon as possible.

@carsonbot carsonbot added this to the 6.1 milestone Dec 26, 2021
@tgalopin tgalopin mentioned this pull request Dec 26, 2021
Merged
2 tasks
@tgalopin tgalopin changed the title Integrate the HtmlSanitizer component to the FrameworkBundle [HtmlSanitizer] FrameworkBundle integration Dec 28, 2021
@derrabus
Copy link
Member

Remember to add symfony/html-sanitizer to the require-dev section of FrameworkBundle's composer.json.

stof
stof requested changes Dec 29, 2021
View reviewed changes
@nicolas-grekas nicolas-grekas mentioned this pull request Dec 29, 2021
Closed
nicolas-grekas
nicolas-grekas reviewed Jan 31, 2022
View reviewed changes < 8000 /span>
Copy link
Member
@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

What's the status here?
I think we're missing a way to configure HtmlSanitizer's $maxInputLength.
The default seems low btw (20000). Shouldn't it be more like 1000000?

@fabpot
Copy link
Member
fabpot commented Mar 26, 2022

@tgalopin Would be great (required?) if you can finish this PR before te 6.1 deadline which is happening soon now :)

@nicolas-grekas nicolas-grekas changed the title [HtmlSanitizer] FrameworkBundle integration [FrameworkBundle] Integrate the HtmlSanitizer component Apr 12, 2022
@wouterj wouterj force-pushed the html-sanitizer-fwb branch 2 times, most recently from af72d96 to a146cba Compare April 14, 2022 19:31
@wouterj wouterj force-pushed the html-sanitizer-fwb branch from a146cba to 4dd3fd6 Compare April 14, 2022 19:42
@wouterj
Copy link
Member
wouterj commented Apr 14, 2022

I've pushed a commit finalizing the tests and XML support. This PR should be ready to merge

@fabpot
Copy link
Member
fabpot commented Apr 15, 2022

Thank you @tgalopin.

@fabpot fabpot mentioned this pull request Apr 15, 2022
Closed
@fabpot fabpot merged commit 8064a5c into symfony:6.1 Apr 15, 2022
@tgalopin tgalopin deleted the html-sanitizer-fwb branch April 15, 2022 06:55
@fabpot fabpot mentioned this pull request Apr 15, 2022
Merged
@nicolas-grekas nicolas-grekas mentioned this pull request Apr 15, 2022
Merged
fabpot added a commit that referenced this pull request Apr 16, 2022
@fabpot
minor #46062 [HtmlSanitizer] Add HtmlSanitizerConfig::withMaxInputLen…
ea96c13 
…gth() (nicolas-grekas)

This PR was merged into the 6.1 branch.

Discussion
----------

[HtmlSanitizer] Add HtmlSanitizerConfig::withMaxInputLength()

| Q             | A
| ------------- | ---
| Branch?       | 6.1
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix ##44798 (review)
| License       | MIT
| Doc PR        | -

Commits
-------

070f2cf [HtmlSanitizer] Add HtmlSanitizerConfig::withMaxInputLength()
symfony-splitter pushed a commit to symfony/framework-bundle that referenced this pull request Apr 16, 2022
@fabpot
minor #46062 [HtmlSanitizer] Add HtmlSanitizerConfig::withMaxInputLen…
0e2d124 
…gth() (nicolas-grekas)

This PR was merged into the 6.1 branch.

Discussion
----------

[HtmlSanitizer] Add HtmlSanitizerConfig::withMaxInputLength()

| Q             | A
| ------------- | ---
| Branch?       | 6.1
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #symfony/symfony#44798 (review)
| License       | MIT
| Doc PR        | -

Commits
-------

070f2cfc03 [HtmlSanitizer] Add HtmlSanitizerConfig::withMaxInputLength()
symfony-splitter pushed a commit to symfony/html-sanitizer that referenced this pull request Apr 16, 2022
@fabpot
minor #46062 [HtmlSanitizer] Add HtmlSanitizerConfig::withMaxInputLen…
867cfda 
…gth() (nicolas-grekas)

This PR was merged into the 6.1 branch.

Discussion
----------

[HtmlSanitizer] Add HtmlSanitizerConfig::withMaxInputLength()

| Q             | A
| ------------- | ---
| Branch?       | 6.1
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #symfony/symfony#44798 (review)
| License       | MIT
| Doc PR        | -

Commits
-------

070f2cfc03 [HtmlSanitizer] Add HtmlSanitizerConfig::withMaxInputLength()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
81F8
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

@stof stof stof requested changes

@fabpot fabpot fabpot approved these changes

@chalasr chalasr Awaiting requested review from chalasr

@wouterj wouterj Awaiting requested review from wouterj

Assignees
No one assigned
Labels
Feature FrameworkBundle Ready Status: Reviewed
Projects
None yet
Milestone
6.1
Development

Successfully merging this pull request may close these issues.
7 participants
@tgalopin @derrabus @fabpot @wouterj @nicolas-grekas @stof @carsonbot
0