From f1ed7ce562128c05d6014cb579bcfb0e624a84d3 Mon Sep 17 00:00:00 2001 From: Nobu Funaki Date: Wed, 5 Mar 2014 18:59:47 -0800 Subject: [PATCH 1/3] fix #84 check if the filename is String. filename could be Tempfile, for instance. --- lib/roo/base.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/roo/base.rb b/lib/roo/base.rb index ab0c48f9..a7531ece 100644 --- a/lib/roo/base.rb +++ b/lib/roo/base.rb @@ -623,7 +623,7 @@ def normalize(row,col) end def uri?(filename) - filename.start_with?("http://", "https://") + filename.is_a? String and filename.start_with?("http://", "https://") end def download_uri(uri, tmpdir) From bf131a159da5d89a550c6d125ab983497c509a6a Mon Sep 17 00:00:00 2001 From: Nobu Funaki Date: Thu, 13 Mar 2014 12:30:25 -0700 Subject: [PATCH 2/3] check if filename is String in excel.rb --- lib/roo/excel.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/roo/excel.rb b/lib/roo/excel.rb index 225dae01..87834034 100644 --- a/lib/roo/excel.rb +++ b/lib/roo/excel.rb @@ -30,7 +30,7 @@ def initialize(filename, options = {}, deprecated_file_warning = :error) file_type_check(filename,'.xls','an Excel', file_warning, packed) make_tmpdir do |tmpdir| filename = download_uri(filename, tmpdir) if uri?(filename) - filename = open_from_stream(filename[7..-1], tmpdir) if filename[0,7] == "stream:" + filename = open_from_stream(filename[7..-1], tmpdir) if String === filename && filename[0,7] == "stream:" filename = unzip(filename, tmpdir) if packed == :zip @filename = filename From 1268beedf86e552b2d368d60c97c3f8542411248 Mon Sep 17 00:00:00 2001 From: Nobu Funaki Date: Sun, 3 Aug 2014 23:07:42 -0700 Subject: [PATCH 3/3] change some code based on @simonoff's suggestion, add a test #84 --- lib/roo/base.rb | 6 +++++- lib/roo/excel.rb | 2 +- test/test_generic_spreadsheet.rb | 15 +++++++++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/lib/roo/base.rb b/lib/roo/base.rb index eb76de1c..ef498b20 100644 --- a/lib/roo/base.rb +++ b/lib/roo/base.rb @@ -659,7 +659,11 @@ def normalize(row,col) end def uri?(filename) - filename.is_a? String and filename.start_with?("http://", "https://") + begin + filename.start_with?("http://", "https://") + rescue + false + end end def download_uri(uri, tmpdir) diff --git a/lib/roo/excel.rb b/lib/roo/excel.rb index 87834034..472f4395 100644 --- a/lib/roo/excel.rb +++ b/lib/roo/excel.rb @@ -30,7 +30,7 @@ def initialize(filename, options = {}, deprecated_file_warning = :error) file_type_check(filename,'.xls','an Excel', file_warning, packed) make_tmpdir do |tmpdir| filename = download_uri(filename, tmpdir) if uri?(filename) - filename = open_from_stream(filename[7..-1], tmpdir) if String === filename && filename[0,7] == "stream:" + filename = open_from_stream(filename[7..-1], tmpdir) if filename.is_a?(::String) && filename[0,7] == "stream:" filename = unzip(filename, tmpdir) if packed == :zip @filename = filename diff --git a/test/test_generic_spreadsheet.rb b/test/test_generic_spreadsheet.rb index 571415ec..7f402a82 100644 --- a/test/test_generic_spreadsheet.rb +++ b/test/test_generic_spreadsheet.rb @@ -102,6 +102,21 @@ def sheets end end + context 'private method Roo::Base.uri?(filename)' do + should "return true when passed a filename starts with http(s)://" do + assert_equal true, @oo.send(:uri?, 'http://example.com/') + assert_equal true, @oo.send(:uri?, 'https://example.com/') + end + + should "return false when passed a filename which does not start with http(s)://" do + assert_equal false, @oo.send(:uri?, 'example.com') + end + + should "return false when passed non-String object such as Tempfile" do + assert_equal false, @oo.send(:uri?, Tempfile.new('test')) + end + end + def test_setting_invalid_type_does_not_update_cell @oo.set(1,1,1) assert_raise(ArgumentError){@oo.set(1,1, :invalid_type)}