[PUP-7880] When logging mixed encoding strings, Puppet may fail Created: 2017/08/25  Updated: 2017/09/14  Resolved: 2017/08/30

Status: Closed
Project: Puppet
Component/s: Windows
Affects Version/s: PUP 5.0.0
Fix Version/s: PUP 5.2.0

Type: CI Blocker Priority: Blocker
Reporter: Ethan Brown Assignee: Ethan Brown
Resolution: Fixed Votes: 0
Labels: i18n, utf-8, windows
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
relates to PUP-5756 Remove usage of win32-eventlog gem Closed
CI Pipeline/s:
platform puppet-agent
Epic Link: Puppet Unicode Adoption Blockers
Team: Platform Core
Story Points: 2
Sprint: Platform Core 2017-09-05
Release Notes: Not Needed
QA Risk Assessment: No Action


The console logger (amongst others) makes assumptions that strings can be concatenated as passed - https://github.com/puppetlabs/puppet/blob/master/lib/puppet/util/log/destinations.rb#L167

str = msg.source == "Puppet" ? str : "#{msg.source}: #{str}"

This assumes that the logger has made all attributes of the log message match encodings (as UTF-8) prior to handing off a message with handle, so that each individual logger is not responsible for doing so - the call to dest.handle(msg) is at https://github.com/puppetlabs/puppet/blob/master/lib/puppet/util/log.rb#L187

It is possible (reproducible through Cygwin on Windows) to get a source that represents a Puppet file resource path, with an encoding of IBM437... and a message of UTF-8 containing characters above the 7-bit ASCII range (that were converted form IBM437 as part of PUP-5756 / https://github.com/puppetlabs/puppet/commit/c47be2d3b3d002f02cbed6edee8dfee119152b46). When this happens, Ruby will fail to concatenate the incompatible strings and crash (without actually mentioning the logger that caused the crash).

For instance when source is in IBM437 as


and message in UTF-8 is

subscribes to File[c:/temp]

as reproducible (only through Cygwin / Beaker) with the manifest:

file { "c:/temp":
  ensure => directory
file { "c:/temp/unicode_dir_ㅀㅅㅶㅅㅲㅂㅄㅉㅑㅧㅩㅙㅘ":
  ensure  => directory,
  require => File['c:/temp']
user { "bob":
  ensure     => present,
  groups     => 'Users',
  managehome => true,
  password   => "L0v3Pupp3t!"
acl { "c:/temp/unicode_dir_ㅀㅅㅶㅅㅲㅂㅄㅉㅑㅧㅩㅙㅘ":
  permissions => [
    { identity => 'bob', rights => ['full'] },

Error: Failed to apply catalog: incompatible character encodings: IBM437 and UTF-8
  C:/Program Files/Puppet Labs/Puppet/sys/ruby/lib/ruby/vendor_ruby/puppet/util/log/destinations.rb:167:in `handle'
  C:/Program Files/Puppet Labs/Puppet/sys/ruby/lib/ruby/vendor_ruby/puppet/util/log.rb:187:in `block in newmessage'
  C:/Program Files/Puppet Labs/Puppet/sys/ruby/lib/ruby/vendor_ruby/puppet/util/log.rb:186:in `each'
  C:/Program Files/Puppet Labs/Puppet/sys/ruby/lib/ruby/vendor_ruby/puppet/util/log.rb:186:in `newmessage'
  C:/Program Files/Puppet Labs/Puppet/sys/ruby/lib/ruby/vendor_ruby/puppet/util/log.rb:304:in `initialize'
  C:/Program Files/Puppet Labs/Puppet/sys/ruby/lib/ruby/vendor_ruby/puppet/util/log.rb:87:in `new'
  C:/Program Files/Puppet Labs/Puppet/sys/ruby/lib/ruby/vendor_ruby/puppet/util/log.rb:87:in `create'
  C:/Program Files/Puppet Labs/Puppet/sys/ruby/lib/ruby/vendor_ruby/puppet/util/logging.rb:12:in `send_log'
  C:/Program Files/Puppet Labs/Puppet/sys/ruby/lib/ruby/vendor_ruby/puppet/util/logging.rb:40:in `debug'

This was discovered while executing tests against the ACL module and Puppet 5 https://github.com/puppetlabs/puppetlabs-acl/pull/111 and is a net new problem as a result of PUP-5756. This is not a problem in the Puppet 4.10 series.

Comment by Ethan Brown [ 2017/08/25 ]

To follow-up on why I was able to run the manifest successfully locally, while not through Beaker / Cygwin. The Beaker / Cygwin situation was using `puppet apply` and taking a UTF-8 manifest and pushing it over stdin / through SSH. When pushed over stdin, the input was treated as IBM437 instead of UTF-8 as would be parsed from a manifest. As a result, things get wacky quickly.

I'm going to update those ACL tests to scp a manifest file over, rather than pushing it out over stdin – but this PR merge need not wait for that.

Comment by Moses Mendoza [ 2017/08/25 ]

merged to master at https://github.com/puppetlabs/puppet/commit/bca40e0d2d67706a4b1719f932b5ef76ef34ccc0

Generated at Sat Jul 11 07:30:13 PDT 2020 using Jira 8.5.2#805002-sha1:a66f9354b9e12ac788984e5d84669c903a370049.