Jira (PUP-11629) Puppet::Util::Json raises when reading an empty file

24 views
Skip to first unread message

Josh Cooper (Jira)

unread,
Sep 21, 2022, 12:49:01 PM9/21/22
to puppe...@googlegroups.com
Josh Cooper created an issue
 
Puppet / Bug PUP-11629
Puppet::Util::Json raises when reading an empty file
Issue Type: Bug Bug
Assignee: Unassigned
Created: 2022/09/21 9:48 AM
Priority: Normal Normal
Reporter: Josh Cooper

See details in PE-34535, a portion of the stack trace is below:

2022-09-08T15:05:11.285-04:00 ERROR [qtp2123677471-318634] [p.r.core] Internal Server Error: org.jruby.exceptions.RuntimeError: (InvalidMetadata) JSON::ParserError
    at opt.puppetlabs.puppet.lib.ruby.vendor_ruby.puppet.module.task.read_metadata(/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/module/task.rb:240)
    at opt.puppetlabs.puppet.lib.ruby.vendor_ruby.puppet.module.task.metadata(/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/module/task.rb:244)
    at opt.puppetlabs.puppet.lib.ruby.vendor_ruby.puppet.info_service.task_information_service.tasks_per_environment(/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/info_service/task_information_service.rb:9)

Puppet uses multi-json to wrap different json implementations. This way you can get better performance when using puppet as a library in different runtimes, e.g. Oj, JrJackson, etc.

When MultiJson.load is called it is supposed to according to the doc:

When loading invalid JSON, MultiJSON will throw a MultiJson::ParseError

However, when using the default json backend, it raises a JSON namespaced exception instead when trying to parse an empty string. This confuses Puppet::Util::Json.load and causes the JSON exception to leak.

This seems like a bug in multijson, but we should be more defensive when rescuing so.

Add Comment Add Comment
 
This message was sent by Atlassian Jira (v8.20.11#820011-sha1:0629dd8)
Atlassian logo

Josh Cooper (Jira)

unread,
Sep 21, 2022, 12:50:03 PM9/21/22
to puppe...@googlegroups.com

Josh Cooper (Jira)

unread,
Sep 21, 2022, 12:51:03 PM9/21/22
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Sprint: Phoenix 2022-10-12

Josh Cooper (Jira)

unread,
Sep 21, 2022, 12:52:01 PM9/21/22
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Fix Version/s: PUP 6.29.0
Fix Version/s: PUP 7.20.0

Josh Cooper (Jira)

unread,
Sep 21, 2022, 3:53:03 PM9/21/22
to puppe...@googlegroups.com

Morgan Rhodes (Jira)

unread,
Sep 22, 2022, 1:11:01 PM9/22/22
to puppe...@googlegroups.com

Morgan Rhodes (Jira)

unread,
Sep 22, 2022, 1:18:02 PM9/22/22
to puppe...@googlegroups.com
Morgan Rhodes updated an issue
Change By: Morgan Rhodes
Epic Link: PUP- 11618 11619

Josh Cooper (Jira)

unread,
Sep 30, 2022, 3:05:02 PM9/30/22
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-11629
 
Re: Puppet::Util::Json raises when reading an empty file

The JSON spec says:

A string is a sequence of zero or more Unicode characters

Ruby's default JSON gem, accepts empty string, but multi_json does not, even when using default JSON gem as the multi_json backend. I filed a bug upstream https://github.com/intridea/multi_json/issues/206.

For puppet, I think we want to accept empty strings and return nil.

Michael Hashizume (Jira)

unread,
Oct 3, 2022, 1:24:02 PM10/3/22
to puppe...@googlegroups.com

Josh Cooper (Jira)

unread,
Oct 4, 2022, 6:14:03 PM10/4/22
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Fix Version/s: PUP 6.29.0

Josh Cooper (Jira)

unread,
Oct 4, 2022, 6:50:02 PM10/4/22
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-11629
 
Re: Puppet::Util::Json raises when reading an empty file

Verified a task with empty metadata is treated as though the file was missing:
 

root@irritating-yell:~# cat test.rb 
require 'puppet'
 
Puppet.initialize_settings
 
environmentpath = Puppet[:environmentpath]
basemodulepath = Puppet::Node::Environment.split_path(Puppet[:basemodulepath])
directories = Puppet::Environments::Directories.from_path(environmentpath, basemodulepath)
loaders = Puppet::Environments::Cached.new(
  Puppet::Environments::Combined.new(*directories)
)
env = loaders.get!(Puppet[:environment])
mod = env.module('test')
tasks = mod.tasks
 
tasks.each do |t|
 puts t.metadata
end
 
root@irritating-yell:~# find /etc/puppetlabs/code/environments/production/modules/test
/etc/puppetlabs/code/environments/production/modules/test
/etc/puppetlabs/code/environments/production/modules/test/tasks
/etc/puppetlabs/code/environments/production/modules/test/tasks/something.json
/etc/puppetlabs/code/environments/production/modules/test/tasks/something.sh
root@irritating-yell:~# file /etc/puppetlabs/code/environments/production/modules/test/tasks/something.json
/etc/puppetlabs/code/environments/production/modules/test/tasks/something.json: empty

Before the fix:

root@irritating-yell:~# /opt/puppetlabs/puppet/bin/ruby test.rb 
Traceback (most recent call last):
	7: from test.rb:16:in `<main>'
	6: from test.rb:16:in `each'
	5: from test.rb:17:in `block in <main>'
	4: from /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/module/task.rb:244:in `metadata'
	3: from /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/module/task.rb:235:in `read_metadata'
	2: from /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/util/json.rb:64:in `load'
	1: from /opt/puppetlabs/puppet/lib/ruby/vendor_gems/gems/multi_json-1.15.0/lib/multi_json.rb:122:in `load'
/opt/puppetlabs/puppet/lib/ruby/vendor_gems/gems/multi_json-1.15.0/lib/multi_json/adapter.rb:20:in `load': JSON::ParserError (JSON::ParserError)

After the fix:

root@irritating-yell:~# /opt/puppetlabs/puppet/bin/ruby test.rb 
{}

Michael Hashizume (Jira)

unread,
Oct 5, 2022, 7:09:03 PM10/5/22
to puppe...@googlegroups.com
Michael Hashizume updated an issue
 
Change By: Michael Hashizume
Release Notes: Bug Fix
Release Notes Summary: Puppet no longer errors when loading an empty task metadata file.

Parker Leach (Jira)

unread,
Oct 6, 2022, 5:40:03 PM10/6/22
to puppe...@googlegroups.com
Reply all
Reply to author
Forward
0 new messages