From 1c5f4205394497ac6399e80e73d70aa3c829af8b Mon Sep 17 00:00:00 2001 From: mgrobelin Date: Tue, 21 Aug 2018 13:54:09 +0200 Subject: [PATCH 1/3] Replace usage of Ruby's stdlib URI by Addressable::URI "Addressable is a replacement for the URI implementation that is part of Ruby's standard library. It more closely conforms to RFC 3986, RFC 3987, and RFC 6570 (level 4), additionally providing support for IRIs and URI templates.", see https://github.com/sporkmonger/addressable Fixes https://github.com/inspec/train/issues/110 Signed-off-by: Markus Grobelin --- lib/train.rb | 30 ++++++++++++------------ lib/train/transports/winrm_connection.rb | 4 ++-- test/unit/train_test.rb | 22 +++++++++++++---- 3 files changed, 34 insertions(+), 22 deletions(-) diff --git a/lib/train.rb b/lib/train.rb index b04437f3..b9ec329c 100644 --- a/lib/train.rb +++ b/lib/train.rb @@ -7,7 +7,7 @@ require 'train/plugins' require 'train/errors' require 'train/platforms' -require 'uri' +require 'addressable/uri' module Train # Create a new transport instance, with the plugin indicated by the @@ -82,7 +82,7 @@ def self.target_config(config = nil) # rubocop:disable Metrics/AbcSize conf[:path] ||= uri.path conf[:password] ||= if conf[:www_form_encoded_password] && !uri.password.nil? - URI.decode_www_form_component(uri.password) + Addressable::URI.unencode_component(uri.password) else uri.password end @@ -112,24 +112,24 @@ def self.symbolize_keys(map) # Parse a URI. Supports empty URI's with paths, e.g. `mock://` # # @param string [string] URI string, e.g. `schema://domain.com` - # @return [URI::Generic] parsed URI object + # @return [Addressable::URI] parsed URI object def self.parse_uri(string) - URI.parse(string) - rescue URI::InvalidURIError => e + u = Addressable::URI.parse(string) # A use-case we want to catch is parsing empty URIs with a schema # e.g. mock://. To do this, we match it manually and fake the hostname - case string - when %r{^([a-z]+)://$} - string += 'dummy' - when /^([a-z]+):$/ - string += '//dummy' - else - raise Train::UserError, e + if u.scheme and (u.host.nil? or u.host.empty?) and u.path.empty? + case string + when %r{^([a-z]+)://$} + string += 'dummy' + when /^([a-z]+):$/ + string += '//dummy' + end + u = Addressable::URI.parse(string) + u.host = nil end - - u = URI.parse(string) - u.host = nil u + rescue Addressable::URI::InvalidURIError => e + raise Train::UserError, e end private_class_method :parse_uri diff --git a/lib/train/transports/winrm_connection.rb b/lib/train/transports/winrm_connection.rb index 7f9e1291..a4c1cfb1 100644 --- a/lib/train/transports/winrm_connection.rb +++ b/lib/train/transports/winrm_connection.rb @@ -114,7 +114,7 @@ def run_command_via_connection(command) # Mac system # @api private def rdp_doc(opts = {}) - host = URI.parse(options[:endpoint]).host + host = Addressable::URI.parse(options[:endpoint]).host content = [ "full address:s:#{host}:#{@rdp_port}", 'prompt for credentials:i:1', @@ -145,7 +145,7 @@ def file_manager def login_command_for_linux args = %W( -u #{options[:user]} ) args += %W( -p #{options[:pass]} ) if options.key?(:pass) - args += %W( #{URI.parse(options[:endpoint]).host}:#{@rdp_port} ) + args += %W( #{Addressable::URI.parse(options[:endpoint]).host}:#{@rdp_port} ) LoginCommand.new('rdesktop', args) end diff --git a/test/unit/train_test.rb b/test/unit/train_test.rb index a1714d0a..316494fb 100644 --- a/test/unit/train_test.rb +++ b/test/unit/train_test.rb @@ -125,7 +125,7 @@ res[:target].must_equal org[:target] end - it 'always takes ruby sumbols as configuration fields' do + it 'always takes ruby symbols as configuration fields' do org = { 'target' => 'ssh://user:pass@host.com:123/path', 'backend' => rand, @@ -154,7 +154,7 @@ res[:target].must_equal org[:target] end - it 'supports IPv6 URIs' do + it 'supports IPv6 URIs (with brackets)' do org = { target: 'mock://[abc::def]:123' } res = Train.target_config(org) res[:backend].must_equal 'mock' @@ -166,6 +166,18 @@ res[:target].must_equal org[:target] end + it 'supports IPv6 URIs (without brackets)' do + org = { target: 'mock://FEDC:BA98:7654:3210:FEDC:BA98:7654:3210:123' } + res = Train.target_config(org) + res[:backend].must_equal 'mock' + res[:host].must_equal 'FEDC:BA98:7654:3210:FEDC:BA98:7654:3210' + res[:user].must_be_nil + res[:password].must_be_nil + res[:port].must_equal 123 + res[:path].must_be_nil + res[:target].must_equal org[:target] + end + it 'supports empty URIs with schema://' do org = { target: 'mock://' } res = Train.target_config(org) @@ -192,7 +204,7 @@ it 'supports www-form encoded passwords when the option is set' do raw_password = '+!@#$%^&*()_-\';:"\\|/?.>,<][}{=`~' - encoded_password = URI.encode_www_form_component(raw_password) + encoded_password = Addressable::URI.normalize_component(raw_password, Addressable::URI::CharacterClasses::UNRESERVED) org = { target: "mock://username:#{encoded_password}@1.2.3.4:100", www_form_encoded_password: true} res = Train.target_config(org) @@ -216,8 +228,8 @@ res[:target].must_equal org[:target] end - it 'it raises UserError on invalid URIs' do - org = { target: 'mock world' } + it 'it raises UserError on invalid URIs (invalid scheme)' do + org = { target: '123://invalid_scheme.example.com/' } proc { Train.target_config(org) }.must_raise Train::UserError end end From 58109409508e0468f4582c14033ba044428ccf3a Mon Sep 17 00:00:00 2001 From: mgrobelin Date: Fri, 24 Aug 2018 09:51:40 +0200 Subject: [PATCH 2/3] Added addressable gem for better URI handling Signed-off-by: Markus Grobelin --- Gemfile | 2 ++ lib/train.rb | 2 +- train.gemspec | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index 5e8204bf..e1d0d8f6 100644 --- a/Gemfile +++ b/Gemfile @@ -12,6 +12,8 @@ if Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.2.2') gem 'json', '< 2.0' end +gem 'addressable', '~> 2.5' + group :test do gem 'bundler', '~> 1.11' gem 'minitest', '~> 5.8' diff --git a/lib/train.rb b/lib/train.rb index b9ec329c..fb8d7d7c 100644 --- a/lib/train.rb +++ b/lib/train.rb @@ -117,7 +117,7 @@ def self.parse_uri(string) u = Addressable::URI.parse(string) # A use-case we want to catch is parsing empty URIs with a schema # e.g. mock://. To do this, we match it manually and fake the hostname - if u.scheme and (u.host.nil? or u.host.empty?) and u.path.empty? + if u.scheme && (u.host.nil? || u.host.empty?) && u.path.empty? case string when %r{^([a-z]+)://$} string += 'dummy' diff --git a/train.gemspec b/train.gemspec index 481763ad..4af7ed2d 100644 --- a/train.gemspec +++ b/train.gemspec @@ -25,6 +25,7 @@ Gem::Specification.new do |spec| spec.required_ruby_version = '>= 2.0' + spec.add_dependency 'addressable', '~> 2.5' spec.add_dependency 'json', '>= 1.8', '< 3.0' # chef-client < 12.4.1 require mixlib-shellout-2.0.1 spec.add_dependency 'mixlib-shellout', '~> 2.0' From 112d2c0e4f88cbe755c2a97973dfe21c6c05eff2 Mon Sep 17 00:00:00 2001 From: mgrobelin Date: Tue, 19 Mar 2019 14:43:14 +0100 Subject: [PATCH 3/3] fixed linting and removed addressable from Gemfile see https://github.com/inspec/train/pull/339#pullrequestreview-215067182 Signed-off-by: Markus Grobelin --- Gemfile | 2 -- lib/train.rb | 8 ++++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/Gemfile b/Gemfile index e1d0d8f6..5e8204bf 100644 --- a/Gemfile +++ b/Gemfile @@ -12,8 +12,6 @@ if Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.2.2') gem 'json', '< 2.0' end -gem 'addressable', '~> 2.5' - group :test do gem 'bundler', '~> 1.11' gem 'minitest', '~> 5.8' diff --git a/lib/train.rb b/lib/train.rb index fb8d7d7c..3371a180 100644 --- a/lib/train.rb +++ b/lib/train.rb @@ -119,10 +119,10 @@ def self.parse_uri(string) # e.g. mock://. To do this, we match it manually and fake the hostname if u.scheme && (u.host.nil? || u.host.empty?) && u.path.empty? case string - when %r{^([a-z]+)://$} - string += 'dummy' - when /^([a-z]+):$/ - string += '//dummy' + when %r{^([a-z]+)://$} + string += 'dummy' + when /^([a-z]+):$/ + string += '//dummy' end u = Addressable::URI.parse(string) u.host = nil