On Mon, 18 Nov 2024 19:47:25 GMT, Eirik Bjørsnøs <eir...@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 with a new target base due to a
> merge or a rebase. The pull request now contains 11 commits:
>
> - Merge branch 'master' into sm-cleanup-sql
>
> # Conflicts:
> # test/jdk/jdk/internal/reflect/CallerSensitive/CheckCSMs.java
> - Revert caller-sensitive inlining of ReflectUtil.forName
> - SerialJavaObject::getFields is no longer @CallerSensitive, remove it from
> CheckCSM test
> - Remove unused class TestPolicy from test/java/sql/testng/util
> - Remove unused SQLPermission constants
> - Update copyright
> - Remove calls to ReflectUtil.checkPackageAccess
> - Inline call to Class.forName
> - Remove call to ReflectUtil::checkPackageAccess
> - Remove @SuppressWarnings("removal")
> - ... and 1 more: https://git.openjdk.org/jdk/compare/92271af6...3f1df59a
Yes, please cleanup the leftover mention, in CheckCSMs. of
ObjectStreamField.getType().
And I'll re-review.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/22185#issuecomment-2484061603
