Deserializing Empty HasMany Relationship Array Params in JSONAPI Adapter

[namack]

When using the JSONAPI Adapter to deserialize incoming parameters, has_many relationships with empty data arrays are deserialized incorrectly.

For example, in the articles_controller.rb, the article_params are deserialized using ActiveModelSerializers::Deserialization.jsonapi_parse(params, only: [:title, :src, :images])

When an incoming payload includes empty has_many relationships, the deserializer changes the key to it's singular (belongs_to/has_one) form. For example, the following payload will cause an error:

{
  "data": {
    "type": "articles",
    "attributes": {
      "title": "Ember Hamster",
      "src": "http://example.com/images/productivity.png"
    },
    "relationships": {
      "images": {
        "data": []
      }
    }
  }
}

The resulting article_params will parse image_id: nil, instead of images: []. Since an article has_many :images, this will cause an error and the POST will fail.

1248

cc: @matt-morris

[beauby]

As you can see here, this should get picked up. I'll investigate your payload tomorrow. Are you certain that's what your client is issuing?

[janvarljen]

@namack

I had the same issue. The thing is Rails converts empty arrays to nils (security issues). Then the AMS deserializer gets "data": nil insted of "data": [] and that's why it's deserialized to image_id instead of image_ids.

This issue in Rails is fixed but only for Rails 5 version:

https://github.com/rails/rails/pull/16924

atm I'm manually fixing my params until I migrate to Rails 5 like this:

params[:images_ids] = [] if params.key?(:image_id)

[namack]

@janvarljen

Thanks for the explanation - I am seeing that exact behavior. The empty array is converted to nil before the deserializer even gets the chance to deserialize, which is why line 185 never gets the chance to change the key correctly.

@beauby - Should I close this since the issue is solved in Rails 5?

[namack]

https://github.com/rails-api/active_model_serializers/pull/1541

[beauby]

Thanks @janvarljen for explaining the cause of the issue. We should definitely warn the rails4 users about this, so @namack I suggest we leave this issue open until the docs have been updated with a warning and an acceptable workaround.

[bf4]

@namack are you registering a JSON API-specific mime-type (format, media type)? If so, you should also set the default parser for it to have different behavior. See for example https://github.com/cerebris/jsonapi-resources/blob/rails5/lib/jsonapi/mime_types.rb and https://github.com/rails/rails/pull/23712#issuecomment-184977238

[namack]

@bf4 this is perfect, thanks! We were registering the mime-type, but were not setting the parser.

@janvarljen the example given by @bf4 will allow empty arrays to be parsed correctly. Just put this in an initializer to override the default parser:

module JSONAPI
  MEDIA_TYPE = 'application/vnd.api+json'
end

Mime::Type.register JSONAPI::MEDIA_TYPE, :jsonapi

parsers = Rails::VERSION::MAJOR >= 5 ?
            ActionDispatch::Http::Parameters :
            ActionDispatch::ParamsParser

parsers::DEFAULT_PARSERS[Mime::Type.lookup(JSONAPI::MEDIA_TYPE)] = lambda do |body|
  data = JSON.parse(body)
  data = {:_json => data} unless data.is_a?(Hash)
  data.with_indifferent_access
end

[bf4]

Prefer name as 'jsonapi' over 'api_json'. It's the future

B mobile phone

On Feb 29, 2016, at 10:29 AM, Nate Amack notifications@github.com wrote:

@bf4 this is perfect, thanks! We were registering the mime-type, but were not setting the parser.

@janvarljen the example given by @bf4 will allow empty arrays to be parsed correctly. Just put this in an initializer to override the default parser:

module JSONAPI MEDIA_TYPE = 'application/vnd.api+json' end

Mime::Type.register JSONAPI::MEDIA_TYPE, :api_json

parsers = Rails::VERSION::MAJOR >= 5 ? ActionDispatch::Http::Parameters : ActionDispatch::ParamsParser

parsers::DEFAULT_PARSERS[Mime::Type.lookup(JSONAPI::MEDIA_TYPE)] = lambda do |body| data = JSON.parse(body) data = {:_json => data} unless data.is_a?(Hash) data.with_indifferent_access end — Reply to this email directly or view it on GitHub.

[bf4]

PR?

B mobile phone

On Feb 29, 2016, at 10:29 AM, Nate Amack notifications@github.com wrote:

@bf4 this is perfect, thanks! We were registering the mime-type, but were not setting the parser.

@janvarljen the example given by @bf4 will allow empty arrays to be parsed correctly. Just put this in an initializer to override the default parser:

module JSONAPI MEDIA_TYPE = 'application/vnd.api+json' end

Mime::Type.register JSONAPI::MEDIA_TYPE, :api_json

parsers = Rails::VERSION::MAJOR >= 5 ? ActionDispatch::Http::Parameters : ActionDispatch::ParamsParser

parsers::DEFAULT_PARSERS[Mime::Type.lookup(JSONAPI::MEDIA_TYPE)] = lambda do |body| data = JSON.parse(body) data = {:_json => data} unless data.is_a?(Hash) data.with_indifferent_access end — Reply to this email directly or view it on GitHub.

[namack]

Prefer name as 'jsonapi' over 'api_json'. It's the future

@bf4 Good call - code example above has been changed.

PR?

I'd love to make a PR to fix this, is it something that can be added to the AMS repo to generate the initializer or just for docs?

[bf4]

@namack I think what I'd like is a file that can be optionally required. I'd worry about messing with people's existing mime type configs if it were in the railtie. I spiked something here the seems to work https://github.com/rails-api/active_model_serializers/pull/1547

[namack]

@bf4 Looks great! I like that approach.

[remear]

Resolved with the addition of register_jsonapi_renderer