Skip to content
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.

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

ALLOW_CHILDREN and PURGE_WITHOUT_ATTRS can't be set at the same time #71

Open
darthnorman opened this issue Nov 18, 2021 · 3 comments
Open
Labels
bug Something isn't working enhancement New feature or request

Comments

@darthnorman
Copy link

I want to use a certain tag that should be allowed to have children. In this case I'd use ALLOW_CHILDREN as a flag on my new tag.

But what if I want to allow it only if it has a specific attribute on it, like a must-have attribute?

I can't give it both flags, only one. So I have to decide whether it shows its content or shows itself if it has an attribute, but then it doesn't render the content inside of it.

Also, what is it with PURGE_WITHOUT_CHILDREN? This can't be used since you'd have to allow children in the first place. If I use this flag, I get "Tag %s does not allow children, but shall be purged without them", what's the point of setting this flag then?

@ohader ohader added the question Further information is requested label Nov 18, 2021
@ohader
Copy link
Member

ohader commented Nov 18, 2021

Can you please give examples describing what you try to do and which input data (HTML) you're using? Thanks!

@darthnorman
Copy link
Author

I want to allow the script-tag for structured data in json format. So the attribute I'd like it to have is type="application/ld+json" but I also have to allow children (the actual json code). When someone tries to abuse it by adding any other type or no type at all, it shouldn't be rendered. Thats why I'd need both flags, ALLOW_CHILDREN and PURGE_WITHOUT_ATTRS, wouldn't I?

ohader added a commit to ohader/html-sanitizer that referenced this issue Jan 13, 2022
@ohader
Copy link
Member

ohader commented Jan 13, 2022

There was an unexpected behavior when parsing <script></script> - which was considered having an empty text-node.
In contrast, parsing <div></div> did not register any child nodes. Thus, PHP's behavior for <script> tags seems to be special.

However, PURGE_WITHOUT_ATTRS is not a real protection for the given use-case. Lets assume id attrs were allowed in general on any tags. That would lead to the situation that the following tag still would be allowed.

<script id="identifier">alert(1)</script> // not purged by PURGE_WITHOUT_ATTRS

→ Thus, the long-term solution for e.g. allowing JSON-LD in <script> tags, would be to make particular attr values mandatory

ohader added a commit to ohader/html-sanitizer that referenced this issue Jan 13, 2022
@ohader ohader added bug Something isn't working enhancement New feature or request and removed question Further information is requested labels Jan 13, 2022
ohader added a commit to ohader/html-sanitizer that referenced this issue Jan 14, 2022
ohader added a commit that referenced this issue Jan 14, 2022
* [BUGFIX] Purge node only having empty text nodes

Related: `Behavior\Tag::PURGE_WITHOUT_CHILDREN`

* [TASK] Add test cases for custom behavior scenarios

Related: #71
ohader added a commit to ohader/html-sanitizer that referenced this issue Jan 14, 2022
ohader added a commit to ohader/html-sanitizer that referenced this issue Oct 6, 2022
ohader added a commit that referenced this issue Oct 6, 2022
* [FEATURE] Introduce mandatory tag attributes (Related: #71)
* [TASK] Add iframe example scenario (Related: #70)
* [TASK] Add tests for Attr flag assignment
* [TASK] Add example for Attr::MANDATORY to README.md
* [TASK] Resolve todos in ScenarioTest
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants