The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com

This issue tracker is now in read-only archive mode and automatic ticket export has been disabled. Redmine users will need to create a new JIRA account to file tickets using https://tickets.puppetlabs.com. See the following page for information on filing tickets with JIRA:

Bug #19128

"puppet module build" doesn't escape PSON correctly

Added by Pierre Carrier about 3 years ago. Updated about 3 years ago.

Status:ClosedStart date:
Priority:HighDue date:
Assignee:Ryan Coleman% Done:

0%

Category:module tool
Target version:3.2.0
Affected Puppet version:3.1.0 Branch:https://github.com/puppetlabs/puppet/pull/1589
Keywords:

We've Moved!

Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com


Description

Erik Dalén’s modules are incompatible with ruby 1.9.3p327 and puppet 3.1.0 because he’s Swedish.

As soon as I specify [main] modulepath = /Users/pierre/repos/spotify-puppet/modules in ~/.puppet/puppet.conf, Puppet gets unhappy:

% puppet help
/Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/external/pson/pure/parser.rb:154:in `rescue in parse_string': Caught Encoding::CompatibilityError: incompatible encoding regexp match (ASCII-8BIT regexp with UTF-8 string) (PSON::GeneratorError)
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/external/pson/pure/parser.rb:133:in `parse_string'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/external/pson/pure/parser.rb:229:in `parse_object'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/external/pson/pure/parser.rb:98:in `parse'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/external/pson/common.rb:133:in `parse'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/module.rb:55:in `has_metadata?'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/module.rb:45:in `initialize'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/node/environment.rb:141:in `new'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/node/environment.rb:141:in `block (2 levels) in <class:Environment>'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/node/environment.rb:139:in `collect'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/node/environment.rb:139:in `block in <class:Environment>'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/util/cacher.rb:60:in `block in cached_value'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/1.9.1/monitor.rb:211:in `mon_synchronize'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/util/cacher.rb:58:in `cached_value'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/util/cacher.rb:31:in `block in cached_attr'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/node/environment.rb:96:in `each_plugin_directory'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/util/command_line.rb:124:in `run'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/util/command_line.rb:86:in `execute'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/bin/puppet:4:in `<top (required)>'
  from /Users/pierre/.gem/ruby/1.9.3/bin/puppet:23:in `load'
  from /Users/pierre/.gem/ruby/1.9.3/bin/puppet:23:in `<main>'
zsh: exit 1     puppet help/Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/external/pson/pure/parser.rb:154:in `rescue in parse_string': Caught Encoding::CompatibilityError: incompatible encoding regexp match (ASCII-8BIT regexp with UTF-8 string) (PSON::GeneratorError)
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/external/pson/pure/parser.rb:133:in `parse_string'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/external/pson/pure/parser.rb:229:in `parse_object'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/external/pson/pure/parser.rb:98:in `parse'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/external/pson/common.rb:133:in `parse'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/module.rb:55:in `has_metadata?'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/module.rb:45:in `initialize'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/node/environment.rb:141:in `new'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/node/environment.rb:141:in `block (2 levels) in <class:Environment>'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/node/environment.rb:139:in `collect'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/node/environment.rb:139:in `block in <class:Environment>'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/util/cacher.rb:60:in `block in cached_value'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/1.9.1/monitor.rb:211:in `mon_synchronize'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/util/cacher.rb:58:in `cached_value'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/util/cacher.rb:31:in `block in cached_attr'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/node/environment.rb:96:in `each_plugin_directory'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/util/command_line.rb:124:in `run'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/lib/puppet/util/command_line.rb:86:in `execute'
  from /Users/pierre/.rubies/1.9.3-p327/lib/ruby/gems/1.9.1/gems/puppet-3.1.0/bin/puppet:4:in `<top (required)>'
  from /Users/pierre/.gem/ruby/1.9.3/bin/puppet:23:in `load'
  from /Users/pierre/.gem/ruby/1.9.3/bin/puppet:23:in `<main>'
zsh: exit 1     puppet help

I decided not to investigate the PSON code as it’s past bedtime.

But from my understanding, the PSON parser.rb expects \u???? sequences for non-ASCII characters and rejects inline Unicode characters.

The PSON generator.rb seems supposed to take care of the escaping, so his Modulefiles are OK.

I reproduced the issue by regenerating metadata.json with my versions, ruby 1.9.3p327 and puppet 3.1.0 (GNU grep needed for this):

puppet-puppetdbquery % puppet module buildNotice: Building /Users/pierre/repos/puppet-puppetdbquery for release
Module built: /Users/pierre/repos/puppet-puppetdbquery/pkg/dalen-puppetdbquery-0.1.0.tar.gz
% tar xfO pkg/dalen-puppetdbquery-0.1.0.tar.gz dalen-puppetdbquery-0.1.0/metadata.json|grep -P '[\x80-\xff]'
  "author": "Erik Dalén <erik.gustav.dalen@gmail.com>",

History

#1 Updated by eric sorenson about 3 years ago

  • Status changed from Unreviewed to Investigating
  • Assignee set to Ryan Coleman

Ryan — this looks like the puppet module build routines in Puppet::ModuleTool::ModulefileReader that read in the modulefile ought to be more defensive about the input they take. I am unclear on whether this is a general pson problem though. Can you or Pieter please investigate?

#2 Updated by Pierre Carrier about 3 years ago

Why so? Shouldn’t you assume that the original document is UTF-8 and escape correctly? I don’t understand why you’d defend against non-ASCII names…

#3 Updated by Anonymous about 3 years ago

JSON is required to be encoded as some form of Unicode. According to the JSON spec:

All Unicode characters may be placed within the quotation marks except for the characters that must be escaped: quotation mark, reverse solidus, and the control characters (U+0000 through U+001F).

So no escaping is required for Unicode characters. Having said that, it may be necessary to convert the string into a Unicode encoding if it was originally in another encoding (like ASCII 8-bit). Failing to properly handle that introduces the possibility of failure from a number of vectors, but it’s most likely to happen in the face of real-world user input – puppet module build is one of the most direct paths between user input and the PSON generator.

I assert that while there are things that the build action could do to mitigate this failure, the PSON library should ultimately be responsible for not breaking in the face of unexpected character encodings.

#4 Updated by Erik Dalén about 3 years ago

  • Status changed from Investigating to In Topic Branch Pending Review
  • Branch set to https://github.com/puppetlabs/puppet/pull/1589

#5 Updated by Adrien Thebo about 3 years ago

  • Status changed from In Topic Branch Pending Review to Merged - Pending Release
  • Target version changed from 3.1.x to 3.2.0

Merged into master as 67b51ae.

This should be released in 3.2.0.

Thanks again for the contribution!

-Adrien

#6 Updated by Ryan Coleman about 3 years ago

Yay! Thank you all!

#7 Updated by Matthaus Owens about 3 years ago

  • Status changed from Merged - Pending Release to Closed

Released in Puppet 3.2.0-rc1

#8 Updated by Matthaus Owens about 3 years ago

Released in Puppet 3.2.0-rc1

Also available in: Atom PDF