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

Object.as_json is too optimistic and causes errors in 3rd party code. #51626

Open
ioquatix opened this issue Apr 21, 2024 · 4 comments
Open

Object.as_json is too optimistic and causes errors in 3rd party code. #51626

ioquatix opened this issue Apr 21, 2024 · 4 comments

Comments

@ioquatix
Copy link
Contributor

This monkey patch is dangerous:

class Object
def as_json(options = nil) # :nodoc:
if respond_to?(:to_hash)
to_hash.as_json(options)
else
instance_values.as_json(options)
end
end
end

Specifically, I've seen this issue crop up a number of times. A casual search yields issues that appear to be related:

I don't think as_json should be monkey patched into Object like this. The default behaviour of using to_s is safer. Alternatively, objects that refer to themselves (circular references) should be handled correctly.

@ioquatix
Copy link
Contributor Author

I guess it was introduced here: e567a5e

cc @jeremy you have worked on that code quite a bit, do you have any opinion?

@ioquatix
Copy link
Contributor Author

A safer default might be this:

 class Object 
   def as_json(options = nil) # :nodoc: 
     if respond_to?(:to_hash) 
       to_hash.as_json(options) 
     else 
       self.to_s
     end 
   end 
 end 

@Earlopain
Copy link
Contributor

I've also seen this reported to sidekiq a few times:

This was eventually fixed by implementing a custom to_hash in sidekiq/sidekiq#6198

@ioquatix
Copy link
Contributor Author

The problem with to_hash is it makes an object implicitly convertible (equivalent to) a Hash. It would have made more sense to use to_h (explicitly convertible).

See https://zverok.space/blog/2016-01-18-implicit-vs-expicit.html for a good summary of the differences.

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

2 participants