(PUP-11841) Fix encoding of empty String #9102
Conversation
|
Can one of the admins verify this patch? |
In 2077971 empty string literals where replaced by a call to `String.new` in preparation for moving to frozen/immutable strings. However, as stated in the String#new documentation, when no value is passed to `#new` the returned string has the 'ASCII-8BIT' encoding instead of the default one (which is assumed to be 'UTF-8' by Puppet). This cause regressions in some areas of the code, for example when using the concat module with exported resource built using epp templates, the incorrect encoding cause the fragment to be misinterpreted and is base64 encoded. When collected, the base64 representation of the string is used instead of the actual value of the string, as reported here: voxpupuli/puppet-bacula#189 Replace calls to `String.new` with `''.dup` which use the current encoding. Do not change the few explicit but redundant occurrences of `String.new.force_encoding('ASCII-8BIT')` (so that the intent is clearly visible). Where appropriate, slightly adjust the code for better readability.
smortex
force-pushed
the
fix-empty-sting-encoding
branch
from
b3a058d to
fb20023
Compare
smortex
changed the title
|
Cc @joshcooper for review 😉 |
|
Thanks @smortex! I completely missed the encoding behavior difference. Why would Ruby do that!? |
My first reaction was that it should default to the current locale (just like From my findings, this behavior seems to have been the same since at least since Ruby 2.2.0: ruby/ruby@72194a8. Tricky, unexpected, but documented. |
|
Validated this with a puppet-agent run in our inteneral adhoc job. Passed integration testing with all supported platforms on the main branch. This LGTM. |
|
I don't have access to https://puppet.atlassian.net/browse/PUP-11841 for some reason, but while searching for this I found https://puppet.atlassian.net/browse/PUP-10096 ("Parameters of resources are base64 encoded in puppetdb if created as raw byte strings by ruby functions") which seems similar, or maybe the same thing. |
Yeah |
|
I filed https://perforce.atlassian.net/browse/PUP-11943 (which is unfortunately not public), but it's the issue that we're currently using the track this bug. It will be included in the next puppet 8.3 release. |
In 2077971 empty string literals where replaced by a call to
String.newin preparation for moving to frozen/immutable strings.However, as stated in the String#new documentation, when no value is passed to
#newthe returned string has the 'ASCII-8BIT' encoding instead of the default one (which is assumed to be 'UTF-8' by Puppet).This cause regressions in some areas of the code, for example when using the concat module with exported resource built using epp templates, the incorrect encoding cause the fragment to be misinterpreted and is base64 encoded. When collected, the base64 representation of the string is used instead of the actual value of the string, as reported here:
voxpupuli/puppet-bacula#189
Replace calls to
String.newwith''.dupwhich use the current encoding. Do not change the few explicit but redundant occurrences ofString.new.force_encoding('ASCII-8BIT')(so that the intent is clearly visible). Where appropriate, slightly adjust the code for betterreadability.