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

Xuelei Fan xuelei.fan at oracle.com
Mon Jul 1 18:01:11 UTC 2019


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.

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