On Sat, 16 May 2026 07:01:49 GMT, Ramesh Bhagavatam Gangadhar 
<[email protected]> wrote:

>> Analysis on Issue
>> ======================
>> 
>> There are two calls made in KeytoolOpensslInteropTest.java in Line 83 and 
>> Line 84
>> 
>> https://github.com/openjdk/jdk/blob/master/test/jdk/sun/security/pkcs12/KeytoolOpensslInteropTest.java#L83-L84
>> 
>>        a)      String opensslPath = OpensslArtifactFetcher.getOpensslPath();
>>        b)     generateInitialKeystores(opensslPath);
>> 
>> 
>> 1. Call made in 
>> https://github.com/openjdk/jdk/blob/master/test/jdk/sun/security/pkcs12/KeytoolOpensslInteropTest.java#L83
>>   to get openssl path which returns "openssl"
>> 
>>              Execution Flow:
>>              ============
>>              
>> https://github.com/openjdk/jdk/blob/master/test/jdk/sun/security/pkcs12/KeytoolOpensslInteropTest.java#L83
>>    calls 
>> https://github.com/openjdk/jdk/blob/master/test/lib/jdk/test/lib/security/OpensslArtifactFetcher.java#L62
>>  calls 
>> https://github.com/openjdk/jdk/blob/master/test/lib/jdk/test/lib/security/OpensslArtifactFetcher.java#L98
>>  and finally returns "openssl" from 
>> https://github.com/openjdk/jdk/blob/master/test/lib/jdk/test/lib/security/OpensslArtifactFetcher.java#L100
>> 
>> 
>> 
>> 2.   return value of  "openssl" is passed as argument to 
>> generateInitialKeystores("openssl") and same value is navigated here as well
>>       
>> https://github.com/openjdk/jdk/blob/master/test/jdk/sun/security/pkcs12/KeytoolOpensslInteropTest.java#L110
>>  and call made to  
>>  OpensslArtifactFetcher.getProviderPath("openssl") 
>>  
>> https://github.com/openjdk/jdk/blob/master/test/lib/jdk/test/lib/security/OpensslArtifactFetcher.java#L133
>>  ==> this is the statement where we are seeing error
>> 
>>  Path openSslRootPath = Path.of("openssl").getParent().getParent();
>> 
>> =====> Path.of("openssl").getParent() = null
>> 
>> 
>> 
>> Solution
>> ==========
>> 
>> 1. We can remove complete System Installed Library dependency and rely on 
>> artifacts installed through artifactory.
>> 2. Instead of returning "openssl" from 
>> https://github.com/openjdk/jdk/blob/master/test/lib/jdk/test/lib/security/OpensslArtifactFetcher.java#L100
>>  return the absolute path of openssl.
>> 
>> 
>> I have opted for Solution 2. and changed the code and i encountered one more 
>> situation where ProviderPath doesn't contain "ossl-modules" folder at all.
>> 
>> e.g.
>> 
>> if 
>> https://github.com/openjdk/jdk/blob/master/test/jdk/sun/security/pkcs12/KeytoolOpensslInteropTest.java#L83
>>  returns absolute path "/usr/bin/openssl" then call made to get Provider 
>> Path here: 
>> https://github.com/openjdk/jdk/blob/master/test/jdk/sun/security/pkcs12/KeytoolOpensslIntero...
>
> Ramesh Bhagavatam Gangadhar has updated the pull request incrementally with 
> one additional commit since the last revision:
> 
>   add error message

test/lib/jdk/test/lib/security/OpensslArtifactFetcher.java line 69:

> 67:         }
> 68:         path = getArtifactsFromArtifactory();
> 69:         if (path == null) {

What do you think about reversing this and returning if `path != null` and 
throwing an exception in the end? Just might be easier to add cases later and 
prevent bugs, also consistency with previous cases

test/lib/jdk/test/lib/security/OpensslArtifactFetcher.java line 132:

> 130:             String output = null;
> 131:             String windowsPath = null;
> 132:             String cygwinPath = null;

This is an odd way to structure a java code, why not just initialise all 
variables as final inside of the try catch? 

Also, this code will always return null since windowsPath is always null

test/lib/jdk/test/lib/security/OpensslArtifactFetcher.java line 134:

> 132:             String cygwinPath = null;
> 133:             try {
> 134:             OutputAnalyzer outputAnalyzer = 
> ProcessTools.executeProcess("openssl","version","-d");

Please mind the code formatting. Also please keep the line length under 80 
chars for security packages

test/lib/jdk/test/lib/security/OpensslArtifactFetcher.java line 138:

> 136:             cygwinPath = output.split(":")[1].trim().replace("\"", "");
> 137:             System.out.println("cygwinPath: "+cygwinPath);
> 138:             OutputAnalyzer windowsOutput = 
> ProcessTools.executeProcess("cygpath", "-w", cygwinPath);

windowsOutput is never used

test/lib/jdk/test/lib/security/OpensslArtifactFetcher.java line 161:

> 159:     }
> 160: 
> 161:     private static String getOpenSslAbsolutePath() {

This is only used in `getDefaultSystemOpensslPath` so it could be moved there. 
Alternatively please add a windows version of the same as a separate method 
just to keep the code consistent

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

PR Review Comment: https://git.openjdk.org/jdk/pull/30561#discussion_r3257581037
PR Review Comment: https://git.openjdk.org/jdk/pull/30561#discussion_r3257559523
PR Review Comment: https://git.openjdk.org/jdk/pull/30561#discussion_r3257567358
PR Review Comment: https://git.openjdk.org/jdk/pull/30561#discussion_r3257562094
PR Review Comment: https://git.openjdk.org/jdk/pull/30561#discussion_r3257540721

Reply via email to