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

TypeError when using json_pure with Chef #20

Closed
tyler-ball opened this issue Apr 1, 2016 · 9 comments
Closed

TypeError when using json_pure with Chef #20

tyler-ball opened this issue Apr 1, 2016 · 9 comments

Comments

@tyler-ball
Copy link

This is a follow-up to #18 (comment)

We're seeing the following stacktrace when running Omnibus (another Chef product):

/home/jenkins/workspace/chef-build/architecture/x86_64/platform/el-5/project/chef/role/builder/omnibus/vendor/bundle/ruby/2.1.0/gems/json_pure-1.8.3/lib/json/pure/generator.rb:366:in `to_json': wrong argument type JSON::Pure::Generator::State (expected Data) (TypeError)
from /home/jenkins/workspace/chef-build/architecture/x86_64/platform/el-5/project/chef/role/builder/omnibus/vendor/bundle/ruby/2.1.0/gems/json_pure-1.8.3/lib/json/pure/generator.rb:366:in `block in json_transform'
from /home/jenkins/workspace/chef-build/architecture/x86_64/platform/el-5/project/chef/role/builder/omnibus/vendor/bundle/ruby/2.1.0/gems/json_pure-1.8.3/lib/json/pure/generator.rb:359:in `each'
from /home/jenkins/workspace/chef-build/architecture/x86_64/platform/el-5/project/chef/role/builder/omnibus/vendor/bundle/ruby/2.1.0/gems/json_pure-1.8.3/lib/json/pure/generator.rb:359:in `json_transform'
from /home/jenkins/workspace/chef-build/architecture/x86_64/platform/el-5/project/chef/role/builder/omnibus/vendor/bundle/ruby/2.1.0/gems/json_pure-1.8.3/lib/json/pure/generator.rb:341:in `to_json'
from /home/jenkins/workspace/chef-build/architecture/x86_64/platform/el-5/project/chef/role/builder/omnibus/vendor/bundle/ruby/2.1.0/gems/json_pure-1.8.3/lib/json/pure/generator.rb:293:in `generate'
from /home/jenkins/workspace/chef-build/architecture/x86_64/platform/el-5/project/chef/role/builder/omnibus/vendor/bundle/ruby/2.1.0/gems/json_pure-1.8.3/lib/json/common.rb:285:in `pretty_generate'
from /home/jenkins/workspace/chef-build/architecture/x86_64/platform/el-5/project/chef/role/builder/omnibus/vendor/bundle/ruby/2.1.0/bundler/gems/omnibus-961695bbc1b2/lib/omnibus/project.rb:1090:in `block in write_json_manifest'
from /home/jenkins/workspace/chef-build/architecture/x86_64/platform/el-5/project/chef/role/builder/omnibus/vendor/bundle/ruby/2.1.0/bundler/gems/omnibus-961695bbc1b2/lib/omnibus/project.rb:1089:in `open'
from /home/jenkins/workspace/chef-build/architecture/x86_64/platform/el-5/project/chef/role/builder/omnibus/vendor/bundle/ruby/2.1.0/bundler/gems/omnibus-961695bbc1b2/lib/omnibus/project.rb:1089:in `write_json_manifest'
from /home/jenkins/workspace/chef-build/architecture/x86_64/platform/el-5/project/chef/role/builder/omnibus/vendor/bundle/ruby/2.1.0/bundler/gems/omnibus-961695bbc1b2/lib/omnibus/project.rb:1080:in `build'
from /home/jenkins/workspace/chef-build/architecture/x86_64/platform/el-5/project/chef/role/builder/omnibus/vendor/bundle/ruby/2.1.0/bundler/gems/omnibus-961695bbc1b2/lib/omnibus/cli.rb:83:in `build'
from /home/jenkins/workspace/chef-build/architecture/x86_64/platform/el-5/project/chef/role/builder/omnibus/vendor/bundle/ruby/2.1.0/gems/thor-0.19.1/lib/thor/command.rb:27:in `run'
from /home/jenkins/workspace/chef-build/architecture/x86_64/platform/el-5/project/chef/role/builder/omnibus/vendor/bundle/ruby/2.1.0/gems/thor-0.19.1/lib/thor/invocation.rb:126:in `invoke_command'
from /home/jenkins/workspace/chef-build/architecture/x86_64/platform/el-5/project/chef/role/builder/omnibus/vendor/bundle/ruby/2.1.0/gems/thor-0.19.1/lib/thor.rb:359:in `dispatch'
from /home/jenkins/workspace/chef-build/architecture/x86_64/platform/el-5/project/chef/role/builder/omnibus/vendor/bundle/ruby/2.1.0/bundler/gems/omnibus-961695bbc1b2/lib/omnibus/cli/base.rb:33:in `dispatch'
from /home/jenkins/workspace/chef-build/architecture/x86_64/platform/el-5/project/chef/role/builder/omnibus/vendor/bundle/ruby/2.1.0/gems/thor-0.19.1/lib/thor/base.rb:440:in `start'
from /home/jenkins/workspace/chef-build/architecture/x86_64/platform/el-5/project/chef/role/builder/omnibus/vendor/bundle/ruby/2.1.0/bundler/gems/omnibus-961695bbc1b2/lib/omnibus/cli.rb:41:in `execute!'
from /home/jenkins/workspace/chef-build/architecture/x86_64/platform/el-5/project/chef/role/builder/omnibus/vendor/bundle/ruby/2.1.0/bundler/gems/omnibus-961695bbc1b2/bin/omnibus:16:in `<top (required)>'
from /home/jenkins/workspace/chef-build/architecture/x86_64/platform/el-5/project/chef/role/builder/omnibus/vendor/bundle/ruby/2.1.0/bin/omnibus:23:in `load'
from /home/jenkins/workspace/chef-build/architecture/x86_64/platform/el-5/project/chef/role/builder/omnibus/vendor/bundle/ruby/2.1.0/bin/omnibus:23:in `<main>'

Omnibus is calling JSON.pretty_generate and passing it a hash. I realize this is probably a bug in json_pure (maybe json_pure#198 or json_pure#186) but we are currently downgrading our jmespath version to prevent pulling in json_pure.

@tyler-ball
Copy link
Author

@xeron We only saw this issue in our build system - can you provide a full stacktrace of where you are seeing this error using Chef?

@xeron
Copy link

xeron commented Apr 1, 2016

I don't have stacktrace right now because we've also locked jmespath version to 1.1.x and already killed bad hosts.

This error was from our internal cookbook which calls JSON.parse.

@lamont-granquist
Copy link

all that omnibus is doing is calling JSON.pretty_generate() with a hash.

https://github.com/chef/omnibus/blob/master/lib/omnibus/project.rb#L1090

its getting redirected to json_pure because that gem patches all its own methods in for the JSON gem.

most likely not a bug in this gem and instead a bug in the json_pure gem itself.

@tyler-ball
Copy link
Author

A confusing aspect is that we're only experiencing this issue on certain builders. On all those builders, we already have the JSON gem installed - so the checks here and here should be loading json/ext and not json/pure.

@trevorrowe
Copy link
Contributor

I have a meeting to run to, but I have been able to reproduce this, and I think I have a few thoughts on how we can avoid this. Sorry for the suspense!

@tyler-ball
Copy link
Author

@trevorrowe No worries - nothing like an exciting friday morning! 🎉 🚀 🎉

@trevorrowe
Copy link
Contributor

I put together a simple ruby script that reproduces the issue above. For this script to demonstrate the failure you need to be running Ruby 1.9.3, have jmespath 1.2.2 installed, and must not have version >= 1.8.1 of the json gem.

require 'json'
require 'jmespath'

puts "ruby version: #{RUBY_VERSION}"          # should be 1.9.3
puts "json version: #{JSON::VERSION}"         # should be 1.5.5
puts "jmespath version: #{JMESPath::VERSION}" # should be 1.2.2

# this should raise a JSON::Pure::Generator::State TypeError
some_hash = { 'jsonrpc' => 'xxxx', 'jsonversion' => 1 }
some_hash.to_json

As shown above, if your application has require 'json' prior to require 'jmespath' then Ruby 1.9.3 will load your json native gem. After the native gem is loaded, jmespath will run the following code:

begin
  # Attempt to load the native version if available, not availble
  # by default for Ruby 1.9.3.
  gem('json', '>= 1.8.1')
  require 'json/ext'
rescue Gem::LoadError
  # Fallback on the json_pure gem dependency.
  gem('json_pure', '>= 1.8.1')
  require 'json/pure'
end

The first attempt will fail, because the #gem method call raises Gem::LoadError as json >= 1.8.1 is not available. It falls back on json_pure >= 1.8.1 which is available. This causes json_pure to mixin its modules on-top of those mixed in by json/ext. This is essentially the crux of ruby/json#186.

A suitable workaround may be to have jmespath check to see if the JSON constant is defined first. If it is, it should not attempt to load json/ext or json/pure. This however may result in a version older than 1.8.1 being loaded. I suppose it could log a warning if this is the case. Having an older version will prevent it from correctly handling jmespath expressions that contains literal json values.

@trevorrowe
Copy link
Contributor

I modified my local copy of jmespath with the following:

if Object.const_defined?(:JSON) && JSON::VERSION < '1.8.1'
  warn("WARNING: jmespath gem requires json gem >= 1.8.1; json #{JSON::VERSION} already loaded")
else
  begin
    gem('json', '>= 1.8.1')
    require 'json/ext'
  rescue Gem::LoadError
    gem('json_pure', '>= 1.8.1')
    require 'json/pure'
  end
end

The result of running jmespath with an older version of json prevents it from parsing json literals, for example:

puts JMESPath.search('`1` > `2`', {})
#=> correct answer is false
#=> old version json, raises unexpected token 0 (JMESPath::Errors::SyntaxError)

This essentially means users that have loaded an older version of json will have a degraded experience using jmespath. To be fair, this is the same situation users found themselves in between 1.1.3 and 1.2.

Any thoughts on if the const defined check would be sufficient?

@trevorrowe
Copy link
Contributor

The condition require has been merged in and released. This should hopefully prevents environments that have loaded json prior to loading jmespath from breaking json methods.

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

4 participants