Skip to content

Commit

Permalink
(PUP-11841) Fix encoding of empty String
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
smortex committed Aug 21, 2023
1 parent 5e1cea2 commit fb20023
Show file tree
Hide file tree
Showing 47 changed files with 69 additions and 70 deletions.
2 changes: 1 addition & 1 deletion lib/puppet/application/doc.rb
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ def rdoc
end

def other
text = String.new
text = ''.dup
with_contents = options[:references].length <= 1
exit_code = 0
require_relative '../../puppet/util/reference'
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/face/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
end

when_rendering :console do |to_be_rendered|
output = String.new
output = ''.dup
if to_be_rendered.keys.length > 1
to_be_rendered.keys.sort.each do |setting|
output << "#{setting} = #{to_be_rendered[setting]}\n"
Expand Down
4 changes: 2 additions & 2 deletions lib/puppet/face/epp.rb
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@
end

def dump_parse(source, filename, options, show_filename = true)
output = String.new
output = ''.dup
evaluating_parser = Puppet::Pops::Parser::EvaluatingParser::EvaluatingEppParser.new
begin
if options[:validate]
Expand Down Expand Up @@ -451,7 +451,7 @@ def render_inline(epp_source, compiler, options)

def render_file(epp_template_name, compiler, options, show_filename, file_nbr)
template_args = get_values(compiler, options)
output = String.new
output = ''.dup
begin
if show_filename && options[:header]
output << "\n" unless file_nbr == 1
Expand Down
4 changes: 2 additions & 2 deletions lib/puppet/face/module/list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
environment = result[:environment]
modules_by_path = result[:modules_by_path]

output = String.new
output = ''.dup

warn_unmet_dependencies(environment)

Expand Down Expand Up @@ -248,7 +248,7 @@ def list_build_tree(list, ancestors=[], parent=nil, params={})
# Returns a Hash
#
def list_build_node(mod, parent, params)
str = String.new
str = ''.dup
str << (mod.forge_name ? mod.forge_name.tr('/', '-') : mod.name)
str << ' (' + colorize(:cyan, mod.version ? "v#{mod.version}" : '???') + ')'

Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/face/parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@
end

def dump_parse(source, filename, options, show_filename = true)
output = String.new
output = ''.dup
evaluating_parser = Puppet::Pops::Parser::EvaluatingParser.new
begin
if options[:validate]
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/indirector/facts/facter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ def self.setup_external_search_paths(request)

