HACK URI::Generic.build to allow a IPv6 host value.

[jrafanie]

It's a temporary workaround until ruby/ruby#765 is
readily available. As of this writing, ruby 2.0.0p643 and ruby 2.2.0 have this fixed.
Ruby 2.1.5, and ruby 2.0.0p598 and lower don't have this fixed and NEED this hack.
Ruby 2.0: https://bugs.ruby-lang.org/issues/10873 (backported and in p643)
Ruby 2.1: https://bugs.ruby-lang.org/issues/10875 (backported and not released yet)
Ruby 2.2: https://bugs.ruby-lang.org/issues/10874 (backported and in 2.2.0)

Workaround before this hack:

 # Don't pass :host => "::1" to .build as it throws URI::InvalidComponentError
 uri = URI::HTTPS.build(:path => "/sdk")
 uri.hostname = "::1"
 uri.to_s # => "https://[::1]/sdk"

After this hack the caller doesn't need to know to use [::1] or hostname= method:

 uri = URI::HTTPS.build(:host => "::1", :path => "/sdk")
 uri.to_s # => "https://[::1]/sdk"

This enables us to use the fixed URI::Generic.build API now and clean up 60c8f7f, 01cb19a, 5cf1f30, e806c46 and any other place we may do this in the future.

cc @matthewd @tenderlove @Fryguy @blomquisg @abonas

[Fryguy]

You shouldn't need this. require 'util/miq-uri' should be sufficient for this spec

[Fryguy]

Looks good to me. Why is this WIP?

[jrafanie]

Ok, so it's not technically WIP but I'm not really pleased with it though. I'd like to have this hack yell at us to remove it when it's not needed anymore although I don't know how to do that. Also, it feels like a lot of code to monkey patch one method. The alternative is to re-implement the whole of URI::Generic initialize in a monkey patch and hope that the code within that method doesn't change.

I was hoping this code would give someone an idea on a better implementation.

[Fryguy]

Can you just throw an else on the outer if and just do a ruby warning or puts something?

[jrafanie]

It's when we move to a ruby 2.0.0 patchlevel that's guaranteed to be >= 643 (rpm based ruby is currently 353 with backports of security fixes) or ruby 2.1.6/2.2.0+ on all appliances/dev machines. I'm afraid it's going to be uselessly noisy if I do it in an outside if/else.

[tenderlove]

Rather than doing explicit version checking, we could do:

def supports_ipv6_on_build?
  URI::HTTPS.build(:host => "::1", :path => "/sdk")
  true
rescue URI::InvalidComponentError
  false
end

if !supports_ipv6_on_build?
  # etc

Then we don't have to update this when the 2.1 backport lands.

[miq-bot]

Checked commit jrafanie@aa33f55 with rubocop 0.27.1
2 files checked, 7 offenses detected

lib/spec/util/extensions/miq-uri_spec.rb

• 🔶 - Line 1, Col 1 - Style/FileName - Use snake_case for source file names.
• 🔶 - Line 8, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• 🔶 - Line 10, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• 🔶 - Line 11, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.

lib/util/extensions/miq-uri.rb

• 🔶 - Line 1, Col 1 - Style/FileName - Use snake_case for source file names.
• 🔶 - Line 3, Col 3 - Style/CommentAnnotation - Annotation keywords should be all upper case, followed by a colon and a space, then a note describing the problem.
• 🔶 - Line 42, Col 7 - Style/EmptyLinesAroundAccessModifier - Keep a blank line before and after private.

[jrafanie]

Thanks @Fryguy, fixed here.

[Fryguy]

@jrafanie Did you still want to add the "yelling at us to remove the patch" code?

[jrafanie]

@Fryguy we can't easily do it since upstream will constantly yell while downstream wouldn't. I'll have to remember somehow.

[jrafanie]

BUMP @Fryguy @tenderlove please review/merge... to clarify... upstream appliances now use ruby 2.0.0p643(a fixed ruby), some developers may also be using a ruby that would have this fixed, meanwhile, downstream appliances most likely won't be at a "fixed" ruby version for months, therefore yelling at developers would be useless because we still need this hack... until we don't.