build: Added configurations to apply constraints to dependencies resolves #134
#159
Conversation
…ent `gestalt` pulling in dependencies with known CVEs, directly or transitively through things like Reflections. Added some suppressions in various files and as well removed some unnecessary imports
Thedrogon
changed the title
Thedrogon
changed the title
resolves #134
...alt-asset-core/src/main/java/org/terasology/gestalt/assets/AbstractFragmentDataProducer.java
Outdated
Show resolved
Hide resolved
gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java
Outdated
Show resolved
Hide resolved
gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java
Outdated
Show resolved
Hide resolved
gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java
Show resolved
Hide resolved
… to prevent `gestalt` pulling in dependencies with known CVEs, directly or transitively through things like Reflections. Added some suppressions in various files and as well removed some unnecessary imports" This reverts commit 43923eb.
…s of (deprecated)access-controller so that it can be removed without errors
gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java
Outdated
Show resolved
Hide resolved
|
@BenjaminAmos does this look good to you now? apart from commented stuff which imo should be better removed ... |
There was a problem hiding this comment.
I appreciate you trying to make changes for the better but not all warnings are actionable here for reasons that I have tried to explain.
If you could constrain the changes to that tiny but beneficial gradle constraint improvement then this would be much easier to take in. Removing logic in the code without understanding why it is there (this library is quite a central piece of our games) just makes this pull request unviable as-is, I'm afraid.
| @@ -328,16 +330,16 @@ Optional<T> createInstance(Asset<U> asset) { | |||
| Optional<? extends Asset<U>> result = asset.createCopy(asset.getUrn().getInstanceUrn()); | |||
| if (!result.isPresent()) { | |||
| try { | |||
| return AccessController.doPrivileged((PrivilegedExceptionAction<Optional<T>>) () -> { | |||
There was a problem hiding this comment.
Despite being deprecated, I'm afraid that for the time being we do still need to use the AccessController APIs. Gestalt sandboxes modules using SecurityManager as an additional precaution and until it's completely unusable I'd rather have that safety net than not. The AccessController.doPrivileged calls are needed when running with a SecurityManager active.
There was a problem hiding this comment.
Then for the requirement I will revert them in the PR.
| @@ -131,7 +133,7 @@ public synchronized void close() { | |||
| /** | |||
| * Disposes any assets queued for disposal. This occurs if an asset is no longer referenced by anything. | |||
| */ | |||
| @SuppressWarnings("unchecked") | |||
| //@SuppressWarnings("unchecked") | |||
There was a problem hiding this comment.
You can either remove those suppressions or leave them be but commenting them out like that in committed code is rather messy.
Added configurations to apply constraints to the
build.gradledependencies to preventgestaltpulling in dependencies with known CVEs, directly or transitively through things like Reflections. Added some suppressions in various files and as well removed some unnecessary imports