Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Addressable::URI, allowing IPv6 URLs #339

Closed
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't a development need - it should only be in the gemspec. Gemfile pulls in all gemspec requirements, so this is redundant.


group :test do
gem 'bundler', '~> 1.11'
gem 'minitest', '~> 5.8'
Expand Down
30 changes: 15 additions & 15 deletions lib/train.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 && (u.host.nil? || u.host.empty?) && 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

Expand Down
4 changes: 2 additions & 2 deletions lib/train/transports/winrm_connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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

Expand Down
22 changes: 17 additions & 5 deletions test/unit/train_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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'
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions train.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ Gem::Specification.new do |spec|

spec.required_ruby_version = '>= 2.0'

spec.add_dependency 'addressable', '~> 2.5'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pin here on 2.5 is going to conflict with Chef which is pinned to 2.4 due to another gem. Does this need to be 2.5 or would 2.4 work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, addressable 2.4 is from end of 2015. Maybe I was required to bump the gem while working on a project for Ruby 2.5 support, but maybe I remember wrong.

For sure a lot of fixes happened since 2.4. I don't know whether the change to addressable gem is even smart when not used with 2.5

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're currently waiting on Travis to release this gem so we can pull in the latest addressable to our overall stack: travis-ci/gh#33

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'
Expand Down