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 1 commit
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
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 and (u.host.nil? or u.host.empty?) and u.path.empty?
Copy link
Contributor

Choose a reason for hiding this comment

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

Our standard is to use the || and &&, instead of and/or due to the precedence differences.

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:[email protected]: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