Introduce SystemAccessUtils to wrap AccessController.doPrivileged() - #1668
Introduce SystemAccessUtils to wrap AccessController.doPrivileged()#1668taoliult wants to merge 1 commit into
Conversation
40bc863 to
12c3b33
Compare
|
|
||
| //disable caching mechanism for windows OS | ||
| final static private boolean isWindows = System.getProperty("os.name").startsWith("Windows"); | ||
| final static private boolean isWindows = SystemAccessUtils.doPrivileged(() -> System.getProperty("os.name")).startsWith("Windows"); |
There was a problem hiding this comment.
Should we change this to something that most resembles the existing call? Could we do something like SystemAccessUtils.getProperty("os.name");
There was a problem hiding this comment.
I considered adding a SystemAccessUtils.getProperty() method before, but the problem is it doesn't stop there — we would also need separate methods for System.load(), File.exists(), File.getAbsolutePath(), new FileReader(), Class.forName(), OCK context initialization, and so on. That would turn SystemAccessUtils into an ever-growing class with one wrapper per JDK operation.
The three existing methods — doPrivileged(), doPrivilegedChecked(), and runPrivileged() — I think they can cover all cases in the current codebase: value-returning without checked exceptions, value-returning with checked exceptions, and void operations respectively. Any future privileged operation can be wrapped with one of these three without touching SystemAccessUtils.
There was a problem hiding this comment.
Hi @taoliult I also lean toward it being a better approach what Kostas mentioned above for a few reasons. I think it will be easier to ensure that a call that needs a privledged action actually calls the utility class. We can review it as so on PRs made to main. Also doing this will avoid having generic lambda functions throughout the code making it a bit cleaner. For example all calls would do a SystemAccessUtils.getProperty instead of having to write a lambda expression for each.
a435aee to
81b622d
Compare
| @@ -143,7 +143,7 @@ public List<ServiceDefinition> readServices() throws IOException { | |||
| throw new IOException("File not found: " + filePath); | |||
There was a problem hiding this comment.
Should we add a privileged check on this Files.exists() function?
4501cde to
13f8e2c
Compare
10601e3 to
a7e07b9
Compare
b0a56a3 to
9323557
Compare
db4db36 to
3274784
Compare
Introduce SystemAccessUtils to wrap AccessController.doPrivileged() in APIs (doPrivileged, doPrivilegedChecked, runPrivileged). On JDK 21 and earlier, all three methods delegate to AccessController.doPrivileged() so privileged operations succeed when SecurityManager enabled and required permission. On JDK 25 and later, where SecurityManager and AccessController are removed entirely, the same SystemAccessUtils class is kept but the methods become pass-through no-ops that simply invoke the supplied action directly, so callers need no changes regardless of JDK version. Signed-off-by: Tao Liu <tao.liu@ibm.com>
1e50e81 to
d725f65
Compare
5 participants
Introduce SystemAccessUtils to wrap AccessController.doPrivileged() in APIs (doPrivileged, doPrivilegedChecked, runPrivileged). On JDK 21 and earlier, all three methods delegate to AccessController.doPrivileged() so privileged operations succeed when SecurityManager enabled and required permission. On JDK 25 and later, where SecurityManager and AccessController are removed entirely, the same SystemAccessUtils class is kept but the methods become pass-through no-ops that simply invoke the supplied action directly, so callers need no changes regardless of JDK version.