mathiasbynens / he

A robust HTML entity encoder/decoder written in JavaScript.
https://mths.be/he
MIT License
3.43k stars 255 forks source link

Error with numerical strings #30

Closed navin09 closed 9 years ago

navin09 commented 9 years ago

If the string passed into "encode()" is a number, there's an error occurring on line no. 193.

TypeError: string.replace is not a function
string = string.replace(regexEscape, hexEscape);
mathiasbynens commented 9 years ago

If the string passed into "encode()" is a number, there's an error occurring on line no. 193.

If it’s a number then how is it a string?

As stated in the docs, he.encode(string) expects an actual string, nothing else. If you do need to pass a number to he.encode() (I don’t see why you would ever need to do this, but hey) then use something like he.encode(String(number)).

navin09 commented 9 years ago

I'm dealing with "strings" that were coming from external sources (user-entered), which I need to preserve as they are, except for the encoding.

If you do need to pass a number to he.encode() (I don’t see why you would ever need to do this, but hey) then use something like he.encode(String(number)).

I did do that to make sure that even if it is a number, it will be passed in to he as a string, however, I think internally to he it's again getting converted to a number somewhere.

Anyway, I have now changed my calling code to check whether the string received is not a number (isNaN) before passing it on to he. I just think that he should deal with it gracefully and not crash with an error, if it gets input that it doesn't expect considering that it's main application is sanitizing input.

Thanks for your time and work on this great plugin.

mathiasbynens commented 9 years ago

I did do that to make sure that even if it is a number, it will be passed in to he as a string, however, I think internally to he it's again getting converted to a number somewhere.

That is not the case.

> he.encode(1) // should throw, since it expects a string
TypeError: undefined is not a function
    at Object.encode (~/projects/he/he.js:192:20)
    at repl:1:4
    at REPLServer.defaultEval (repl.js:116:27)
    at bound (domain.js:254:14)
    at REPLServer.runBound [as eval] (domain.js:267:12)
    at REPLServer.<anonymous> (repl.js:269:12)
    at emitOne (events.js:77:13)
    at REPLServer.emit (events.js:166:7)
    at REPLServer.Interface._onLine (readline.js:195:10)
    at REPLServer.Interface._line (readline.js:534:8)
> he.encode('1') // does not throw
'1'
cburatto commented 7 years ago

First, thanks for this library. Great work. And your website is full of great information.

On a generic code, we would have to add "String(something)" to everything, because we don't know what's coming. Seems unnecessary, no?

What about just adding a check on ".encode(x)" like "if not string, return x"?

Thanks

mathiasbynens commented 7 years ago

You can handle that in your own code. I’m not going to make all uses of he slower (by adding the additional check everywhere) just because you don’t feel like type-checking your parameters.