From commits-return-22669-archive-asf-public=cust-asf.ponee.io@accumulo.apache.org Thu Feb 28 22:19:14 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id D0EDB180657 for ; Thu, 28 Feb 2019 23:19:13 +0100 (CET) Received: (qmail 72169 invoked by uid 500); 28 Feb 2019 22:19:12 -0000 Mailing-List: contact commits-help@accumulo.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@accumulo.apache.org Delivered-To: mailing list commits@accumulo.apache.org Received: (qmail 72160 invoked by uid 99); 28 Feb 2019 22:19:12 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 28 Feb 2019 22:19:12 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id 3E86B82E8A; Thu, 28 Feb 2019 22:19:12 +0000 (UTC) Date: Thu, 28 Feb 2019 22:19:12 +0000 To: "commits@accumulo.apache.org" Subject: [accumulo] branch master updated: Fix code quality alerts found by lgtm (#991) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Message-ID: <155139235214.2113.5164897275889589672@gitbox.apache.org> From: mmiller@apache.org X-Git-Host: gitbox.apache.org X-Git-Repo: accumulo X-Git-Refname: refs/heads/master X-Git-Reftype: branch X-Git-Oldrev: bee584bcb689a6b9394a3cea30486c6c3f43c49e X-Git-Newrev: 2b1c4d008a7e868f0cb54f79c7b1613685b98921 X-Git-Rev: 2b1c4d008a7e868f0cb54f79c7b1613685b98921 X-Git-NotificationType: ref_changed_plus_diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated 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 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;