Adding javascript escaping support #23

Merged
merged 4 commits into from Mar 30, 2013

Projects

None yet

2 participants

@geek
Member
geek commented Mar 25, 2013

No description provided.

@geek geek referenced this pull request in hapijs/boom Mar 30, 2013
Merged

Adding extra XSS protection #3

@hueniverse hueniverse merged commit e09fc91 into hapijs:master Mar 30, 2013

1 check passed

default The Travis build passed
Details
@hueniverse hueniverse commented on the diff Mar 30, 2013
lib/escaper.js
+
+
+internals.safeCharCodes = (function () {
+
+ var safe = {};
+
+ for (var i = 32; i < 123; ++i) {
+
+ if ((i >= 97 && i <= 122) || // a-z
+ (i >= 65 && i <= 90) || // A-Z
+ (i >= 48 && i <= 57) || // 0-9
+ i === 32 || // space
+ i === 46 || // .
+ i === 44 || // ,
+ i === 45 || // -
+ i === 95) { // _
@hueniverse
hueniverse Mar 30, 2013 Member

Any reason why : is not allowed? I added it because it was breaking tests in other modules that used it.

@geek
geek Apr 1, 2013 Member

Sounds good to me. If we used this for escaping html attributes then we may want to escape :, to prevent href="javascript: "

@hueniverse
Member

Should we test specifically for that? "javascript:"

@geek
Member
geek commented Apr 1, 2013

I think we are fine as is, especially since we encode all <>/() characters. This would really only be needed if we want to provide an escapeHtmlAttribute function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment