On Tue, 7 Apr 2026 05:45:13 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:
> 
>   removed sysout statements

test/jdk/sun/security/pkcs12/KeytoolOpensslInteropTest.java line 2:

> 1: /*
> 2:  * Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights 
> reserved.

Suggestion:

 * Copyright (c) 2018, 2026, Oracle and/or its affiliates. All rights reserved.

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

> 26: import java.io.IOException;
> 27: import java.nio.file.Path;
> 28: import java.io.*;

Nit: could you please convert this to explicit imports?

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

> 57:      *         or if OpenSSL is not available on the target platform
> 58:      */
> 59:      public static String getOpensslPath() {

Suggestion:

    public static String getOpensslPath() {

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

> 73:                 OPENSSL_BUNDLE_VERSION, Platform.getOsName(), 
> Platform.getOsArch()));
> 74:         }
> 75:         System.out.println("Artifacts Path is"+path);

Suggestion:

        System.out.println("Artifacts Path is "+path);

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

> 85:         path = 
> getDefaultSystemOpensslPathWithProviderPathPresent(OPENSSL_BUNDLE_VERSION);
> 86:         if (path != null) {
> 87:             System.out.println("Using OpenSSL from System Installed 
> Library:"+path);

Suggestion:

            System.out.println("Using OpenSSL from System Installed Library: 
"+path);

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

> 89:         }
> 90:         path = getArtifactsFromArtifactory();
> 91:         if(path == null) {

Suggestion:

        if (path == null) {

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

> 93:                 OPENSSL_BUNDLE_VERSION, Platform.getOsName(), 
> Platform.getOsArch()));
> 94:         }
> 95:         System.out.println("Artifacts Path is:"+path);

Suggestion:

        System.out.println("Artifacts Path is "+path);

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

> 146:         String absolutePath = null;
> 147:         try {
> 148:             OutputAnalyzer outputAnalyzer = 
> ProcessTools.executeProcess("which","openssl");

AFAIK `which` is not present for windows. Do you think this would work on 
windows machines?

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

> 148:             OutputAnalyzer outputAnalyzer = 
> ProcessTools.executeProcess("which","openssl");
> 149:             absolutePath = outputAnalyzer.getOutput();
> 150:             System.out.println("absolutePath:"+absolutePath);

Suggestion:

            System.out.println("absolutePath: "+absolutePath);

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

> 150:             System.out.println("absolutePath:"+absolutePath);
> 151:         } catch(Throwable t) {
> 152:             t.printStackTrace();

Minor: I'd also put this in the `System.err` as well as `System.out`

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

PR Review Comment: https://git.openjdk.org/jdk/pull/30561#discussion_r3044680461
PR Review Comment: https://git.openjdk.org/jdk/pull/30561#discussion_r3044630245
PR Review Comment: https://git.openjdk.org/jdk/pull/30561#discussion_r3044682615
PR Review Comment: https://git.openjdk.org/jdk/pull/30561#discussion_r3044632932
PR Review Comment: https://git.openjdk.org/jdk/pull/30561#discussion_r3044636381
PR Review Comment: https://git.openjdk.org/jdk/pull/30561#discussion_r3044670049
PR Review Comment: https://git.openjdk.org/jdk/pull/30561#discussion_r3044639133
PR Review Comment: https://git.openjdk.org/jdk/pull/30561#discussion_r3044651605
PR Review Comment: https://git.openjdk.org/jdk/pull/30561#discussion_r3044659238
PR Review Comment: https://git.openjdk.org/jdk/pull/30561#discussion_r3044663786

Reply via email to