Active Support JSON Encoding options_for should not return nil #3353

Closed
nuoyan opened this Issue Oct 17, 2011 · 7 comments

Comments

Projects
None yet
4 participants

nuoyan commented Oct 17, 2011

In Rails 3.0.10 activesupport-3.0.10/lib/active_support/json/encoding.rb, the options_for method returns "options", which can be nil, if the value parameter passed in is not an Array or Hash.

This is problematic because later it passes the nil value back to the encode method:

  value.as_json(options_for(value))

If we define an empty hash as as_json's default optional parameter in the model code, it will break because now it's nil but not empty hash.

Please consider returning empty hash if options is nil in the options_for method, like below:

    def options_for(value)
      if value.is_a?(Array) || value.is_a?(Hash)
        # hashes and arrays need to get encoder in the options, so that they can detect circular references
        (options || {}).merge(:encoder => self)
      else
        options || {}
      end
    end

@azimux azimux pushed a commit to azimux/rails that referenced this issue Dec 11, 2011

@chielwester @josevalim chielwester + josevalim Only call save on belongs_to associations if the record has changed o…
…r any nested associations have changed (resolves #3353)

Signed-off-by: José Valim <jose.valim@gmail.com>
658bb4f

@azimux azimux pushed a commit to azimux/rails that referenced this issue Dec 11, 2011

@chielwester @vijaydev chielwester + vijaydev Only call save on belongs_to associations if the record has changed o…
…r any nested associations have changed (resolves #3353)

Signed-off-by: José Valim <jose.valim@gmail.com>
bda16eb
Contributor

isaacsanders commented Apr 28, 2012

@nuoyan Is this still an issue?

nuoyan commented Apr 29, 2012

This was only a problem to me in the models that I override the as_json(options={}) method in the way that even I defined options={} as default parameter options would be nil if as_json is called without a parameter so I would get an exception.

My workaround was setting options ||= {} in the first line of my as_json method.

I haven't touched that code since then but I believe if the code mentioned in the original ticket hasn't changed this is probably still an issue, although not a blocking issue.

Contributor

isaacsanders commented Apr 29, 2012

Mk. Is it worth it to keep the ticket open, or would you be fine closing it?

nuoyan commented Apr 30, 2012

No I don't need to keep it open, although I believe there are people other than me running into this issue and using the similar workarounds, so I think it'd be helpful to people if that gets fixed. :)

Contributor

isaacsanders commented Apr 30, 2012

Would you be willing to contribute some code for this? @josevalim @rafaelfranca

Contributor

markmcspadden commented May 22, 2012

I took a look at this and I could be wrong but I believe that the following commit by @josevalim should have fixed this.

37b9594

Unfortunately it looks like it's not in until Rails 3.2 so 3.1 and 3.0 apps will have this issue.

In spending a little bit of time in edge rails, I can't find a way for options_for to return nil so it looks resolved moving forward.

Member

steveklabnik commented May 24, 2012

Since this is in 3.2, I'm closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment