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

Feature: safeAppend #699

Closed
wants to merge 1 commit into from
Closed

Feature: safeAppend #699

wants to merge 1 commit into from

Conversation

yonjah
Copy link

@yonjah yonjah commented Apr 9, 2019

This should fix #661 and #683 by only allowing html to be set inside jQuery object
This introduce a minor breaking change -

// old API
bootbox.alert('<b>Hello World!</b>');
// new API
bootbox.alert($('<b>Hello World!</b>'));

So naively using bootbox without sanitizing input should be relatively safe without the risk of XSS

bootbox.alert(`Hello ${user.name}`); 
bootbox.alert(result.message);

I added a few tests to check this changes but since there were no existing tests to verify
the old functionality of html strings so I only added a few basic ones.

I know this is a breaking change and there was a bit of a push back against actually adding it in so I understand if you prefer to keep the old behavior as is.

If you do think this is simple enough to be included in I can probably add another pull request to update the docs

	- Allow using full jQuery nodes in messages, titles and button labels
	- Everything else is treated as text
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.

Document or fix possible XSS vulnerability (via jquery)
1 participant