Hi Jie,
fix looks good to me!
/Claes
On 2020-08-25 04:12, jiefu(傅杰) wrote:
Thanks Serguei for your review.
Claes, are you okay with this change:
http://cr.openjdk.java.net/~jiefu/8251155/webrev.00/
Thanks.
Best regards,
Jie
------------------------------------------------------------------------
*From:* serguei.spit...@oracle.com <serguei.spit...@oracle.com>
*Sent:* Tuesday, August 25, 2020 8:38 AM
*To:* jiefu(傅杰); serviceability-dev@openjdk.java.net; Claes Redestad
*Subject:* Re: 8251155: HostIdentifier fails to canonicalize hostnames
starting with digits(Internet mail)
Hi Jie,
I'm okay with the fix.
Thanks,
Serguei
On 8/24/20 09:21, jiefu(傅杰) wrote:
Hi Serguei and Claes,
I forget to mention that you can also verify this fix using the
following tests:
----------------------------------------------------------
test/jdk/sun/tools/jstatd/TestJstatdExternalRegistry.java
test/jdk/sun/tools/jstatd/TestJstatdPort.java
test/jdk/sun/tools/jstatd/TestJstatdPortAndServer.java
test/jdk/sun/tools/jstatd/TestJstatdRmiPort.java
----------------------------------------------------------
Without the patch, All of them will fail if the hostname starting from
digits.
We've found that it seems very common that the hostname will start
with digits in dockers.
So it would be better to fix it.
What do you think?
Thanks.
Best regards,
Jie
*From: *"jiefu(傅杰)" <ji...@tencent.com>
*Date: *Wednesday, August 19, 2020 at 4:05 PM
*To: *"serguei.spit...@oracle.com" <serguei.spit...@oracle.com>,
"serviceability-dev@openjdk.java.net"
<serviceability-dev@openjdk.java.net>, Claes Redestad
<claes.redes...@oracle.com>
*Subject: *Re: 8251155: HostIdentifier fails to canonicalize hostnames
starting with digits(Internet mail)
Hi Serguei,
Thanks for your review and help.
Please see comments inline.
------------------------------------------------------------------------
*From:*serguei.spit...@oracle.com <serguei.spit...@oracle.com>
*Sent:* Wednesday, August 19, 2020 4:03 AM
*To:* jiefu(傅杰); serviceability-dev@openjdk.java.net; Claes Redestad
*Subject:* Re: 8251155: HostIdentifier fails to canonicalize hostnames
starting with digits(Internet mail)
83 * <ul>
84 * <li>{@code <null>} - transformed into "//localhost"</li>
85 * <li>localhost - transformed into "//localhost"</li>
86 * <li>hostname - transformed into "//hostname"</li>
87 * <li>hostname:port - transformed into "//hostname:port"</li>
88 * <li>proto:hostname - transformed into "proto://hostname"</li>
89 * <li>proto:hostname:port - transformed into
90 * "proto://hostname:port"</li>
91 * <li>proto://hostname:port</li>
92 * </ul>
>> Is it worth to add an example to the list above?
Yes. It's really helpful for the review process. Thanks.
>> I wander if this fix needs a CSR.
I don't think so.
This is just a bug fix which doesn't add/remove/change any feature of
the tools.
The original design has claimed to support hostname and hostname:port
cases.
But it fails to do so when the hostname starts with digits.
It seems to be very common that the hostname will be started with
digits in dockers.
So I think it's worth to fix this bug.
>> How did you check this fix does not introduce any regressions?
In fact, Claes had helped me to answer this question here:
https://mail.openjdk.java.net/pipermail/serviceability-dev/2020-August/032691.html.
Also, I've tested this patch on Linux/x64 with
tier1 ~ tier3 (no regression).
Thanks a lot.
Best regards,
Jie
