-
Notifications
You must be signed in to change notification settings - Fork 3k
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
ssl: Handle ip-address as string correctly #7974
ssl: Handle ip-address as string correctly #7974
Conversation
CT Test Results 2 files 66 suites 47m 55s ⏱️ Results for commit 2da9c1e. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
d4b50f6
to
40607cd
Compare
@dgud @IngelaAndin shouldn't some type specs be updated also to describe this change? |
Hostname already has the type string() which is wide enough to allow also "127.0.0.1" etc, this just a special handling of such strings that mostly was already possible. I guess the question is should we really use inet:hostname() instead of having ssl:hostname() ? This would allow the hostname also to be an atom. @dgud @u3s @RaimoNiskanen @bmk ? |
Well:
So, not quite identical. |
My point was that when looking at the specs currently : Lines 163 to 165 in 40607cd
it is not obvious (at all) that an IP address can be passed and accepted as a string since 1/ an IP address is not a hostname and 2/ the If not the specs, maybe more a potentail documentation improvement? |
Well, maybe it is a documentation improvement that is needed, even though we ought to use inet:hostname() and behave the same as gen_tcp, it was surprising to me that "127.0.0.1" etc would be parsed and interpreted as an ip-address. |
@IngelaAndin wrote:
This behaviour is implemented, known and sometimes documented for basic OS resolver API functions such as |
06a9ac1
to
a4badaa
Compare
Allowed |
2da9c1e
to
d348bd7
Compare
If the host was specified as string ip-address, server_name_indication didn't work.
Convert "ip-address" to tuple form in default hostname and also fallback to check ip if dns_id doesn't work, as was done for upgraded sockets previously.
Also while at it cleanup handling when hostname is undefined.
Fixes #7968