From f5b75ed0d66d8195d43ca6ccc2e0496a8f00d6b1 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 4 May 2020 13:25:53 -0700 Subject: [PATCH] Update IO util to properly handle unknown conversion errors When converting encoding to UTF-8 on Windows, allow for unknown conversions to be properly handled and prevent generating an error. --- lib/vagrant/util/io.rb | 34 ++------- test/unit/vagrant/util/io_test.rb | 111 ++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 27 deletions(-) create mode 100644 test/unit/vagrant/util/io_test.rb diff --git a/lib/vagrant/util/io.rb b/lib/vagrant/util/io.rb index c75257b9e..5e079c08f 100644 --- a/lib/vagrant/util/io.rb +++ b/lib/vagrant/util/io.rb @@ -29,42 +29,22 @@ module Vagrant break if !results || results[0].empty? # Read! - data << io.readpartial(READ_CHUNK_SIZE).encode("UTF-8", Encoding.default_external) + data << io.readpartial(READ_CHUNK_SIZE).encode( + "UTF-8", Encoding.default_external, + invalid: :replace, + undef: :replace + ) else # Do a simple non-blocking read on the IO object data << io.read_nonblock(READ_CHUNK_SIZE) end - rescue Exception => e - # The catch-all rescue here is to support multiple Ruby versions, - # since we use some Ruby 1.9 specific exceptions. - - breakable = false - if e.is_a?(EOFError) - # An `EOFError` means this IO object is done! - breakable = true - elsif defined?(::IO::WaitReadable) && e.is_a?(::IO::WaitReadable) - # IO::WaitReadable is only available on Ruby 1.9+ - - # An IO::WaitReadable means there may be more IO but this - # IO object is not ready to be read from yet. No problem, - # we read as much as we can, so we break. - breakable = true - elsif e.is_a?(Errno::EAGAIN) - # Otherwise, we just look for the EAGAIN error which should be - # all that IO::WaitReadable does in Ruby 1.9. - breakable = true - end - - # Break out if we're supposed to. Otherwise re-raise the error - # because it is a real problem. - break if breakable - raise + rescue EOFError, Errno::EAGAIN, ::IO::WaitReadable + break end end data end - end end end diff --git a/test/unit/vagrant/util/io_test.rb b/test/unit/vagrant/util/io_test.rb new file mode 100644 index 000000000..504b5207c --- /dev/null +++ b/test/unit/vagrant/util/io_test.rb @@ -0,0 +1,111 @@ +# -*- coding: utf-8 -*- +require File.expand_path("../../../base", __FILE__) + +require 'vagrant/util/io' + +describe Vagrant::Util::IO do + describe ".read_until_block" do + let(:io) { double("io") } + + before do + # Ensure that we don't get stuck in a loop + allow(io).to receive(:read_nonblock).and_raise(EOFError) + allow(io).to receive(:readpartial).and_raise(EOFError) + end + + context "on non-Windows system" do + before { allow(Vagrant::Util::Platform).to receive(:windows?). + and_return(false) } + + it "should use a non-blocking read" do + expect(io).to receive(:read_nonblock).and_return("") + described_class.read_until_block(io) + end + + it "should receive data until breakable event" do + expect(io).to receive(:read_nonblock).and_return("one") + expect(io).to receive(:read_nonblock).and_return("two") + expect(io).to receive(:read_nonblock).and_return("three") + data = described_class.read_until_block(io) + expect(data).to eq("onetwothree") + end + + context "with breakable errors" do + [EOFError, Errno::EAGAIN, IO::EAGAINWaitReadable, IO::EINPROGRESSWaitReadable, IO::EWOULDBLOCKWaitReadable].each do |err_class| + it "should break without error on #{err_class}" do + expect(io).to receive(:read_nonblock).and_raise(err_class) + expect(described_class.read_until_block(io)).to be_empty + end + end + end + + context "with non-breakable errors" do + it "should raise the error" do + expect(io).to receive(:read_nonblock).and_raise(StandardError) + expect { described_class.read_until_block(io) }.to raise_error(StandardError) + end + end + end + + context "on Windows system" do + before do + allow(Vagrant::Util::Platform).to receive(:windows?). + and_return(true) + allow(IO).to receive(:select).with([io], any_args). + and_return([io]) + allow(io).to receive(:empty?).and_return(false) + end + + it "should use select" do + expect(IO).to receive(:select).with([io], any_args) + described_class.read_until_block(io) + end + + it "should receive data until breakable event" do + expect(io).to receive(:readpartial).and_return("one") + expect(io).to receive(:readpartial).and_return("two") + expect(io).to receive(:readpartial).and_return("three") + data = described_class.read_until_block(io) + expect(data).to eq("onetwothree") + end + + context "with breakable errors" do + [EOFError, Errno::EAGAIN, IO::EAGAINWaitReadable, IO::EINPROGRESSWaitReadable, + IO::EWOULDBLOCKWaitReadable].each do |err_class| + it "should break without error on #{err_class}" do + expect(io).to receive(:readpartial).and_raise(err_class) + expect(described_class.read_until_block(io)).to be_empty + end + end + end + + context "with non-breakable errors" do + it "should raise the error" do + expect(io).to receive(:readpartial).and_raise(StandardError) + expect { described_class.read_until_block(io) }.to raise_error(StandardError) + end + end + + context "encoding" do + let(:output) { "output".force_encoding("ASCII-8BIT") } + + before { expect(io).to receive(:readpartial).and_return(output) } + + it "should encode output to UTF-8" do + expect(described_class.read_until_block(io).encoding.name).to eq("UTF-8") + end + + context "when output includes characters with undefined conversion" do + let(:output) { "output\xFF".force_encoding("ASCII-8BIT") } + + before { expect(Encoding).to receive(:default_external). + and_return(Encoding.find("ASCII-8BIT")) } + + it "should return data with invalid characters replaced" do + expect(described_class.read_until_block(io)).to include("�") + end + end + end + end + end +end