JDK-8304264 isn't quite right
Nico Williams
Nico.Williams at twosigma.com
Mon Apr 10 20:57:11 UTC 2023
JDK-8304264 is due to c63fabe3d582ce0828b04b0224cea49aab5fedf3 (8284935:
Improve debug in java.security.jgss) which did this:
```
@@ -51,14 +52,14 @@ public final class SunNativeProvider extends Provider {
private static final String INFO = "Sun Native GSS provider";
private static final String MF_CLASS =
"sun.security.jgss.wrapper.NativeGSSFactory";
- static boolean DEBUG;
+ private static final boolean DEBUG =
+ GetBooleanAction.privilegedGetProperty("sun.security.nativegss.debug");
+
static void debug(String message) {
- if (DEBUG) {
- if (message == null) {
- throw new NullPointerException();
- }
- System.out.println(NAME + ": " + message);
+ if (message == null) {
+ throw new NullPointerException();
}
+ System.out.println(NAME + ": " + message);
}
@SuppressWarnings("removal")
```
and then changed all the `debug()` method call sites to check
SunNativeProvider.DEBUG. Several JGSS classes have their own DEBUG
methods.
This was done to avoid string concatenation at the call sites when debug
is disabled. Fair enough. But the fix for JDK-8284935 (or subsequent
commits) missed some call sites, yielding JDK-8304264.
It would be better to also have the debug check in the debug() method(s)
just in case call sites are missed. Because the boolean class member is
a `static final` the JIT should optimize the superfluous branch away.
Also, while we're at it, printing debug messages to `stdout` is a
terrible thing to do. The only reason I'm even writing this is that
those messages go to `stdout`. The chance of accidental writing to
`stdout` (which can cause problems for apps) should be zero.
Thanks,
Nico
--
More information about the security-dev
mailing list