accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mmil...@apache.org
Subject [accumulo] branch master updated: Fix code quality alerts found by lgtm (#991)
Date Thu, 28 Feb 2019 22:19:12 GMT
This is an automated email from the ASF dual-hosted git repository.

mmiller pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/master by this push:
     new 2b1c4d0  Fix code quality alerts found by lgtm (#991)
2b1c4d0 is described below

commit 2b1c4d008a7e868f0cb54f79c7b1613685b98921
Author: Mike Miller <mmiller@apache.org>
AuthorDate: Thu Feb 28 17:19:07 2019 -0500

    Fix code quality alerts found by lgtm (#991)
    
    * Improve ByteUtils and ByteUtilsTest
    * Add condition to ColumnVisibility and test to throw proper error
    * Adjust loop counter in SummarizerConfiguration to prevent ArrayIndexOutOfBounds
    * Make ColumnSet throw IllegalArgumentException
    * Remove unused js variable in vis.js
---
 .../client/summary/SummarizerConfiguration.java    |  4 ++--
 .../core/clientImpl/lexicoder/ByteUtils.java       |  4 ++++
 .../accumulo/core/iterators/conf/ColumnSet.java    | 15 +++++++++----
 .../accumulo/core/security/ColumnVisibility.java   |  3 ++-
 .../core/clientImpl/lexicoder/ByteUtilsTest.java   | 25 ++++++++++++++++++++++
 .../core/security/ColumnVisibilityTest.java        |  1 +
 .../apache/accumulo/monitor/resources/js/vis.js    |  1 -
 7 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/core/src/main/java/org/apache/accumulo/core/client/summary/SummarizerConfiguration.java
b/core/src/main/java/org/apache/accumulo/core/client/summary/SummarizerConfiguration.java
index d72fd0f..cf27e75 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/summary/SummarizerConfiguration.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/summary/SummarizerConfiguration.java
@@ -252,8 +252,8 @@ public class SummarizerConfiguration {
     public Builder addOptions(String... keyValuePairs) {
       Preconditions.checkArgument(keyValuePairs.length % 2 == 0 && keyValuePairs.length
> 0,
           "Require an even, positive number of arguments, got %s", keyValuePairs.length);
-      for (int i = 0; i < keyValuePairs.length; i += 2) {
-        addOption(keyValuePairs[i], keyValuePairs[i + 1]);
+      for (int i = 1; i < keyValuePairs.length; i += 2) {
+        addOption(keyValuePairs[i - 1], keyValuePairs[i]);
       }
       return this;
     }
diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/lexicoder/ByteUtils.java
b/core/src/main/java/org/apache/accumulo/core/clientImpl/lexicoder/ByteUtils.java
index 4d1622a..2b1faa1 100644
--- a/core/src/main/java/org/apache/accumulo/core/clientImpl/lexicoder/ByteUtils.java
+++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/lexicoder/ByteUtils.java
@@ -76,6 +76,10 @@ public class ByteUtils {
     for (int i = 0; i < in.length; i++) {
       if (in[i] == 0x01) {
         i++;
+        if (i >= in.length) {
+          throw new IllegalArgumentException("Bad bytes when attempting to unescape. Expected
"
+              + "more bytes after last byte " + String.format("x%02x", in[in.length - 1]));
+        }
         ret[index++] = (byte) (in[i] - 1);
       } else {
         ret[index++] = in[i];
diff --git a/core/src/main/java/org/apache/accumulo/core/iterators/conf/ColumnSet.java b/core/src/main/java/org/apache/accumulo/core/iterators/conf/ColumnSet.java
index 0e93026..d0a049c 100644
--- a/core/src/main/java/org/apache/accumulo/core/iterators/conf/ColumnSet.java
+++ b/core/src/main/java/org/apache/accumulo/core/iterators/conf/ColumnSet.java
@@ -146,10 +146,17 @@ public class ColumnSet {
       if (sb[i] != '%') {
         t.append(new byte[] {sb[i]}, 0, 1);
       } else {
-        byte[] hex = {sb[++i], sb[++i]};
-        String hs = new String(hex, UTF_8);
-        int b = Integer.parseInt(hs, 16);
-        t.append(new byte[] {(byte) b}, 0, 1);
+        int x = ++i;
+        int y = ++i;
+        if (y < sb.length) {
+          byte[] hex = {sb[x], sb[y]};
+          String hs = new String(hex, UTF_8);
+          int b = Integer.parseInt(hs, 16);
+          t.append(new byte[] {(byte) b}, 0, 1);
+        } else {
+          throw new IllegalArgumentException("Invalid characters in encoded string (" + s
+ ")."
+              + " Expected two characters after '%'");
+        }
       }
     }
 
diff --git a/core/src/main/java/org/apache/accumulo/core/security/ColumnVisibility.java b/core/src/main/java/org/apache/accumulo/core/security/ColumnVisibility.java
index 3898eb4..0defe24 100644
--- a/core/src/main/java/org/apache/accumulo/core/security/ColumnVisibility.java
+++ b/core/src/main/java/org/apache/accumulo/core/security/ColumnVisibility.java
@@ -384,7 +384,8 @@ public class ColumnVisibility {
             while (index < expression.length && expression[index] != '"') {
               if (expression[index] == '\\') {
                 index++;
-                if (expression[index] != '\\' && expression[index] != '"')
+                if (index == expression.length
+                    || (expression[index] != '\\' && expression[index] != '"'))
                   throw new BadArgumentException("invalid escaping within quotes",
                       new String(expression, UTF_8), index - 1);
               }
diff --git a/core/src/test/java/org/apache/accumulo/core/clientImpl/lexicoder/ByteUtilsTest.java
b/core/src/test/java/org/apache/accumulo/core/clientImpl/lexicoder/ByteUtilsTest.java
index cf044d8..81db2ee 100644
--- a/core/src/test/java/org/apache/accumulo/core/clientImpl/lexicoder/ByteUtilsTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/clientImpl/lexicoder/ByteUtilsTest.java
@@ -19,10 +19,15 @@ package org.apache.accumulo.core.clientImpl.lexicoder;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 public class ByteUtilsTest {
 
+  @Rule
+  public ExpectedException exception = ExpectedException.none();
+
   private final byte[] empty = new byte[0];
   private final byte[] noSplits = "nosplits".getBytes();
   private final byte[] splitAt5 = ("1234" + (char) 0x00 + "56789").getBytes();
@@ -46,6 +51,7 @@ public class ByteUtilsTest {
     assertArrayEquals("56789".getBytes(), result[1]);
   }
 
+  @Test
   public void testSplitWithOffset() {
     int offset;
     byte[][] result;
@@ -70,4 +76,23 @@ public class ByteUtilsTest {
     assertEquals(1, result.length);
     assertArrayEquals("5678".getBytes(), result[0]);
   }
+
+  @Test
+  public void testEscape() {
+    byte[] bytes = {0x00, 0x01};
+    byte[] escaped = ByteUtils.escape(bytes);
+    assertArrayEquals(bytes, ByteUtils.unescape(escaped));
+
+    // no escaped bytes found so returns the input
+    byte[] notEscaped = {0x02, 0x02, 0x02};
+    assertArrayEquals(notEscaped, ByteUtils.unescape(notEscaped));
+  }
+
+  @Test
+  public void testIllegalArgument() {
+    // incomplete bytes would cause an ArrayIndexOutOfBounds in the past
+    exception.expect(IllegalArgumentException.class);
+    byte[] errorBytes = {0x01};
+    ByteUtils.unescape(errorBytes);
+  }
 }
diff --git a/core/src/test/java/org/apache/accumulo/core/security/ColumnVisibilityTest.java
b/core/src/test/java/org/apache/accumulo/core/security/ColumnVisibilityTest.java
index 7a9d030..5cd86ea 100644
--- a/core/src/test/java/org/apache/accumulo/core/security/ColumnVisibilityTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/security/ColumnVisibilityTest.java
@@ -151,6 +151,7 @@ public class ColumnVisibilityTest {
     shouldThrow("\"B");
     shouldThrow("A&\"B");
     shouldThrow("A&\"B\\'");
+    shouldThrow("A&\"B\\");
 
     shouldNotThrow("\"A\"");
     shouldNotThrow("(\"A\")");
diff --git a/server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/vis.js
b/server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/vis.js
index 78a3d83..dd936e2 100644
--- a/server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/vis.js
+++ b/server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/vis.js
@@ -238,7 +238,6 @@ function getXML() {
 function drawDots() {
   requestAnimFrame(drawDots);
   var width = Math.ceil(Math.sqrt(stats.servers.length));
-  var height = Math.ceil(stats.servers.length / width);
   var x;
   var y;
   var server;


Mime
View raw message