Catch encoding problems with sources provided to Vagrant::Config::Loader#set

Here we implement a naive solution to #5605 which catches the case that
a provided source contains an object which cannot be inspected, because
an object contained within in has an #inspect string that returns a
string that is incompatible with the encoding in
`Encoding.default_external` or a string which cannot be downcast to
7-bit ascii.

The Ruby VM implementation of "#inspect" implements this checking on
these lines of code: http://git.io/vZYNS. A Ruby level override of
this method does not cause this problem. For example:

```ruby
class Foo
  def inspect
    "😍".encode("UTF-16LE")
  end
```

will not cause the problem, because that's a Ruby implementation and the
VM's checks don't occur.

However, if we have an Object which **does** use the VM implementation
of inspect, that contains an object that has an inspect string which
returns non-ascii, we encounter the bug. For example:

```ruby
class Bar
  def inspect
    "😍".encode("UTF-16LE")
  end
end

class Foo
  def initialize
     @bar = Bar.new
  end
end

Foo.new.inspect
```

Will cause the issue.

The solution this patch provides basically catches the encoding error
and inserts a string which attempts to help the user work out which
object was provided without blowing up. Most likely, this was caused
by a user having a weird encoding coming out of one of the sources
passed in, but without a full repro case, it's not clear whether a patch
should be applied to a different object in the system.

Closes #5605.
This commit is contained in:
Sam Phippen 2015-09-08 17:30:50 +01:00
parent 790fa9f8e2
commit eeb750cd33
2 changed files with 20 additions and 7 deletions

View File

@ -41,11 +41,21 @@ module Vagrant
# `set` multiple times with the same name will override any previously
# set values. In this way, the last set data for a given name wins.
def set(name, sources)
@logger.info("Set #{name.inspect} = #{sources.inspect}")
# Sources should be an array
sources = [sources] if !sources.kind_of?(Array)
reliably_inspected_sources = sources.reduce({}) { |accum, source|
begin
accum[source] = source.inspect
rescue Encoding::CompatibilityError
accum[source] = "<!Vagrant failed to call #inspect source with object id #{source.object_id} and class #{source.class} due to a string encoding error>"
end
accum
}
@logger.info("Set #{name.inspect} = #{reliably_inspected_sources.values}")
# Gather the procs for every source, since that is what we care about.
procs = []
sources.each do |source|
@ -53,8 +63,8 @@ module Vagrant
# Load the procs for this source and cache them. This caching
# avoids the issue where a file may have side effects when loading
# and loading it multiple times causes unexpected behavior.
@logger.debug("Populating proc cache for #{source.inspect}")
@proc_cache[source] = procs_for_source(source)
@logger.debug("Populating proc cache for #{reliably_inspected_sources[source]}")
@proc_cache[source] = procs_for_source(source, reliably_inspected_sources)
end
# Add on to the array of procs we're going to use
@ -164,7 +174,7 @@ module Vagrant
# The `Proc` objects returned will expect a single argument for
# the configuration object and are expected to mutate this
# configuration object.
def procs_for_source(source)
def procs_for_source(source, reliably_inspected_sources)
# Convert all pathnames to strings so we just have their path
source = source.to_s if source.is_a?(Pathname)
@ -180,7 +190,7 @@ module Vagrant
# Strings are considered paths, so load them
return procs_for_path(source)
else
raise ArgumentError, "Unknown configuration source: #{source.inspect}"
raise ArgumentError, "Unknown configuration source: #{reliably_inspected_sources[source]}"
end
end

View File

@ -61,8 +61,11 @@ describe Vagrant::Config::Loader do
end
end.new(klass_with_bad_inspect_string)
}
it "does not raise the ascii encoding exception" do
instance.set(:arbitrary, test_source)
expect {
instance.set(:arbitrary, test_source)
}.to raise_error(ArgumentError, /Unknown configuration source/)
end
end
end