[jdk8u-dev] RFR: 8200468: Port the native GSS-API bridge to Windows

[Andrew John Hughes]

On Mon, 12 Jun 2023 06:20:11 GMT, Alexey Bakhtin <abakhtin at openjdk.org> wrote:

> Hello,
> I'd like to backport JDK-8200568: port the native GSS-API bridge to Windows to JDK8u
> This is a prerequisite of the JDK-6722928 and JDK-8225687
> 
> The backport is not clean. I have to to make the following changes:
> jdk/make/lib/SecurityLibraries.gmk
> - Patched manually, just removed platform dependency
> 
> jdk/src/share/native/sun/security/jgss/wrapper/NativeUtil.c
> - Clean merge except for copyright year
> 
> jdk/src/share/native/sun/security/jgss/wrapper/NativeUtil.h
> - Visual Studio 2010-2012 doesn't provide inttypes.h so provide appropriate definitions the header file
> - Manually updated copyright year
> 
> jdk/src/share/classes/sun/security/jgss/GSSManagerImpl.java
> - Manually removed platform OS dependency
> 
> jdk/src/solaris/native/sun/security/jgss/wrapper/NativeFunc.c is moved to jdk/src/share/native/sun/security/jgss/wrapper/NativeFunc.c
> - All changes applied manually because of JDK-8238388 already applied 
> 
> jdk/src/solaris/native/sun/security/jgss/wrapper/NativeFunc.h is moved to jdk/src/share/native/sun/security/jgss/wrapper/NativeFunc.h
> - All changes applied clean 
> 
> jdk/test/sun/security/krb5/auto/BasicProc.java
> - Manually added Windows platform dependency code
> 
> jdk/test/java/security/testlibrary/Proc.java
> - Manually extended debug output
> 
> All related jtreg tests passed successfully on Windows

Mostly looks good but there are a couple of issues:

* You mention adjusting the copyright header for `NativeUtil.c` and `NativeUtil.h`, but I don't see them in the patch. They should be bumped from 2014 to 2018. The other omissions are correct, as the files already have newer copyright headers.
* The change of the `@library` line in `BasicProc.java` is unneeded. There is a Platform class in `jdk.testlibrary` which can be imported as `jdk.testlibrary.Platform`. We should aim to resolve this duplication (caused by the JFR import) going forward, but I think for now, the test should stick to one test library. Adding the `/lib` reference means it could potentially mix the two test libraries together as they have duplicated classes, so please drop the `lib` and use `jdk.testlibrary.Platform` for now.
* In the 11u patch, the `pc.env` line is outside the `!Platform.isWindows` block, but seems to be inside in the 8u version.
11u:
~~~
+            if (!Platform.isWindows()) {
+                Files.setPosixFilePermissions(Paths.get(label + ".ccache"),
+                        Set.of(PosixFilePermission.OWNER_READ,
+                                PosixFilePermission.OWNER_WRITE));
+            }
+            pc.env("KRB5CCNAME", "FILE:" + label + ".ccache");

$ find jdk/test/ -name 'Platform.java'
jdk/test/lib/testlibrary/jdk/testlibrary/Platform.java
jdk/test/lib/jdk/test/lib/Platform.java

~~~

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

PR Review: https://git.openjdk.org/jdk8u-dev/pull/335#pullrequestreview-1502913957