[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
relates to PUP-5756 Remove usage of win32-eventlog gem Closed
CI Pipeline/s:
platform puppet-agent
Epic Link: Phase 1.1 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

 Description   

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

/Stage[main]/Main/File[c:/temp/unicode_dir_ㅀㅅㅶㅅㅲㅂㅄㅉㅑㅧㅩㅙㅘ]/require

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.



 Comments   
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 Sun Aug 18 14:02:14 PDT 2019 using JIRA 7.7.1#77002-sha1:e75ca93d5574d9409c0630b81c894d9065296414.