Request for Review [14] JDK-8226976, SessionTimeOutTests uses == operator for String value check

Xuelei Fan Xuelei.Fan at Oracle.Com
Tue Jul 2 05:35:02 UTC 2019



> On Jul 1, 2019, at 10:28 PM, Jaikiran Pai <jai.forums2013 at gmail.com> wrote:
> 
> Hi Xuelei, Bernd,
> 
>> On 01/07/19 11:31 PM, Xuelei Fan wrote:
>>> On 7/1/2019 10:51 AM, Bernd Eckenfels wrote:
>>> Also the `is*` prefix would point to a boolean, that’s maybe a
>>> cleaner data type than a case sensitive string?
>>> 
>> I agreed.  The "isTimedout" is also used to for strings other than
>> "YES"/"No".  I don't think it is a good code convention.
>>     isTimedout = "Invalidated before timeout";
>> 
>> As Jaikiran had taken care of the issue in a previous thread:
>> 
>> https://mail.openjdk.java.net/pipermail/security-dev/2019-June/020307.html
>> 
>> 
>> I will leave it to Jaikiran if we want to make more improvement in
>> that code review thread.
> 
> I can take up the cleanup of this testcase as a separate task. I will
> need to understand the current semantics[1] of some of the APIs being
> used in this test and how it impacts this test.
> 
It makes sense to me.  Thanks for taking care of the update.

Xuelei

> [1]
> http://mail.openjdk.java.net/pipermail/security-dev/2019-July/020308.html
> 
> -Jaikiran
> 
> 
>> 
>> Thanks,
>> Xuelei
>> 
>>> 
>>> -- 
>>> http://bernd.eckenfels.net
>>> ------------------------------------------------------------------------
>>> *Von:* security-dev <security-dev-bounces at openjdk.java.net> im
>>> Auftrag von Xuelei Fan <xuelei.fan at oracle.com>
>>> *Gesendet:* Montag, Juli 1, 2019 6:44 PM
>>> *An:* security-dev at openjdk.java.net
>>> *Betreff:* Request for Review [14] JDK-8226976, SessionTimeOutTests
>>> uses == operator for String value check
>>> Hi,
>>> 
>>> In the following test case, "==" is used to compare two strings. As is
>>> not a comment coding convention. I would like to use "equals()" method
>>> instead.
>>> 
>>> Thanks,
>>> Xuelei
>>> 
>>> 
>>> $ hg diff test/jdk/javax/net/ssl/SSLSession/SessionTimeOutTests.java
>>> diff -r 73f1c84ca264
>>> test/jdk/javax/net/ssl/SSLSession/SessionTimeOutTests.java
>>> --- a/test/jdk/javax/net/ssl/SSLSession/SessionTimeOutTests.java
>>> Thu Jun 27 22:03:19 2019 +0200
>>> +++ b/test/jdk/javax/net/ssl/SSLSession/SessionTimeOutTests.java
>>> Mon Jul 01 09:29:23 2019 -0700
>>> @@ -283,7 +283,7 @@
>>> }
>>> System.out.print(sess + " " + lifetime);
>>> if (((timeout == 0) || (lifetime < timeout)) &&
>>> - (isTimedout == "YES")) {
>>> + isTimedout.equals("YES")) {
>>> isTimedout = "Invalidated before timeout";
>>> }



More information about the security-dev mailing list