You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by mm...@apache.org on 2019/02/28 22:19:12 UTC
[accumulo] branch master updated: Fix code quality alerts found by
lgtm (#991)
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 <mm...@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;