[GitHub] [commons-io] garydgregory commented on a change in pull request #197: [IO-712] SecurityExceptions are hidden instead of breaking the regular …

[GitBox <gi...@apache.org>]

garydgregory commented on a change in pull request #197:
URL: https://github.com/apache/commons-io/pull/197#discussion_r567153417



##########
File path: src/main/java/org/apache/commons/io/FileSystem.java
##########
@@ -146,27 +146,16 @@ private static boolean getOsMatchesName(final String osNamePrefix) {
     }
 
     /**
-     * <p>
-     * Gets a System property, defaulting to {@code null} if the property cannot be read.
-     * </p>
-     * <p>
-     * If a {@code SecurityException} is caught, the return value is {@code null} and a message is written to
-     * {@code System.err}.
-     * </p>
+     * Gets a System property, defaulting to {@code null} if the property cannot be read,
+     * except for {@code SecurityException}.
      *
      * @param property
      *            the system property name
-     * @return the system property value or {@code null} if a security problem occurs
+     * @return the system property value or {@code null} if a problem occurs
+     * @throws SecurityException if underlying calls fail due to missing permissions.
      */
     private static String getSystemProperty(final String property) {
-        try {
-            return System.getProperty(property);
-        } catch (final SecurityException ex) {
-            // we are not allowed to look at this property
-            System.err.println("Caught a SecurityException reading the system property '" + property
-                    + "'; the SystemUtils property value will default to null.");
-            return null;
-        }
+        return System.getProperty(property);

Review comment:
       This is a special case. The whole purpose of this API is to never blow up. I am not sure how useful this method really is in practice though. If you are running under JAAS and this call returns null because you can't read a sys prop and the rest of your app runs (or not), then you must have a very specific set of JAAS permissions set. But... I can see that one might want to write out or log sys props on start up, and then, writing 'null' instead of blowing up provides... what? I'm not sure. All of this rambling to say that in this case we should not change the behavior IMO.
   
   Writing to the console is a no-no so we should get rid of that for sure.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org