[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: 8251155: HostIdentifier fails to canonicalize hostnames starting with digits(Internet mail)
From:       jiefu(傅杰) <jiefu () tencent ! com>
Date:       2020-08-25 12:12:13
Message-ID: BA7F89ED-91BC-4402-9BD5-A82BCCF80093 () tencent ! com
[Download RAW message or body]

Thanks Claes for your review.
Pushed.

Best regards,
Jie

On 2020/8/25, 7:26 PM, "Claes Redestad" <claes.redestad@oracle.com> wrote:

    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.spitsyn@oracle.com <serguei.spitsyn@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(傅杰)" <jiefu@tencent.com>
    >> *Date: *Wednesday, August 19, 2020 at 4:05 PM
    >> *To: *"serguei.spitsyn@oracle.com" <serguei.spitsyn@oracle.com>, 
    >> "serviceability-dev@openjdk.java.net" 
    >> <serviceability-dev@openjdk.java.net>, Claes Redestad 
    >> <claes.redestad@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.spitsyn@oracle.com <serguei.spitsyn@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
    >>
    > 
    
    


[prev in list] [next in list] [prev in thread] [next in thread]