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

Jaikiran Pai jai.forums2013 at gmail.com
Tue Jul 2 05:28:30 UTC 2019


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.

[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