----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68983/#review209428 -----------------------------------------------------------
agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerPathResourceMatcher.java Line 230 (original), 230 (patched) <https://reviews.apache.org/r/68983/#comment293832> Instead of returning null for empty string, consider return the incoming value - like: if (StringUtils.isEmpty(policyValue)) { return policyValue; } This will ensure no behavior changes due to current handling of null and empty values. agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerPathResourceMatcher.java Line 249 (original), 249 (patched) <https://reviews.apache.org/r/68983/#comment293830> Given getStringToCompare() was updated to handle empty values, is it necessary to handle this case in in lines #249, #251, #281, #283? Given value 'null' is treated specially in some cases, I would suggest to not make these updates. agents-common/src/main/java/org/apache/ranger/plugin/util/StringTokenReplacer.java Line 53 (original), 56 (patched) <https://reviews.apache.org/r/68983/#comment293833> Consider replacing #56 to #62 with the following - for better clarity: if (token != null) { // if next char is not the escape char or endChar, retain the escapeChar if (c != escapeChar && c != endChar) { token.append(escapeChar); } token.append(c); } else { // if next char is not the escape char or startChar, retain the escapeChar if (c != escapeChar && c != startChar) { ret.append(escapeChar); } ret.append(c); } - Madhan Neethiraj On Oct. 10, 2018, 10:13 p.m., Abhay Kulkarni wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68983/ > ----------------------------------------------------------- > > (Updated Oct. 10, 2018, 10:13 p.m.) > > > Review request for ranger, Madhan Neethiraj and Velmurugan Periasamy. > > > Bugs: RANGER-2247 > https://issues.apache.org/jira/browse/RANGER-2247 > > > Repository: ranger > > > Description > ------- > > With a hdfs policy with a single \ as a resource, Ranger plugin throws > exception. > > % hdfs dfs -ls /user/ > ls: String index out of range: -1 > > > The root cause is token-replacer code returns empty string, as \ is default > escape character for tokens. Fix is to return \ in such cases. > > > Diffs > ----- > > > agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerPathResourceMatcher.java > 78a3b8a8e > > agents-common/src/main/java/org/apache/ranger/plugin/util/StringTokenReplacer.java > ace04d65c > agents-common/src/test/resources/policyengine/test_policyengine_hdfs.json > ea167f49c > > > Diff: https://reviews.apache.org/r/68983/diff/1/ > > > Testing > ------- > > Ran all unit tests successfully > > > Thanks, > > Abhay Kulkarni > >
