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
Fix accessing HTTPS sites with an IPv4 address #494
Conversation
src/hackney_ssl.erl
Outdated
@@ -49,6 +49,13 @@ connect(Host, Port, Opts) -> | |||
|
|||
connect(Host, Port, Opts, Timeout) when is_list(Host), is_integer(Port), | |||
(Timeout =:= infinity orelse is_integer(Timeout)) -> | |||
Host1 = try |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be better to use inet:parse_address/1
there. Can you update your PR to use it? (sorry for the late answer)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, no problem. Just to be sure, you mean something like this, right?
f(Host) ->
Host1 = case inet:parse_address(Host) of
{ok, Address} -> Address;
{error, _} -> Host
end,
Host1.
Would it be better if I moved this to a separate function, or should it remain inline in connect/4
?
I will probably update the PR when I get home in the evening (can't really code right now, family&stuff...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe in a separate function to make it more readable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@piotrklibert would be cool if you can send your commit today to be part of the release. Let me know :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think ther eis still an issue with that PR. We should probably check if we can have an ipv6 address. Thoughts?
Yes, it seems to be broken still for the IPv6 addresses (testing with Google's |
Scratch the above, it now kind of works, from what I understand as long as the hackney/src/hackney_http_connect.erl Lines 27 to 37 in 6ef0f9b
service_name_indication is not added to options). Or it can be something else entirely - when I test with rebar3 shell it works:
But it does not when called from Elixir via the HTTPoison library in my project, returning an error:
It's kind of hard to debug the latter because there are at least two layers above Hackney that could (and probably did) screw this up (HTTPoison which wraps Hackney and Mix which wraps rebar) and also the traceback info is swallowed. It's also not the highest priority for me, as I have no machines to monitor with IPv6 addresses. Anyway, what I can confirm is that URLs containing both IPv4 and IPv6 addresses seem to work fine in a |
Precisely the same issue here, I keep getting back an Using OTP 20.1. |
And manually applying this patch did indeed fix it. :-) Any chance of a release to at least get IPv4 working? |
@OvermindDL1 thanks, sorry for the delay to apply it. |
Awesome thanks! Any chance on a hex.pm release so I don't have to depend on a git branch anymore? ^.^ |
Hi, I was writing an internal monitoring tool and found that I need to access URLs in the form of
https://123.123.123.123/something/
- that is, URLs with HTTPS protocol and numeric IP in the host part. Unfortunately, when I tried issuing such requests I discovered that Hackney doesn't allow this case. (Actually, I was writing in Elixir with HTTPoison, which in turn uses Hackney.)After some time debugging I discovered that Erlang's ssl application supports requests to IP addresses only if they are presented as four-tuples of ints, not as strings.
I created a quick fix for this which works for me - just before the
ssl:connect
I split the Host string on dots and check if they can be interpreted as an IP address. I'm not sure if it's any good, it's fine if it's not, but then I think it would be worth it to at least state in the README that HTTPS+IP doesn't work.