serviejs / popsicle

Simple HTTP requests for node and the browser
MIT License
246 stars 19 forks source link

x-www-form-urlencoded data is encoded using URL encoding #85

Open XuluWarrior opened 7 years ago

XuluWarrior commented 7 years ago

This almost certainly isn't a real problem but as I was debugging my queries I noticed that space characters in form data are encoded as %20 rather than +

http://stackoverflow.com/questions/1211229/in-a-url-should-spaces-be-encoded-using-20-or

Before I discovered popsicle I was using my own hacked up promised web session code and I ended up using the form-urlencoded package for my encoding. But my decision was based on using the first package I found rather than any careful analysis.

blakeembrey commented 7 years ago

Wow, it sounds like you are working with some legacy systems! Do you mind if I ask what isn't accepting this? Encoding it yourself instead of the internal is very straightforward, thankfully, but I'll do an analysis of what it would take to do + instead.

blakeembrey commented 7 years ago

Some initial research: It seems other libraries like request and superagent don't do this - https://github.com/request/request/blob/9f702bf4b5d075035c28eea55dda44d3a0235926/lib/querystring.js and https://github.com/ljharb/qs/blob/8aa9c26f90335b5483a4f456dea9acbada8a881c/lib/utils.js#L86 are the methods that get invoked, neither are handling the space correctly. I'll look at formatting according to https://url.spec.whatwg.org/#urlencoded-serializing when I get the chance, if there's minimal overhead to doing so.

XuluWarrior commented 7 years ago

Sorry I should have made it more clear. This hasn't actually caused me an issue. It was just that when debugging I noticed the difference between my node code and my browser. I don't know enough about the http specs to know if this is likely to ever cause a real problem but I thought I would point it out and leave it to your judgement.

blakeembrey commented 7 years ago

Sounds good. I don't think it'll hurt to try and follow the spec though, so I'll check it out.