search

tj / node-querystring

querystring parser for node and the browser - supporting nesting (used by Express, Connect, etc)
MIT License
455 stars 66 forks source link

Exceptions on non-utf8 data #5

Open bkw opened 13 years ago

bkw commented 13 years ago

Right now qs (and thus connect's bodyParser) cannot handle formdata that would be illegal utf8 (i.e. %DF) and return an error. A prominent Example would be paypal's ipn messages, which use a windows-1252 charset.

The real culprit is the use of decodeURIComponent(), which doesn't seem to be flexible enough.

The "competition" library isaacs/node-querystring implemented a "safer" decoder and a way to specify custom functions for decoding. I'll try to massage bodyParser into using this one by overriding the parser for application/x-www-form-urlencoded, but I wonder if there shouldn't be a better solution for cases like this one.

Any thoughts?

bkw commented 13 years ago

For the sake of completeness: Paypal does offer a configuration option to switch to utf8. Nontheless, other services might not.

tj commented 12 years ago

did isaac's work ok? I doubt "fast" is much of an argument seeing as it was slower last time I benched, and it's a nearly irrelevant thing to benchmark anyway haha but if his decoder works then cool

tcha-tcho commented 12 years ago

Hi Bernhard, did it work?? I already lost 2 days trying to figure out this... paypal ipn crash the entire process. Its insane. maybe a try catch on line 47?

bkw commented 12 years ago

@tcha-tcho: Configuring our paypal account to always use utf8 worked for us, so I stopped investigating. Now that You bring it up, I start feeling a bit uncomfortable again...

tcha-tcho commented 12 years ago

Unfortunately I cannot ask for my clients to follow 11 steps to configure the utf8. This will transform my "Get your store working in 1 min" to "Get your store working on 20 min" hehehe... Well I think the only solution to this is to create a different server written in another language just to receive IPNs.... But I think they have to advise all users "Node.JS doesn't work with PayPal IPN"

tj commented 12 years ago

@tcha-tcho why in the world would you need a different language just for that? it's just a problem we need to fix

tcha-tcho commented 12 years ago

"All we have to do is to say 'try catch'" please

hahaha sorry Im desperate, Heroku doesn't accept ssh and I cannot change this at the server. There are a way to override qs.parse on the code?

tcha-tcho commented 12 years ago

curl -H "Content-Type: application/x-www-form-urlencoded" -d "{%:%}" -X POST "http://localhost:3000" And boooooommmm!