HTTPCLIENT-2353: Fix IDN hostname mismatch by normalizing host and identity with IDN #607
Conversation
arturobernalg
force-pushed
the
HTTPCLIENT-2353
branch
from
4ef1ca2 to
83bdee3
Compare
| @@ -228,8 +229,18 @@ private static boolean matchIdentity(final String host, final String identity, | |||
| final PublicSuffixMatcher publicSuffixMatcher, | |||
| final DomainType domainType, | |||
| final boolean strict) { | |||
|
|
|||
| final String punycodeHost; | |||
There was a problem hiding this comment.
@arturobernalg Could we do it the other way around and convert identity from punycode to Unicode instead?
There was a problem hiding this comment.
@ok2c you mean Instead of converting both strings to punycode, bring both into the same Unicode form—i.e., use IDN.toUnicode(...) rather than toASCII(...) ???
There was a problem hiding this comment.
@ok2c you mean Instead of converting both strings to punycode, bring both into the same Unicode form—i.e., use IDN.toUnicode(...) rather than toASCII(...) ???
@arturobernalg That is exactly what I mean. This way one would not need to normalize the hostname as it is has already been normalized to Unicode at the construction time.
There was a problem hiding this comment.
@ok2c changed. please take another look
arturobernalg
force-pushed
the
HTTPCLIENT-2353
branch
from
078a60c to
accc382
Compare
| @@ -228,36 +229,44 @@ private static boolean matchIdentity(final String host, final String identity, | |||
| final PublicSuffixMatcher publicSuffixMatcher, | |||
| final DomainType domainType, | |||
| final boolean strict) { | |||
|
|
|||
| final String unicodeIdentity; | |||
There was a problem hiding this comment.
unicodeIdentity -> normalizedIdentity would be a better name?
|
@arturobernalg Please note we still have a problem with punycode encoded hostname resolution. We might address it in a separate PR though. |
arturobernalg
force-pushed
the
HTTPCLIENT-2353
branch
from
2c31053 to
758ad8d
Compare
@ok2c I have fixed the test within this PR. |
| final String normalizedHost; | ||
| final String normalizedIdentity; | ||
| try { | ||
| normalizedHost = IDN.toUnicode(host); |
There was a problem hiding this comment.
@arturobernalg This is unnecessary. The hostname has already been normalized. Please see Host and HttpHost classes
There was a problem hiding this comment.
@ok2c
you right. changed. Please do another pass
…th IDN.toUnicode before comparison so that Unicode and punycode forms match correctly.
arturobernalg
force-pushed
the
HTTPCLIENT-2353
branch
from
6f20228 to
0f40601
Compare
|
@arturobernalg Please also cherry-pick this commit to |
@ok2c on it |
Cherry-picked to |
Normalize Unicode and punycode strings before comparing host and certificate identities. This approach ensures domains like поиск-слов.рф properly match their punycode equivalents.