def find_with_options(request)
options = request.options
options_for_facter = String.new
options_for_facter = ''.dup
options_for_facter += options[:user_query].join(' ')
options_for_facter += " --config #{options[:config_file]}" if options[:config_file]
options_for_facter += " --show-legacy" if options[:show_legacy]
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/indirector/file_bucket_file/file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def list(request)
end
# Setting hash's default value to [], needed by the following loop
bucket = Hash.new {[]}
msg = String.new
msg = ''.dup
# Get all files with mtime between 'from' and 'to'
Pathname.new(request.options[:bucket_path]).find { |item|
if item.file? and item.basename.to_s == "paths"
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/indirector/indirection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def expiration

# Generate the full doc string.
def doc
text = String.new
text = ''.dup

text << scrub(@doc) << "\n\n" if @doc

Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/module_tool.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def self.is_module_root?(path)
# Builds a formatted tree from a list of node hashes containing +:text+
# and +:dependencies+ keys.
def self.format_tree(nodes, level = 0)
str = String.new
str = ''.dup
nodes.each_with_index do |node, i|
last_node = nodes.length - 1 == i
deps = node[:dependencies] || []
Expand Down
6 changes: 3 additions & 3 deletions lib/puppet/network/formats.rb
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ def render(datum)

# Simple hash to table
if datum.is_a?(Hash) && datum.keys.all? { |x| x.is_a?(String) || x.is_a?(Numeric) }
output = String.new
output = ''.dup
column_a = datum.empty? ? 2 : datum.map{ |k,v| k.to_s.length }.max + 2
datum.sort_by { |k,v| k.to_s } .each do |key, value|
output << key.to_s.ljust(column_a)
Expand All @@ -169,7 +169,7 @@ def render(datum)

# Print one item per line for arrays
if datum.is_a? Array
output = String.new
output = ''.dup
datum.each do |item|
output << item.to_s
output << "\n"
Expand Down Expand Up @@ -227,7 +227,7 @@ def flatten_array(array)
end

def construct_output(data)
output = String.new
output = ''.dup
data.each do |key, value|
output << "#{key}=#{value}"
output << "\n"
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/network/http/memory_response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ class Puppet::Network::HTTP::MemoryResponse
attr_reader :code, :type, :body

def initialize
@body = String.new
@body = ''.dup
end

def respond_with(code, type, body)
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/parameter/value_collection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def aliasvalue(name, other)
#
def doc
unless defined?(@doc)
@doc = String.new
@doc = ''.dup
unless values.empty?
@doc << "Valid values are "
@doc << @strings.collect do |value|
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/parser/functions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ def self.function(name, environment = Puppet.lookup(:current_environment))
def self.functiondocs(environment = Puppet.lookup(:current_environment))
autoloader.delegatee.loadall(environment)

ret = String.new
ret = ''.dup

merged_functions(environment).sort { |a,b| a[0].to_s <=> b[0].to_s }.each do |name, hash|
ret << "#{name}\n#{"-" * name.to_s.length}\n"
Expand Down
8 changes: 4 additions & 4 deletions lib/puppet/pops/loader/loader_paths.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def effective_path(typed_name, start_index_in_name)

def typed_name(type, name_authority, relative_path, module_name)
# Module name is assumed to be included in the path and therefore not added here
n = String.new
n = ''.dup
unless extension.empty?
# Remove extension
relative_path = relative_path[0..-(extension.length+1)]
Expand Down Expand Up @@ -153,7 +153,7 @@ def effective_path(typed_name, start_index_in_name)
end

def typed_name(type, name_authority, relative_path, module_name)
n = String.new
n = ''.dup
n << module_name unless module_name.nil?
unless extension.empty?
# Remove extension
Expand Down Expand Up @@ -249,7 +249,7 @@ def relative_path
end

def typed_name(type, name_authority, relative_path, module_name)
n = String.new
n = ''.dup
n << module_name unless module_name.nil?

# Remove the file extension, defined as everything after the *last* dot.
Expand Down Expand Up @@ -351,7 +351,7 @@ def typed_name(type, name_authority, relative_path, module_name)
if @init_filenames.include?(relative_path) && !(module_name.nil? || module_name.empty?)
TypedName.new(type, module_name, name_authority)
else
n = String.new
n = ''.dup
n << module_name unless module_name.nil?
ext = @extensions.find { |extension| relative_path.end_with?(extension) }
relative_path = relative_path[0..-(ext.length+1)]
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/pops/lookup/explainer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def to_hash
end

def explain
io = String.new
io = ''.dup
dump_on(io, '', '')
io
end
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/pops/lookup/hiera_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ def find_line_matching(regexp, start_line = 1)
line_number += 1
next if line_number < start_line
quote = nil
stripped = String.new
stripped = ''.dup
line.each_codepoint do |cp|
if cp == 0x22 || cp == 0x27 # double or single quote
if quote == cp
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/pops/model/factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1112,7 +1112,7 @@ def is_interop_rewriteable?(o)
end

def self.concat(*args)
result = String.new
result = ''.dup
args.each do |e|
if e.instance_of?(Factory) && e.model_class <= LiteralString
result << e[KEY_VALUE]
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/pops/model/tree_dumper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def indent
end

def format(x)
result = String.new
result = ''.dup
parts = format_r(x)
parts.each_index do |i|
if i > 0
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/pops/parser/epp_support.rb
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ def scan(skip_leading=false)
@skip_leading = skip_leading

return nil if scanner.eos?
s = String.new
s = ''.dup
until scanner.eos?
part = @scanner.scan_until(/(<%)|\z/)
if @skip_leading
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/pops/parser/evaluating_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ def quote(x)
# @return [String] The quoted string
#
def self.quote(x)
escaped = String.new('"')
escaped = '"'.dup
p = nil
x.each_char do |c|
case p
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/pops/parser/pn_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ def consume_identifier(s)

def consume_string
s = @pos
b = String.new
b = ''.dup
loop do
c = next_cp
case c
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/pops/pn.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def ==(o)
end

def to_s
s = String.new
s = ''.dup
format(nil, s)
s
end
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/pops/serialization/json_path.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module JsonPath
#
# @api private
def self.to_json_path(path)
p = String.new('$')
p = '$'.dup
path.each do |seg|
if seg.nil?
p << '[null]'
Expand Down
6 changes: 3 additions & 3 deletions lib/puppet/pops/time/timespan.rb
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,7 @@ def initialize(format, segments)
end

def format(timespan)
bld = String.new(timespan.negative? ? '-' : '')
bld = timespan.negative? ? '-'.dup : ''.dup
@segments.each { |segment| segment.append_to(bld, timespan) }
bld
end
Expand Down Expand Up @@ -575,7 +575,7 @@ def regexp
end

def build_regexp
bld = String.new('\A-?')
bld = '\A-?'.dup
@segments.each { |segment| segment.append_regexp(bld) }
bld << '\z'
Regexp.new(bld)
Expand Down Expand Up @@ -613,7 +613,7 @@ def bad_format_specifier(format, start, position)

def append_literal(bld, codepoint)
if bld.empty? || !bld.last.is_a?(Format::LiteralSegment)
bld << Format::LiteralSegment.new(String.new.concat(codepoint))
bld << Format::LiteralSegment.new(''.dup.concat(codepoint))
else
bld.last.concat(codepoint)
end
Expand Down
4 changes: 2 additions & 2 deletions lib/puppet/pops/types/ruby_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def create_class(obj)
if cls.nil?
rp = key.resolved_parent
parent_class = rp.is_a?(PObjectType) ? rp.implementation_class : Object
class_def = String.new
class_def = ''.dup
class_body(key, EMPTY_ARRAY, class_def)
cls = Class.new(parent_class)
cls.class_eval(class_def)
Expand Down Expand Up @@ -109,7 +109,7 @@ def module_definition(types, comment, *impl_subst)
end

# Create class definition of all contained types
bld = String.new
bld = ''.dup
start_module(common_prefix, comment, bld)
class_names = []
names_by_prefix.each_pair do |seg_array, index_and_name_array|
Expand Down
12 changes: 6 additions & 6 deletions lib/puppet/pops/types/string_converter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,7 @@ def string_PBooleanType(val_type, val, format_map, indentation)
# Performs post-processing of literals to apply width and precision flags
def apply_string_flags(f, literal_str)
if f.left || f.width || f.prec
fmt = String.new('%')
fmt = '%'.dup
fmt << '-' if f.left
fmt << f.width.to_s if f.width
fmt << '.' << f.prec.to_s if f.prec
Expand Down Expand Up @@ -853,7 +853,7 @@ def puppet_quote(str, enforce_double_quotes = false)
end

# Assume that the string can be single quoted
bld = String.new('\'')
bld = "'".dup
bld.force_encoding(str.encoding)
escaped = false
str.each_codepoint do |codepoint|
Expand All @@ -879,12 +879,12 @@ def puppet_quote(str, enforce_double_quotes = false)
# If string ended with a backslash, then that backslash must be escaped
bld << 0x5c if escaped

bld << '\''
bld << "'"
bld
end

def puppet_double_quote(str)
bld = String.new('"')
bld = '"'.dup
str.each_codepoint do |codepoint|
case codepoint
when 0x09
Expand Down Expand Up @@ -940,7 +940,7 @@ def string_PArrayType(val_type, val, format_map, indentation)

case format.format
when :a, :s, :p
buf = String.new
buf = ''.dup
if indentation.breaks?
buf << "\n"
buf << indentation.padding
Expand Down Expand Up @@ -1055,7 +1055,7 @@ def string_PHashType(val_type, val, format_map, indentation)

when :h, :s, :p
indentation = indentation.indenting(format.alt? || indentation.is_indenting?)
buf = String.new
buf = ''.dup
if indentation.breaks?
buf << "\n"
buf << indentation.padding
Expand Down
4 changes: 2 additions & 2 deletions lib/puppet/pops/types/type_formatter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def ruby(ref_ctor)
# @api public
#
def string(t)
@bld = String.new
@bld = ''.dup
append_string(t)
@bld
end
Expand All @@ -64,7 +64,7 @@ def string(t)
#
# @api public
def indented_string(t, indent = 0, indent_width = 2)
@bld = String.new
@bld = ''.dup
append_indented_string(t, indent, indent_width)
@bld
end
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/pops/types/types.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1713,7 +1713,7 @@ def self.append_flags_group(rx_string, options)
if options == 0
rx_string
else
bld = String.new('(?')
bld = '(?'.dup
bld << 'i' if (options & Regexp::IGNORECASE) != 0
bld << 'm' if (options & Regexp::MULTILINE) != 0
bld << 'x' if (options & Regexp::EXTENDED) != 0
Expand Down
Loading

0 comments on commit fb20023

Please sign in to comment.