[accumulo] branch 1.9 updated: Fix array out of bounds in FastFormat (#562)

[kt...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/1.9 by this push:
     new a7aee79  Fix array out of bounds in FastFormat (#562)
a7aee79 is described below

commit a7aee79b986cc7f8c91ff8cf933525aa76718e80
Author: Keith Turner <ke...@deenlo.com>
AuthorDate: Tue Jul 17 09:42:32 2018 -0400

    Fix array out of bounds in FastFormat (#562)
---
 .../org/apache/accumulo/core/util/FastFormat.java  |  26 +++--
 .../apache/accumulo/core/util/FastFormatTest.java  | 114 +++++++++++++++++++++
 2 files changed, 132 insertions(+), 8 deletions(-)

diff --git a/core/src/main/java/org/apache/accumulo/core/util/FastFormat.java b/core/src/main/java/org/apache/accumulo/core/util/FastFormat.java
index ceae8d5..54169c3 100644
--- a/core/src/main/java/org/apache/accumulo/core/util/FastFormat.java
+++ b/core/src/main/java/org/apache/accumulo/core/util/FastFormat.java
@@ -18,11 +18,16 @@ package org.apache.accumulo.core.util;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 
+import com.google.common.base.Preconditions;
+
 public class FastFormat {
+
   // this 7 to 8 times faster than String.format("%s%06d",prefix, num)
   public static byte[] toZeroPaddedString(long num, int width, int radix, byte[] prefix) {
-    byte ret[] = new byte[width + prefix.length];
-    if (toZeroPaddedString(ret, 0, num, width, radix, prefix) != ret.length)
+    Preconditions.checkArgument(num >= 0);
+    String strNum = Long.toString(num, radix);
+    byte ret[] = new byte[Math.max(strNum.length(), width) + prefix.length];
+    if (toZeroPaddedString(ret, 0, strNum, width, prefix) != ret.length)
       throw new RuntimeException(" Did not format to expected width " + num + " " + width + " "
           + radix + " " + new String(prefix, UTF_8));
     return ret;
@@ -30,10 +35,15 @@ public class FastFormat {
 
   public static int toZeroPaddedString(byte output[], int outputOffset, long num, int width,
       int radix, byte[] prefix) {
-    if (num < 0)
-      throw new IllegalArgumentException();
+    Preconditions.checkArgument(num >= 0);
+
+    String strNum = Long.toString(num, radix);
+
+    return toZeroPaddedString(output, outputOffset, strNum, width, prefix);
+  }
 
-    String s = Long.toString(num, radix);
+  private static int toZeroPaddedString(byte output[], int outputOffset, String strNum, int width,
+      byte[] prefix) {
 
     int index = outputOffset;
 
@@ -41,13 +51,13 @@ public class FastFormat {
       output[index++] = prefix[i];
     }
 
-    int end = width - s.length() + index;
+    int end = width - strNum.length() + index;
 
     while (index < end)
       output[index++] = '0';
 
-    for (int i = 0; i < s.length(); i++) {
-      output[index++] = (byte) s.charAt(i);
+    for (int i = 0; i < strNum.length(); i++) {
+      output[index++] = (byte) strNum.charAt(i);
     }
 
     return index - outputOffset;
diff --git a/core/src/test/java/org/apache/accumulo/core/util/FastFormatTest.java b/core/src/test/java/org/apache/accumulo/core/util/FastFormatTest.java
new file mode 100644
index 0000000..8d510fe
--- /dev/null
+++ b/core/src/test/java/org/apache/accumulo/core/util/FastFormatTest.java
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.accumulo.core.util;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import java.util.Arrays;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class FastFormatTest {
+
+  @Test
+  public void testArrayOffset() {
+
+    byte[] str = new byte[8];
+
+    Arrays.fill(str, (byte) '-');
+    int len = FastFormat.toZeroPaddedString(str, 4, 64L, 1, 16, new byte[] {});
+    Assert.assertEquals(2, len);
+    Assert.assertEquals("----40--", new String(str, UTF_8));
+
+    Arrays.fill(str, (byte) '-');
+    len = FastFormat.toZeroPaddedString(str, 4, 64L, 2, 16, new byte[] {});
+    Assert.assertEquals(2, len);
+    Assert.assertEquals("----40--", new String(str, UTF_8));
+
+    Arrays.fill(str, (byte) '-');
+    len = FastFormat.toZeroPaddedString(str, 4, 64L, 3, 16, new byte[] {});
+    Assert.assertEquals(3, len);
+    Assert.assertEquals("----040-", new String(str, UTF_8));
+
+    Arrays.fill(str, (byte) '-');
+    len = FastFormat.toZeroPaddedString(str, 4, 64L, 1, 16, new byte[] {'P'});
+    Assert.assertEquals(3, len);
+    Assert.assertEquals("----P40-", new String(str, UTF_8));
+
+    Arrays.fill(str, (byte) '-');
+    len = FastFormat.toZeroPaddedString(str, 4, 64L, 2, 16, new byte[] {'P'});
+    Assert.assertEquals(3, len);
+    Assert.assertEquals("----P40-", new String(str, UTF_8));
+
+    Arrays.fill(str, (byte) '-');
+    len = FastFormat.toZeroPaddedString(str, 4, 64L, 3, 16, new byte[] {'P'});
+    Assert.assertEquals(4, len);
+    Assert.assertEquals("----P040", new String(str, UTF_8));
+
+    Arrays.fill(str, (byte) '-');
+    len = FastFormat.toZeroPaddedString(str, 2, 64L, 4, 16, new byte[] {'P'});
+    Assert.assertEquals(5, len);
+    Assert.assertEquals("--P0040-", new String(str, UTF_8));
+  }
+
+  @Test
+  public void testFormat() {
+    Assert.assertEquals("100",
+        new String(FastFormat.toZeroPaddedString(1296, 1, 36, new byte[] {}), UTF_8));
+    Assert.assertEquals("100",
+        new String(FastFormat.toZeroPaddedString(1296, 2, 36, new byte[] {}), UTF_8));
+    Assert.assertEquals("100",
+        new String(FastFormat.toZeroPaddedString(1296, 3, 36, new byte[] {}), UTF_8));
+    Assert.assertEquals("0100",
+        new String(FastFormat.toZeroPaddedString(1296, 4, 36, new byte[] {}), UTF_8));
+    Assert.assertEquals("00100",
+        new String(FastFormat.toZeroPaddedString(1296, 5, 36, new byte[] {}), UTF_8));
+
+    Assert.assertEquals("PA100",
+        new String(FastFormat.toZeroPaddedString(1296, 1, 36, new byte[] {'P', 'A'}), UTF_8));
+    Assert.assertEquals("PA100",
+        new String(FastFormat.toZeroPaddedString(1296, 2, 36, new byte[] {'P', 'A'}), UTF_8));
+    Assert.assertEquals("PA100",
+        new String(FastFormat.toZeroPaddedString(1296, 3, 36, new byte[] {'P', 'A'}), UTF_8));
+    Assert.assertEquals("PA0100",
+        new String(FastFormat.toZeroPaddedString(1296, 4, 36, new byte[] {'P', 'A'}), UTF_8));
+    Assert.assertEquals("PA00100",
+        new String(FastFormat.toZeroPaddedString(1296, 5, 36, new byte[] {'P', 'A'}), UTF_8));
+    Assert.assertEquals("PA000100",
+        new String(FastFormat.toZeroPaddedString(1296, 6, 36, new byte[] {'P', 'A'}), UTF_8));
+    Assert.assertEquals("PA0000100",
+        new String(FastFormat.toZeroPaddedString(1296, 7, 36, new byte[] {'P', 'A'}), UTF_8));
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void testNegative1() {
+    FastFormat.toZeroPaddedString(-5, 1, 36, new byte[] {});
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void testNegative2() {
+    byte[] str = new byte[8];
+    FastFormat.toZeroPaddedString(str, 0, -5, 1, 36, new byte[] {});
+  }
+
+  @Test(expected = ArrayIndexOutOfBoundsException.class)
+  public void testArrayOutOfBounds() {
+    byte[] str = new byte[8];
+    FastFormat.toZeroPaddedString(str, 4, 64L, 4, 16, new byte[] {'P'});
+  }
+}