rubyonjets / jets

Ruby on Jets
http://rubyonjets.com
MIT License
2.6k stars 181 forks source link

Array as the POST body causes issues in the deep_merge #434

Closed panbanda closed 6 months ago

panbanda commented 4 years ago

Checklist

My Environment

Software Version
Operating System Mac
Jets 2.3.4
Ruby 2.5.5

Expected Behaviour

Allow POST with an Array for a body, but allow developer to deal with it manually

Current Behavior

I have an angular apollo batch http request that batches requests from the frontend for multiplex processing on the backend. This library used to send a [:_json] parameter but now it looks like its just sending the post body as an array. This causes issues in the initialization functions for deep merging the parameters of body into the params as it treats it as a hash.

Step-by-step reproduction instructions

  1. Send a post request to the controller with an array payload.
  2. Look at the controller failing lib/jets/controller/params.rb:20

Code Sample

An example body payload that causes issues:

[{"operationName":null,"variables":{"email":"xxxxx","password":"xxxxx"},"query":"mutation {\n  authLogin(email: $email, password: $password) {\n    token\n    __typename\n  }\n}\n"}]

Solution Suggestion

https://github.com/tongueroo/jets/blob/master/lib/jets/controller/params.rb#L20

I think its as simple as adding this to the line in order to avoid deep merging an array and expect the developer to deal with body_params manually if it's an array. The body can probably also be refactored into a hash [if array -> set :json with array]

params = params.deep_merge(body_params) if body_parameters && !body_parameters.is_a?(Array)

# or
if body_parameters 
  if body_parameters.is_a?(Array)
    params.deep_merge({ _json: body_parameters })
  else
    params.deep_merge(body_parameters)
  end
end
tongueroo commented 4 years ago

Thanks for the solution suggestions. Unsure on which one to go with at the moment 🤔 Wondering if there’s any consistency to how other frameworks handle Arrays in POST. Will have to dig into it.

panbanda commented 4 years ago

Yeah totally get it. I wonder if there is a way to just defer the merge params? So its a method and merges when it is called rather than in an earlier middleware? I think other frameworks just allow you to grab the body data and it doesn't get merged into a variable with query and body together. However, in my case, I wouldn't need the params I would just need the body but its failing before the action is called.

tongueroo commented 6 months ago

No longer applies. Jets 6.0 deploys a Rails app now. So whatever that's needed can just be implemented as a Rails engine or plugin, etc.

Related