Catch security exception in get environment call #1852
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
frankfliu
approved these changes
Aug 1, 2022
Codecov Report
@@ Coverage Diff @@
## master #1852 +/- ##
============================================
- Coverage 72.08% 69.74% -2.34%
- Complexity 5126 5532 +406
============================================
Files 473 526 +53
Lines 21970 24449 +2479
Branches 2351 2660 +309
============================================
+ Hits 15838 17053 +1215
- Misses 4925 6089 +1164
- Partials 1207 1307 +100
Continue to review full report at Codecov.
|
frankfliu
reviewed
Aug 1, 2022
| try { | ||
| return System.getenv(name); | ||
| } catch (SecurityException e) { | ||
| logger.error("Security manager doesn't allow access to the environment variable"); |
There was a problem hiding this comment.
Suggested change
| logger.error("Security manager doesn't allow access to the environment variable"); | |
| logger.warn("Security manager doesn't allow access to the environment variable"); |
| * the system environment or security manager doesn't allow access to the environment | ||
| * variable | ||
| */ | ||
| public static String getenv(String name) { |
There was a problem hiding this comment.
I think it's better have an overload function:
public static String getenv(String name, String def) {
frankfliu
reviewed
Aug 1, 2022
| * variable | ||
| */ | ||
| public static String getenv(String name) { | ||
| try { |
There was a problem hiding this comment.
return getenv(name, null);
frankfliu
reviewed
Aug 1, 2022
| } catch (SecurityException e) { | ||
| logger.warn("Security manager doesn't allow access to the environment variable"); | ||
| } | ||
| return Collections.unmodifiableMap(new HashMap<>()); |
There was a problem hiding this comment.
Suggested change
| return Collections.unmodifiableMap(new HashMap<>()); | |
| return Collections.emptyMap(); |
| * | ||
| * @param name the name of the environment variable | ||
| * @param def a default value | ||
| * @return the string value of the variable, or {@code null} if the variable is not defined in |
There was a problem hiding this comment.
Suggested change
| * @return the string value of the variable, or {@code null} if the variable is not defined in | |
| * @return the string value of the variable, or {@code def} if the variable is not defined in |
| @@ -30,7 +31,9 @@ | |||
| import java.util.ArrayList; | |||
| import java.util.Collections; | |||
| import java.util.Comparator; | |||
| import java.util.HashMap; | |||
There was a problem hiding this comment.
Suggested change
| import java.util.HashMap; |
patins1
pushed a commit
to patins1/djl
that referenced
this pull request
Aug 26, 2022
* Catch security exception in get environment call * Review changes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Brief description of what this PR is about
This PR is to fix #1848
The customer run into the get environment security issue. This PR put the environment call into a utility function and catch the exception.