Comments (5)
I didn't express my point about the JDK very well. What I meant was that if we add a workaround that is supposed to function when there is no AccessController
class, we can't really test it until we have a JDK that really does have no AccessController
class.
Thanks for reminding me of the previous discussion, where indeed I noted that enum constants are public but enums themselves aren't necessarily. Still, the whole thing feels very complicated and I'm not sure it is worthwhile now. I would be somewhat inclined to rip it all out and see if anybody complains. If they do, we can put it back again in another release and perhaps also look at handling a missing AccessController
.
from gson.
Yes, the more I think about it, the more I agree with my earlier comment about just ripping out the AccessController
usage. :-)
It may be that there are people who are depending on it and who are keeping up to date with Gson versions but not JDK versions. If so we can learn about their use cases when they complain after the next release, and then we can decide what to do. And if there is in fact nobody depending on the AccessController
then the code will be that much simpler without it.
from gson.
It's going to be difficult to be sure about workarounds for a missing AccessController
until we have a JDK that has a missing AccessController
.
I'm a bit puzzled about the code in question. Aren't enum constants public? So then we should be able just to use Class.getFields()
to read them? Technically that can still throw SecurityException
:
If a security manager, s, is present and the caller's class loader is not the same as or an ancestor of the class loader for the current class and invocation of
s.checkPackageAccess()
denies access to the package of this class.
I am inclined to think that that is going to affect hardly anybody. I'm also not sure we need to be able to call Field.get
, since the name of the constant is the name of the field.
from gson.
It's going to be difficult to be sure about workarounds for a missing
AccessController
until we have a JDK that has a missingAccessController
.
I understood JEP 411 as if the idea was to remove SecurityManager
and related classes without adding any replacement. But you are right, it is not clear what exactly will happen; it seems there is no follow-up JEP yet.
I'm a bit puzzled about the code in question.
Coincidentally you were the one who suggested using AccessController#doPrivileged
in #1495 (comment) 😅, but for the setAccessible
call.
I then later expanded it to only have one AccessController#doPrivileged
call for all fields instead of one per field.
I'm also not sure we need to be able to call
Field.get
, since the name of the constant is the name of the field.
Unfortunately not when ProGuard or R8 are involved. If I remember correctly, name()
still returns the expected value, but the field names might be obfuscated. And the enum values()
method to obtain all constants (which is used by Class.getEnumConstants()
) might have been removed by the code shrinker (at least #1147 mentions that).
Another option would also be to simply remove the AccessController#doPrivileged
call. There are only a few issues and pull requests related to SecurityManager
(#566, #1361, #1712), and maybe nowadays most users don't use a SecurityManager
.
But maybe we can wait with that until it becomes clearer which approach the JDK maintainers will take regarding removal of SecurityManager
.
from gson.
It seems usage of AccessController
actually makes a difference. Without it the following test testJdkEnum_SecurityManager
fails with AccessControlException: access denied ("java.lang.RuntimePermission" "accessDeclaredMembers")
.
(Though I am not completely sure if this is a proper SecurityManager
/ Policy
test setup.)
@Test
public void testJdkEnum() {
assertThat(gson.toJson(Thread.State.NEW)).isEqualTo("\"NEW\"");
assertThat(gson.fromJson("\"NEW\"", Thread.State.class)).isEqualTo(Thread.State.NEW);
}
@SuppressWarnings("removal") // java.lang.SecurityManager deprecation in Java 17
@Test
public void testJdkEnum_SecurityManager() {
// Skip for newer Java versions where `System.setSecurityManager` is unsupported
assumeTrue(Runtime.version().feature() <= 17);
var gsonProtectionDomain = Gson.class.getProtectionDomain();
var testProtectionDomain = getClass().getProtectionDomain();
assertWithMessage(
"Gson code and test code should have different ProtectionDomain; otherwise test might"
+ " not work properly")
.that(gsonProtectionDomain)
.isNotEqualTo(testProtectionDomain);
Policy originalPolicy = Policy.getPolicy();
SecurityManager originalSecurityManager = System.getSecurityManager();
try {
Policy.setPolicy(
new Policy() {
@Override
public boolean implies(ProtectionDomain domain, Permission permission) {
// Allow removing the SecurityManager again
if (permission instanceof RuntimePermission
&& permission.getName().equals("setSecurityManager")) {
return true;
}
if (domain.equals(gsonProtectionDomain)) {
return true;
}
return originalPolicy.implies(domain, permission);
}
});
System.setSecurityManager(new SecurityManager());
assertThat(gson.toJson(Thread.State.NEW)).isEqualTo("\"NEW\"");
assertThat(gson.fromJson("\"NEW\"", Thread.State.class)).isEqualTo(Thread.State.NEW);
} finally {
System.setSecurityManager(originalSecurityManager);
Policy.setPolicy(originalPolicy);
}
}
However, it seems there are a few other places which fail when a custom SecurityManager
is used:
com.google.gson.reflect.TypeToken.isCapturingTypeVariablesForbidden()
Due to it's usage ofSystem#getProperty
- Reflection-based adapter
Probably not that surprising, and should probably fail to prevent accessing implementation details of third-party classes. But it even fails for Record classes. - Getting no-args constructor for
Collection
andMap
adapter com.google.gson.internal.$Gson$Types.equals(Type, Type)
This callsTypeVariable.getGenericDeclaration()
and apparently the implementationsun.reflect.generics.reflectiveObjects.TypeVariableImpl
performs undocumented access checks. (Or my test setup is wrong?)
So unless my test setup is wrong or flawed this suggests that usage with a SecurityManager
was already cumbersome, or was not working in some cases. So maybe removing the AccessController
usage might be fine since most users don't rely on it?
from gson.
Related Issues (20)
- Edge case where R8 removes the TypeToken's generic type (while consuming the library's recently updated proguard rules)
- `JsonReader#beginObject` should throw checked exceptions, instead throws runtime `IllegalStateException`. HOT 2
- Serialization failure at index 0 and 1 in camel case field naming convention HOT 2
- [2.11.0] error_prone_annotations dependency HOT 5
- The module declaration in version 2.11.0 is not visible HOT 11
- TypeAdapterFactory INTEGER_FACTORY will change INTEGER type to LONG type HOT 2
- errorprone cannot be considered optional HOT 4
- Can't read multiline string in JSON5 HOT 3
- Performance regressions from 2.10.1 to 2.11.0 on R8 minification task HOT 2
- JDK21 toJson failure HOT 6
- Make com.google.gson.JsonElement Serializeable HOT 1
- Custom Gson, singleton mode, deserialization in multiple coroutines, resulting in crash HOT 2
- error HOT 4
- Read `json_name` from protobuf field annotations for serialization and deserialization
- TypeToken.getParameterized(Response.class, List.class, Long.class) get IllegalArgumentException: requires 1 type arguments, but got 2 HOT 3
- Just a Question for nested JSON HOT 2
- Yes can confirm this; it looks like this was indeed introduced by 3e3266cf48f132928225e1561a6ae4cb5503d08f (by accident): HOT 1
- java.lang.StackOverflowError: stack size 8MB HOT 1
- Gson behaviour is different in Unit test and in actual build HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from gson.