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

connection: Handle IPv6 address String on #proxy_from_env #1252

Conversation

cosmo0920
Copy link
Contributor

Description

The following error occurs with current master:

/Users/cosmo/.rbenv/versions/3.0.0/lib/ruby/3.0.0/uri/rfc3986_parser.rb:67:in `split': bad URI(is not URI?): "http://::1" (URI::InvalidURIError)
	from /Users/cosmo/.rbenv/versions/3.0.0/lib/ruby/3.0.0/uri/rfc3986_parser.rb:72:in `parse'
	from /Users/cosmo/.rbenv/versions/3.0.0/lib/ruby/3.0.0/uri/common.rb:171:in `parse'
	from /Users/cosmo/GitHub/faraday/lib/faraday/connection.rb:580:in `proxy_from_env'
	from /Users/cosmo/GitHub/faraday/lib/faraday/connection.rb:100:in `initialize_proxy'
	from /Users/cosmo/GitHub/faraday/lib/faraday/connection.rb:87:in `initialize'
	from /Users/cosmo/GitHub/faraday/lib/faraday.rb:103:in `new'
	from /Users/cosmo/GitHub/faraday/lib/faraday.rb:103:in `new'
	from (irb):2:in `<main>'
	from /Users/cosmo/GitHub/fluent-plugin-elasticsearch/vendor/bundle/ruby/3.0.0/gems/irb-1.3.4/exe/irb:11:in `<top (required)>'
	from /Users/cosmo/GitHub/fluent-plugin-elasticsearch/vendor/bundle/ruby/3.0.0/bin/irb:23:in `load'
	from /Users/cosmo/GitHub/fluent-plugin-elasticsearch/vendor/bundle/ruby/3.0.0/bin/irb:23:in `<top (required)>'
	from /Users/cosmo/.rbenv/versions/3.0.0/lib/ruby/3.0.0/bundler/cli/exec.rb:63:in `load'
	from /Users/cosmo/.rbenv/versions/3.0.0/lib/ruby/3.0.0/bundler/cli/exec.rb:63:in `kernel_load'
	from /Users/cosmo/.rbenv/versions/3.0.0/lib/ruby/3.0.0/bundler/cli/exec.rb:28:in `run'
	from /Users/cosmo/.rbenv/versions/3.0.0/lib/ruby/3.0.0/bundler/cli.rb:497:in `exec'
	from /Users/cosmo/.rbenv/versions/3.0.0/lib/ruby/3.0.0/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
	... 10 levels...

This is because, in IPv6 world, URI::XXX#host and URI::XXX#hostname
should be different:

irb> uri = URI.parse("http://[::1]:9200")
irb> uri.host
=> "[::1]"
irb> uri.hostname
=> "::1"

This difference causes the following invalid URI:

When using uri.host case,

irb> "#{uri.scheme}://#{uri.host}"
=> "http://[::1]"
irb> URI.parse("#{uri.scheme}://#{uri.host}")
=> #<URI::HTTP http://[::1]>

we got succeeded to parse created URI with URI.parse.

When using uri.hostname case,

