Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HashConversions does not handle objects with varying keys in arrays #719

Open
twelve17 opened this issue Jan 22, 2021 · 2 comments
Open

Comments

@twelve17
Copy link

twelve17 commented Jan 22, 2021

Using a variant of the example in HashConversions, given object obj:

{
  "name": "Bob",
  "phones": [
    {
      "type": "home" // He finally cut the cord!
    },
    {
      "type": "work",
      "phone": "333-333-3333"
    }
  ]
}

The following params are produced by to_params:

[51] pry> params = HTTParty::HashConversions.to_params(obj)
=> "name=Bob&phones%5B%5D%5Btype%5D=home&phones%5B%5D%5Btype%5D=work&phones%5B%5D%5Bphone%5D=333-333-3333"
[52] pry>)> puts JSON.pretty_generate(CGI.parse(params))
{
  "name": [
    "Bob"
  ],
  "phones[][type]": [
    "home",
    "work"
  ],
  "phones[][phone]": [
    "333-333-3333"
  ]
}

Bob's work phone number is now his home number.

I realize working around this would require inspecting all array entries and compiling a superset of all keys.

Perhaps the bigger issue I'm seeing is that the default behavior of HTTParty.post when body is an object is to delegate to non-standard serialization of the object, versus assuming the caller wants to send JSON (and thus switch to Content-type: application/json).

Since serialization of array query parameters do not seem to have an agreed-to standard, it would seem more sensical to have this be an opt-in code path, and the default be the JSON code path. Especially when you consider the ambiguous format: :json parameter:

HTTParty.post(
  "/some/url",
  format: :json,
  body: {
    "name: "Bob",
    "phones": [
      {
        "type": "home"
      },
      {
        "type": "work",
        "phone": "333-333-3333"
      }
    ]
  }
)

The combination of format: :json and passing of an object to the body really kinda makes this request seem like it's going to send a JSON payload. But to the unsuspecting eye, it can inadvertently lead to a lot of Bobs getting their work calls at home.

Of course, if you RTFM, you'll understand that format: :json only applies to the request, but "Hey, RTFM" sounds like the kind of thing someone might respond to with "You must be fun at parties". And well, this is a HTTParty, right? 😉

@twelve17 twelve17 changed the title HashConversions does not handle varying keys in arrays HashConversions does not handle objects with varying keys in arrays Jan 22, 2021
@JangoSteve
Copy link

FYI, I recently ran into something similar, but I think one of the issues with the post as described here is that you're serializing with HTTParty::HashConversions.to_params but then parsing with CGI.parse. Those two have very different ways of thinking about serializing and de-serializing nested hashes to and from query strings, respectively, so they're not really inverses of each other.

The HTTParty::HashConversions.to_params serializes nested hashes and arrays the Rails way, and Rails uses Rack::Utils.parse_nested_query to then marshal those hashes and arrays back from the query strings.

Meanwhile, CGI.parse would be the way to marshal back hashes and arrays from a query string that was serialized instead with URI.encode_www_form.

I.e. with:

obj = {
  "name" => "Bob",
  "phones" => [
    {
      "type" => "home"
    },
    {
      "type" => "work",
      "phone" => "333-333-3333"
    }
  ]
}

This works:

obj == Rack::Utils.parse_nested_query(HTTParty::HashConversions.to_params(obj))
#=> true

And so does this:

obj == CGI.parse(URI.encode_www_form(obj))
#=> false, but close (name is now an array and strings are escaped, but home/work aren't mixed up)

But if you mix the two, you're going to have a bad time. In other words, if you use parsed_response instead of parsing it yourself with CGI.parse, or you parse it yourself with the Rack::Utils.parse_nested_query, it keeps everything aligned. If you need HTTParty to serialize things so that they can be properly parsed with CGI.parse, you can always pass the query in yourself as a string (in which case HTTParty skips the hash conversion), and serialize the string yourself with URI.encode_www_form.

All that said, I was just thinking of submitting a pull request that allows passing to HTTParty your own conversion method.

@jnunemaker
Copy link
Owner

@JangoSteve thanks for researching that. I'd love a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants