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