RFR: 8350582: Correct the parsing of the ssl value in javax.net.debug

Mikhail Yankelevich duke at openjdk.org
Thu Feb 27 09:59:00 UTC 2025


On Tue, 25 Feb 2025 16:20:59 GMT, Sean Coffey <coffeys at openjdk.org> wrote:

> Breaking the parent JDK-8044609 JBS issue into sub tasks. 
> 
> This patch addresses the main issue which is that `javax.net.debug=ssl ` option is completely broken since TLSv1.3 support was introduced.  This patch should be easier for backporting also.
> 
> Wider corrections can be followed up via parent bug.

test/jdk/sun/security/ssl/SSLLogger/DebugPropertyValues.java line 47:

> 45: import jdk.test.lib.process.OutputAnalyzer;
> 46: 
> 47: public class DebugPropertyValues extends SSLSocketTemplate {

Do you think test name should end with `Test` if it runs with Junit? AFAIK the convention for the naming looks like `[A-Z][A-Za-z\d]*Test(s|Case)?`

test/jdk/sun/security/ssl/SSLLogger/DebugPropertyValues.java line 49:

> 47: public class DebugPropertyValues extends SSLSocketTemplate {
> 48: 
> 49:     static Path LOG_FILE;

Should this file be in the format of a constant? It's changed by the code so it seems to me it can be either changed to `logFile` or line 53 could be moved straight here (something like `private static final Path LOG_FILE = Path.of(...`). 
What do you think?

test/jdk/sun/security/ssl/SSLLogger/DebugPropertyValues.java line 53:

> 51:     @BeforeAll
> 52:     static void setup() throws Exception {
> 53:         LOG_FILE = Path.of(System.getProperty("test.classes"), "logging.conf");

I think this should go the the [scratch](https://openjdk.org/jtreg/faq.html#scratch-directory) directory, as this file is temporary. 
This would look like `Path.of("logging.conf");`

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23781#discussion_r1973227742
PR Review Comment: https://git.openjdk.org/jdk/pull/23781#discussion_r1973246148
PR Review Comment: https://git.openjdk.org/jdk/pull/23781#discussion_r1973236305


More information about the security-dev mailing list