minchenkov / node-html-encoder

NodeJS wrapper for JavaScript HTML Encoder library http://www.strictly-software.com/htmlencode
24 stars 4 forks source link

Prevent htmlEncode() and htmlDecode() from converting blank strings into empty strings #2

Closed rodw closed 11 years ago

rodw commented 11 years ago

Generally, when no characters need to be encoded, htmlEncode() returns input string. For example,

encoder.htmlEncode("Foo")

yields

"Foo"

This holds true even when the input string has leading or trailing whitespace. For example,

encoder.htmlEncode("   Foo ")

yields

" Foo "

But when the input string is comprised entirely of whitespace, htmlEncode() always returns a blank string, rather than the original input string.

For example,

encoder.htmlEncode(" ")

yields

""

or

encoder.htmlEncode("     ")

also yields

""

I think we'd like to see the encoder return the orginal string in these cases.

htmlDecode() has the same problem.

encoder.htmlDecode("Foo")

yields

"Foo"

And

encoder.htmlDecode("   Foo ")

yields

"   Foo "

But

encoder.htmlDecode(" ")

yields

""

and

encoder.htmlDecode("     ")

yields

""

This is easy to fix. Rather than something like

if(this.isBlank(str)) { return ''; }

we should use

if(this.isBlank(str)) { return str; }

When the input string was truly empty, the behavior doesn't change, but when given a non-empty but blank string (one comprised entirely of whitespace), now we get back the orginal string, unchanged.

It doesn't look like any of the code depends on htmlEncode and htmlDecode reducing blank strings to empty ones, so I believe this is safe change to make.

There doesn't seem to be a test suite for node-html-encoder (yet?) and I didn't want to take the liberty of adding one for this small patch, but I posted a small script that demonstrates the issue at https://gist.github.com/rodw/4736648#file-example-js

rodw commented 11 years ago

@minchenkov: Awesome. Thanks for the quick response!