post request params broken

[kostya]

seems this is after crystal update.

require "kemal"

post "/bla" do |env|
  p env.params.body["x"]?
end

Kemal.run

curl -s http://127.0.0.1:3000/bla -X POST --data 'x%3D1'
curl -s http://127.0.0.1:3000/bla -X POST --data 'x=1'

nil
nil

works in previous crystal, kemal

[sdogruyol]

Now this is strange, all the specs are passing 😢

[airinfection]

My mother always says "don't trust the specs".

[Fusion]

I can confirm this is broken.
The only way I can retrieve these params is:

puts env.request.body.as(IO).gets_to_end

I suspect this has to do with Crystal now supporting streaming...

[Fusion]

(note: for now, one can use this but really I hope we get a cleaner fix ;) )

env.request.body.as(IO).gets_to_end.split("&").each { |r| k,v=r.split("="); env.params.body[k] ||= v }

[sdogruyol]

This is now fixed 👍 Thanks everyone for reporting ❤️

[airinfection]

❤️S❤️E❤️R❤️D❤️A❤️R❤️ ❤️D❤️O❤️G❤️R❤️U❤️Y❤️O❤️L❤️

[kostya]

what about spec?

[sdogruyol]

@kostya the specs are correct, this was due to a behaviour change in HTTP::Request.body in Crystal std

[Fusion]

I think Kostya was asking whether you are going to add a test case for params.body to the specs.

[sdogruyol]

It's already there @Fusion https://github.com/sdogruyol/kemal/blob/master/spec/param_parser_spec.cr#L53

[Fusion]

Ooh, nice.
Question: are we definitely comfortable using gets_to_end? I mean, depending on the change in Crystal, you may, down the road, get something other than a FixedLengthContent object.

[kostya]

this is unit test, i think better to add integration test also, run http server, and run requests. in crystal its quite easy. i do it in many projects. example in msgpack: https://github.com/benoist/msgpack-crystal/blob/master/spec/socket_spec.cr#L15

[sdogruyol]

@Fusion i don't think that HTTP::Server will get another rewrite like this.

@kostya you are right. I've thought of that but not sure.

[Fusion]

Thanks, Serdar. That was definitely a nitpick 👍

[rdp]

FWIW this should also work:

curl -X POST -H "Content-Type: multipart/form-data; boundary=----------------------------4ebf00fbcf09" -d $'------------------------------4ebf00fbcf09\r\nContent-Disposition: form-data; name="x"\r\n\r\ntest\r\n------------------------------4ebf00fbcf09--\r\n' http://localhost:3000/bla

However the first example and this one both fail currently...it gives empty env.params.body second example works though :)

[rdp]

OK after some research it appears that --data 'x%3D1' should be nil (that's a very long named key, named x=1, with no value) https://stackoverflow.com/questions/52641356/can-you-escape-the-equals-sign-in-post-data for the form-data one filed a new issue, seems unrelated.