-
Notifications
You must be signed in to change notification settings - Fork 355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow to use additional properties with security manager/4323 #4327
The head ref may contain hidden characters: "jbescos/\u00964323"
Conversation
…manager is on Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
…rsey Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
...c/test/java/org/glassfish/jersey/internal/config/SystemPropertiesConfigurationModelTest.java
Show resolved
Hide resolved
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
assertTrue(properties.containsKey(MultiPartProperties.BUFFER_THRESHOLD)); | ||
assertTrue(properties.containsKey(OAuth1ServerProperties.ACCESS_TOKEN_URI)); | ||
} finally { | ||
System.setSecurityManager(null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or just remove that line? because you are setting securityManager 3 lines below...
* It doesn't work for higher JDKs because it is using different classloader | ||
* (jdk.internal.loader.ClassLoaders$AppClassLoader) that current Guava version doesn't support. | ||
*/ | ||
String version = System.getProperty("java.version"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is JdkVersion class in Jersey which helps to work with versions. Could it be useful here?
|
||
grant { | ||
permission java.util.PropertyPermission "jersey.config.allowSystemPropertiesProvider", "read"; | ||
permission java.io.FilePermission "<<ALL FILES>>", "read,write,delete"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alignment?
return Void.class; | ||
} | ||
}); | ||
for (Predicate<Class<?>> predicate : predicates) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since you are already using streams I suppose it would be appropriate to rewrite this FOR cycle with :
.filter(Arrays.stream(predicates).reduce(x->true, Predicate::and))
and then you can just return
.collect(Collectors.toList())
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now it looks pretty neat
This pull request is related to this issue:
#4323
The properties are not loaded by reflection, they are explicitly set in a list. The reason I didn't use reflection is we only want to load jersey properties, so we already know them.
In case we create a new property class and we forget to add it in the list, there is a new test that should fail.