RFR: 8344365: SecurityManager cleanups in java.sql and java.sql.rowset modules [v2]

[Lance Andersen]

On Mon, 18 Nov 2024 08:37:26 GMT, Eirik Bjørsnøs <eirbjo at openjdk.org> wrote:

>> Please review this PR which cleans up SecurityManager-related code in `java.sql` and `java.sql.rowset` modules post JEP-486
>> 
>> There are quite a few changes to review, but all relatively straightforward:
>> 
>> `DriverManager`
>> * Remove `SecurityManager::checkPermission` calls in the `setLogWriter`, `setLogStream` and `deregisterDriver` methods
>> * Remove two now-unused package private SQLPermission constants
>> * `ensureDriversInitialized` is updated to remove `AccessController::doPrivileged` when reading a system property and when initializing drivers
>> 
>> `CachedRowSetImpl`
>> *  Remove `AccessController::doPrivileged` when getting a `SyncFactory` instance
>> * `getObject` is update to remove a call to `ReflectUtil::checkPackageAccess`
>> 
>> `CachedRowSetWriter`
>> * A call to `ReflectUtil::checkPackageAccess` is removed.
>> 
>> `SerialJavaObject`
>> *  `getFields` is updated to remove call to `ReflectUtil::checkPackageAccess`. `@CallerSensitive` is no longer needed for this method.
>> * The test `CheckCSMs.java` is updated to remove references to `SerialJavaObject:getFields`
>> 
>> `SyncFactory`
>> * `initMapIfNecessary` is updated to remove call to `AccessController::doPrivileged` when reading system properties and when reading properties from an input stream
>> * `getInstance` is updated to remove calls to `ReflectUtil::checkPackageAccess`
>> * `setLogger` method is updated to remove call to `SecurityManager::checkPermission`
>> * `setJNDIContext` methods are updated to remove call to `SecurityManager::checkPermission`
>> 
>> `RowsetProvider`
>> *  Static initializer is updated to call `System::getProperty` directly
>> * `newFactory` is updated to call `System::getProperty` directly
>> * `newFactory` is updated to not call `ReflectUtil.checkPackageAccess`
>> * `getContextClassLoader` is updated to not call `AccessController::doPrivileged`
>> * `getFactoryClass` is updated to not call `ReflectUtil.checkPackageAccess`
>> * `getSystemProperty` is removed
>> 
>> 
>> `SQLInputImpl`
>> *  A call to `ReflectUtil::checkPackageAccess` is removed
>> 
>>  `TestPolicy.java` in `test/java/sql/testng/util`
>> * This  is now unused and removed
>> 
>> Ran `test/jdk/java/sql` and `test/jdk/javax/sql` tests locally. GHA results pending.
>
> Eirik Bjørsnøs has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Revert caller-sensitive inlining of ReflectUtil.forName
>  - SerialJavaObject::getFields is no longer @CallerSensitive, remove it from CheckCSM test

Thanks for tackling this Eirik.  I made a pass through and I think the changes are reasonable.

I have asked Sean to also make a pass as well

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

PR Review: https://git.openjdk.org/jdk/pull/22185#pullrequestreview-2443473243