----------------------------------------------------------- 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 > >
