Do not concatenate an array if passed to escapeLiteral #3489
Conversation
Deploying node-postgres with
|
| Latest commit: |
bf59f44
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d55076a2.node-postgres.pages.dev |
| Branch Preview URL: | https://bmc-fix-escape-literal.node-postgres.pages.dev |
| } | ||
|
|
||
| if (typeof str !== 'string') { | ||
| return "''" |
There was a problem hiding this comment.
How about throwing a TypeError? Seems about the same level of breaking change, but potentially less confusing.
There was a problem hiding this comment.
that's what i was going to do at first, but after writing a bunch of tests for previously indeterminate behavior, almost everything returned "''" when passed to this function - numbers, boolean, date, object. The only problem is with things with a .length property which aren't strings (namely: array). It actually kinda feels more "escapey" to me to just turn your "not a string" into an empty string in the query if you're concatenating in there. But I was definitely only the fence.
There was a problem hiding this comment.
Now that you mention those examples… how about casting everything to String? pg 9?
There was a problem hiding this comment.
yeah that isn't a good idea - def breakage, though subtle. tbh whenever I, very rarely, use these functions to concatenate some form of user-input into a query directly I do a ton of external sanitization first. Check types, if its a number make sure its in an expected range, etc. It's scary & should be a last resort in most cases.
nullorundefined.length