Hi John,
Could you please upload the report to
http://cr.openjdk.java.net/~jjiang/8186057 ?
One question about Compatibility.caseStatus(). What's the case when you
expect a timeout and no client output? Should it always result to a test
case failure?
Otherwise, looks fine to me.
A couple of minor comments which you may want to address:
1. Cert.getCert(): it would be nice to add a comment which explains
what's returned in the array. It might be good to add an "else" block
and throw an exception there.
2. Client.java, lines 83-84: it's not necessary to print a stack trace
if you put the original exception to runtime exception. The same with
ProcessUtils.
3. JdkInfo: the fields of this class should be final since you don't
modify them then. If compilation fails because of complex logic in
constructor, then it may be a sign that the logic should be simplified.
Or, you can put the logic to a static factory method.
4. "sequence" field of JdkRelease looks redundant. A enum already
defines the order. The same with Protocol.
5.
On 11/14/2017 09:16 AM, sha.ji...@oracle.com wrote:
Hi Artem,
Please find the new webrev [1].
This test has been refactored significantly. It supports more cipher
suites, and case combinations among the TLS communication parameters.
For more details, please look through README [2].
[1] http://cr.openjdk.java.net/~jjiang/8186057/webrev.04/
[2]
http://cr.openjdk.java.net/~jjiang/8186057/webrev.04/test/jdk/javax/net/ssl/compatibility/README.html
Best regards,
John Jiang
On 07/09/2017 18:47, sha.ji...@oracle.com wrote:
Hi Artem,
On 07/09/2017 16:28, Artem Smotrakov wrote:
In case of SNI, SSLParemeters.setServerNames() method (and others
related to SNI) was introduced in 8. I don't think the code which
use this method can be compiled with 6 and 7.
All of Java source are compiled by a JDK 10 build, which is specified
by jtreg option "-jdk". Other testing JDKs, including 6 and 7, just
use the .class files on runtime.
That's why the below line exists in Compatibility.java,
32 * @compile -source 1.6 -target 1.6 Server.java Client.java
JdkUtils.java
But if you want to test SNI with 8+, you need to call them.
https://docs.oracle.com/javase/8/docs/api/javax/net/ssl/SSLParameters.html#setServerNames-java.util.List-
If a feature, like ALPN and SNI, is not supported, such method would
not be called on runtime.
Best regards,
John Jiang
Artem
On 09/07/2017 11:24 AM, sha.ji...@oracle.com wrote:
Hi Artem,
On 07/09/2017 16:07, Artem Smotrakov wrote:
- Please use try-with-resources if possible (files, sockets, etc)
The test uses only JDK 6-supported language features, but
try-with-resources is introduced by JDK 7.
Here we come to another issue. I believe that it would be good to
use write clients for all JDK versions. If you use the same code
for all Java versions, that means that you use only JDK 6 API.
Let's consider the following:
- we want to test 8 vs 9
- 8 and 9 may have some API (or just functionality) which is not
supported by 6 (for example, ALPN, SNI)
If you write code which is compatible with 6, then you can use API
from newer versions, but we still may want to test it.
In fact, the current test has already covered ALPN, though only JDK
9 and 10 support the associated methods, like
SSLParameters.getApplicationProtocols().
ALPN-associated case combinations are ignored if a JDK doesn't
support this feature. Exactly, JdkUtils checks if a specific JDK
build contains method SSLParameters.getApplicationProtocols().
I think the same approach could be applied for SNI in the future.
Best regards,
John Jiang
Artem
Best regards,
John Jiang
Artem
On 09/07/2017 08:52 AM, sha.ji...@oracle.com wrote:
Hi,
Please review this test for checking the interop compatibility
on JSSE among different JDK releases (from 6 to 10).
It covers the cases, like handshake, data exchange, client
authentication and APLN, on all TLS versions (if possible).
And the selected TLS cipher suites are:
TLS_RSA_WITH_AES_128_CBC_SHA, TLS_DHE_DSS_WITH_AES_128_CBC_SHA
and TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA.
For more details, please look though the README.
Webrev: http://cr.openjdk.java.net/~jjiang/8186057/webrev.00
Issue: https://bugs.openjdk.java.net/browse/JDK-8186057
Best regards,
John Jiang