-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61202/#review182398
-----------------------------------------------------------



Thanks, indentation looks fine now. I'd like to see just a few minor 
improvements to the code in RangerHiveAuthorizer.

1) The "if" statement could be moved up to the top under the for loop to avoid 
creating unnecessary objects.
2) Also in the if statement it references "msObjRef.getObjectType()", whereas 
just above we have the definition of "objectType" that could be used instead.

- Colm O hEigeartaigh


On Aug. 4, 2017, 1:25 p.m., pengjianhua wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61202/
> -----------------------------------------------------------
> 
> (Updated Aug. 4, 2017, 1:25 p.m.)
> 
> 
> Review request for ranger, Don Bosco Durai, Colm O hEigeartaigh, and Qiang 
> Zhang.
> 
> 
> Bugs: RANGER-1669
>     https://issues.apache.org/jira/browse/RANGER-1669
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> ** CID 166074:  Null pointer dereferences  (NULL_RETURNS)
>     
> /hive-agent/src/main/java/org/apache/ranger/authorization/hive/authorizer/RangerHiveAuthorizer.java:
>  1474 in 
> org.apache.ranger.authorization.hive.authorizer.RangerHiveAuthorizer.showPrivileges(org.apache.hadoop.hive.ql.security.authorization.plugin.HivePrincipal,
>  
> org.apache.hadoop.hive.ql.security.authorization.plugin.HivePrivilegeObject)()
>     
>     
>     
> ________________________________________________________________________________________________________
>     *** CID 166074:  Null pointer dereferences  (NULL_RETURNS)
>     
> /hive-agent/src/main/java/org/apache/ranger/authorization/hive/authorizer/RangerHiveAuthorizer.java:
>  1474 in 
> org.apache.ranger.authorization.hive.authorizer.RangerHiveAuthorizer.showPrivileges(org.apache.hadoop.hive.ql.security.authorization.plugin.HivePrincipal,
>  
> org.apache.hadoop.hive.ql.security.authorization.plugin.HivePrivilegeObject)()
>     1468                                 .getType());
>     1469     
>     1470                 List<HiveObjectPrivilege> msObjPrivs = 
> mClient.list_privileges(
>     1471                         principalName, principalType,
>     1472                         this.getThriftHiveObjectRef(privObj));
>     1473     
>     >>>     CID 166074:  Null pointer dereferences  (NULL_RETURNS)
>     >>>     Calling a method on null object "msObjPrivs".
>     1474                 for (HiveObjectPrivilege msObjPriv : msObjPrivs) {
>     1475                     HivePrincipal resPrincipal = new HivePrincipal(
>     1476                             msObjPriv.getPrincipalName(),
>     1477                             
> AuthorizationUtils.getHivePrincipalType(msObjPriv
>     1478                                     .getPrincipalType()));
>     1479     
>     
>     ** CID 166073:  FindBugs: Bad practice  (FB.DMI_RANDOM_USED_ONLY_ONCE)
>     
> /hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java:
>  601 in org.apache.ranger.authorization.hadoop.RangerHdfsPlugin.init()()
>     
>     
>     
> ________________________________________________________________________________________________________
>     *** CID 166073:  FindBugs: Bad practice  (FB.DMI_RANDOM_USED_ONLY_ONCE)
>     
> /hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java:
>  601 in org.apache.ranger.authorization.hadoop.RangerHdfsPlugin.init()()
>     595             RangerHdfsPlugin.hadoopAuthEnabled = 
> RangerConfiguration.getInstance().getBoolean(RangerHadoopConstants.RANGER_ADD_HDFS_PERMISSION_PROP,
>  RangerHadoopConstants.RANGER_ADD_HDFS_PERMISSION_DEFAULT);
>     596             RangerHdfsPlugin.fileNameExtensionSeparator = 
> RangerConfiguration.getInstance().get(RangerHdfsAuthorizer.RANGER_FILENAME_EXTENSION_SEPARATOR_PROP,
>  RangerHdfsAuthorizer.DEFAULT_FILENAME_EXTENSION_SEPARATOR);
>     597             RangerHdfsPlugin.optimizeSubAccessAuthEnabled = 
> RangerConfiguration.getInstance().getBoolean(RangerHadoopConstants.RANGER_OPTIMIZE_SUBACCESS_AUTHORIZATION_PROP,
>  RangerHadoopConstants.RANGER_OPTIMIZE_SUBACCESS_AUTHORIZATION_DEFAULT);
>     598     
>     599             // Build random string of random length
>     600             byte[] bytes = new byte[1];
>     >>>     CID 166073:  FindBugs: Bad practice  
> (FB.DMI_RANDOM_USED_ONLY_ONCE)
>     >>>     Random object created and used only once.
>     601             new Random().nextBytes(bytes);
>     602             int count = bytes[0];
>     603             count = count < 56 ? 56 : count;
>     604             count = count > 112 ? 112 : count;
>     605     
>     606             String random = RandomStringUtils.random(count, 
> "^&#@!%()-_+=@:;'<>`~abcdefghijklmnopqrstuvwxyz01234567890");
> 
> 
> Diffs
> -----
> 
>   
> hive-agent/src/main/java/org/apache/ranger/authorization/hive/authorizer/RangerHiveAuthorizer.java
>  6872e50 
>   
> hive-agent/src/test/java/org/apache/ranger/services/hive/HIVERangerAuthorizerTest.java
>  011d2c3 
> 
> 
> Diff: https://reviews.apache.org/r/61202/diff/4/
> 
> 
> Testing
> -------
> 
> tested it
> 
> 
> Thanks,
> 
> pengjianhua
> 
>

Reply via email to