irb> "#{uri.scheme}://#{uri.hostname}"
=> "http://::1"
irb> > URI.parse("#{uri.scheme}://#{uri.hostname}")
/Users/cosmo/.rbenv/versions/3.0.0/lib/ruby/3.0.0/uri/rfc3986_parser.rb:67:in `split': bad URI(is not URI?): "http://::1" (URI::InvalidURIError)
 <snip>

we got failing to parse created URI with URI.parse.

This problematic behavior breaks our use case which passes URL as IPv6
address String.

Signed-off-by: Hiroshi Hatake cosmo0920.oucc@gmail.com

Todos

List any remaining work that needs to be done, i.e:

  • Tests

Additional Notes

Optional section

Otherwise, the following error occurs:

```log
/Users/cosmo/.rbenv/versions/3.0.0/lib/ruby/3.0.0/uri/rfc3986_parser.rb:67:in `split': bad URI(is not URI?): "http://::1" (URI::InvalidURIError)
	from /Users/cosmo/.rbenv/versions/3.0.0/lib/ruby/3.0.0/uri/rfc3986_parser.rb:72:in `parse'
	from /Users/cosmo/.rbenv/versions/3.0.0/lib/ruby/3.0.0/uri/common.rb:171:in `parse'
	from /Users/cosmo/GitHub/faraday/lib/faraday/connection.rb:580:in `proxy_from_env'
	from /Users/cosmo/GitHub/faraday/lib/faraday/connection.rb:100:in `initialize_proxy'
	from /Users/cosmo/GitHub/faraday/lib/faraday/connection.rb:87:in `initialize'
	from /Users/cosmo/GitHub/faraday/lib/faraday.rb:103:in `new'
	from /Users/cosmo/GitHub/faraday/lib/faraday.rb:103:in `new'
	from (irb):2:in `<main>'
	from /Users/cosmo/GitHub/fluent-plugin-elasticsearch/vendor/bundle/ruby/3.0.0/gems/irb-1.3.4/exe/irb:11:in `<top (required)>'
	from /Users/cosmo/GitHub/fluent-plugin-elasticsearch/vendor/bundle/ruby/3.0.0/bin/irb:23:in `load'
	from /Users/cosmo/GitHub/fluent-plugin-elasticsearch/vendor/bundle/ruby/3.0.0/bin/irb:23:in `<top (required)>'
	from /Users/cosmo/.rbenv/versions/3.0.0/lib/ruby/3.0.0/bundler/cli/exec.rb:63:in `load'
	from /Users/cosmo/.rbenv/versions/3.0.0/lib/ruby/3.0.0/bundler/cli/exec.rb:63:in `kernel_load'
	from /Users/cosmo/.rbenv/versions/3.0.0/lib/ruby/3.0.0/bundler/cli/exec.rb:28:in `run'
	from /Users/cosmo/.rbenv/versions/3.0.0/lib/ruby/3.0.0/bundler/cli.rb:497:in `exec'
	from /Users/cosmo/.rbenv/versions/3.0.0/lib/ruby/3.0.0/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
	... 10 levels...
```

This is because, in IPv6 world, URI::XXX#host and URI::XXX#hostname
should be different:

```irb
irb> uri = URI.parse("http://[::1]:9200")
irb> uri.host
=> "[::1]"
irb> uri.hostname
=> "::1"
```

This difference causes the following invalid URI:

When using `uri.host` case,

```irb
irb> "#{uri.scheme}://#{uri.host}"
=> "http://[::1]"
irb> URI.parse("#{uri.scheme}://#{uri.host}")
=> #<URI::HTTP http://[::1]>
```

we got succeeded to parse created URI with `URI.parse`.

When using `uri.hostname` case,

```irb
irb> "#{uri.scheme}://#{uri.hostname}"
=> "http://::1"
irb> > URI.parse("#{uri.scheme}://#{uri.hostname}")
/Users/cosmo/.rbenv/versions/3.0.0/lib/ruby/3.0.0/uri/rfc3986_parser.rb:67:in `split': bad URI(is not URI?): "http://::1" (URI::InvalidURIError)
 <snip>
```

we got failing to parse created URI with `URI.parse`.

This problematic behavior breaks our use case which passes URL as IPv6
address String.

Signed-off-by: Hiroshi Hatake <cosmo0920.oucc@gmail.com>
@cosmo0920
Copy link
Contributor Author

cosmo0920 commented Mar 22, 2021

@iMacTia Could you kindly take a look? This behavior blocks our requested use case on uken/fluent-plugin-elasticsearch#876. Our use case is: uken/fluent-plugin-elasticsearch#877

Copy link
Member

@iMacTia iMacTia left a comment

Choose a reason for hiding this comment

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

I was surprised at first because after #621 and #714 I thought #hostname was always a better choice, but my understanding is that in this case we're not calling #host to make an API call (which would probably fail if we did), but to build a URI for URI.parse.

The fact that tests are all green as well seem to show that this change is not breaking any existing functionality either, as expected given that the only difference between #host and #hostname should affect IPv6 only

@iMacTia iMacTia merged commit 850155e into lostisland:master Mar 22, 2021
@cosmo0920 cosmo0920 deleted the handle-ipv6-address-string-on-proxy_from_env branch March 22, 2021 11:35
@cosmo0920
Copy link
Contributor Author

Thanks!

@cosmo0920
Copy link
Contributor Author

cosmo0920 commented Apr 16, 2021

Hi, is there any ETA to release a stable version of faraday gem?
fluent-plugin-elasticsearch users still complain IPv6 support is broken on current released faraday gem.
Anyway, thanks for providing tremendous gem! 😄

@iMacTia
Copy link
Member

iMacTia commented Apr 16, 2021

Sorry about this @cosmo0920, you're totally right!
We fell behind with releases while being focused on moving adapters out 😅
I've now released v1.3.1 that contains yours and a few small fixes, together with v1.4.0 which contains bigger changes 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants