1.x: Plugin lookup workaround for System.properties access restrictions #5820
Conversation
Codecov Report
@@ Coverage Diff @@
## 1.x #5820 +/- ##
============================================
+ Coverage 84.07% 84.25% +0.17%
- Complexity 2881 2891 +10
============================================
Files 290 290
Lines 18258 18264 +6
Branches 2495 2495
============================================
+ Hits 15351 15388 +37
+ Misses 2013 1996 -17
+ Partials 894 880 -14
Continue to review full report at Codecov.
|
| * therefore, the SecurityException is turned into an empty properties. | ||
| * @return the Properties to use for looking up settings | ||
| */ | ||
| /* test */ static Properties getSystemPropertiesSafe() { |
There was a problem hiding this comment.
It indicates the method is package private for testing purposes instead of private.
There was a problem hiding this comment.
It's consistent with the other methods. ¯_(ツ)_/¯
There was a problem hiding this comment.
Ah, haven't seen these in a while, my bad
| // https://github.com/ReactiveX/RxJava/issues/5819 | ||
| // We don't seem to have access to all properties. | ||
| // At least print the exception to the console. | ||
| ex.printStackTrace(); |
There was a problem hiding this comment.
Maybe call RxJavaPlugins.onError() or however that api is called in 1.x?
I believe people won't like stacktrace popping up in their logs without ability to swallow it
There was a problem hiding this comment.
Can't call that because this is part of the initialization process of RxJavaPlugins, the error handler would come from the same system properties and the default does nothing.
The PR adds a
try-catcharound the System property lookup inside theRxJavaPluginsin case a security manager prevents reading arbitrary property entries.This mainly affects the
rxjava.plugin.[index].classlookup which were introduced due to the 31 character key limit on Android.However, when running in a container such as Tomcat, a security manager may prevent reading these type of prefixed entries (where
[index]can't be known upfront), crashing the initialization.Update:
The
System.getProperties()can also fail, therefore, retrieving the properties has been factored out into a separate method that returns an empty properties.Fixes #5819.