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.

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

feat: ability to specify if selfClosing element is a void element #670

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

feat: ability to specify if selfClosing element is a void element #670

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c54eeb1
feat: ability to specify if selfClosing tag is a void element
gastonyte Jul 21, 2024
d705545
chore: ignore .vscode folder
gastonyte Jul 29, 2024
96d3b63
chore: add a lockfile
gastonyte Jul 29, 2024
85f6340
refactor: rename `voidTag` to `voidElement`
gastonyte Jul 29, 2024
344c408
chore: add tests for selfClosing option (including voidElement property)
gastonyte Jul 29, 2024
c884bd4
makes the changes requested
gastonyte Sep 24, 2024
9b0e176
Merge remote-tracking branch 'upstream/main'
gastonyte Sep 24, 2024
e8a6bab
improve readability with a for-of
gastonyte Sep 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ node_modules
.idea
# We do not commit CSS, only LESS
public/css/*.css
.vscode
20 changes: 16 additions & 4 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,15 @@ function sanitizeHtml(html, options, _recursing) {
options = Object.assign({}, sanitizeHtml.defaults, options);
options.parser = Object.assign({}, htmlParserDefaults, options.parser);

const tagAllowed = function (name) {
const { selfClosing } = options;
Copy link
Member

Choose a reason for hiding this comment

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

Localized and then never used in a localized away again because of the reassignment for the array case. So I think it would be clearer if we didn't do this at all and just referred to options.selfClosing throughout this bit.

Copy link
Member

Choose a reason for hiding this comment

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

The README must be updated to cover this new syntax.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the delay on this one!

if (Array.isArray(selfClosing)) {
options.selfClosing = selfClosing.reduce((before, tagName) => ({
Copy link
Member

Choose a reason for hiding this comment

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

It would be more performant to use a for loop here, avoiding the need to make a new object on every pass.

Copy link
Author

Choose a reason for hiding this comment

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

i replaced the reduce with a for-of loop

...before,
[tagName]: true
}), {});
}

const tagAllowed = function(name) {
return options.allowedTags === false || (options.allowedTags || []).indexOf(name) > -1;
};

Expand Down Expand Up @@ -484,15 +492,19 @@ function sanitizeHtml(html, options, _recursing) {
}
});
}
if (options.selfClosing.indexOf(name) !== -1) {
result += ' />';

if (options.selfClosing[name]) {
Copy link
Member

Choose a reason for hiding this comment

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

Use Object.hasOwn(options.selfClosing, name) to make sure it's a real property and not something sneaky like __prototype__. Or use a Map.

Copy link
Author

Choose a reason for hiding this comment

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

done

result += options.selfClosing[name]?.voidElement === true
? '>'
: ' />';
} else {
result += '>';
if (frame.innerText && !hasText && !options.textFilter) {
result += escapeHtml(frame.innerText);
addedText = true;
}
}

if (skip) {
result = tempResult + escapeHtml(result);
tempResult = '';
Expand Down Expand Up @@ -584,7 +596,7 @@ function sanitizeHtml(html, options, _recursing) {

if (
// Already output />
options.selfClosing.indexOf(name) !== -1 ||
options.selfClosing[name] ||
// Escaped tag, closing tag is implied
(isImplied && !tagAllowed(name) && [ 'escape', 'recursiveEscape' ].indexOf(options.disallowedTagsMode) >= 0)
) {
Expand Down
Loading