Utils: Add some checks to bigIntegerToBytes().

core/src/main/java/org/bitcoinj/core/Utils.java

@@ -59,20 +59,34 @@ public class Utils {
     private static BlockingQueue<Boolean> mockSleepQueue;
 
     /**
-     * The regular {@link java.math.BigInteger#toByteArray()} method isn't quite what we often need: it appends a
-     * leading zero to indicate that the number is positive and may need padding.
-     *
+     * <p>
+     * The regular {@link java.math.BigInteger#toByteArray()} includes the sign bit of the number and 
+     * might result in an extra byte addition. This method removes this extra byte.
+     * </p>
+     * <p>
+     * Assuming only positive numbers, it's possible to discriminate if an extra byte
+     * is added by checking if the first element of the array is 0 (0000_0000).
+     * Due to the minimal representation provided by BigInteger, it means that the bit sign
+     * is the least significant bit 0000_000<b>0</b> .
+     * Otherwise the representation is not minimal.
+     * For example, if the sign bit is 0000_00<b>0</b>0, then the representation is not minimal due to the rightmost zero.
+     * </p>
      * @param b the integer to format into a byte array
      * @param numBytes the desired size of the resulting byte array
      * @return numBytes byte long array.
      */
     public static byte[] bigIntegerToBytes(BigInteger b, int numBytes) {
-        byte[] bytes = new byte[numBytes];
-        byte[] biBytes = b.toByteArray();
-        int start = (biBytes.length == numBytes + 1) ? 1 : 0;
-        int length = Math.min(biBytes.length, numBytes);
-        System.arraycopy(biBytes, start, bytes, numBytes - length, length);
-        return bytes;        
+        checkArgument(b.signum() >= 0, "b must be positive or zero");
+        checkArgument(numBytes > 0, "numBytes must be positive");
+        byte[] src = b.toByteArray();
+        byte[] dest = new byte[numBytes];
+        boolean isFirstByteOnlyForSign = src[0] == 0;
+        int length = isFirstByteOnlyForSign ? src.length - 1 : src.length;
+        checkArgument(length <= numBytes, "The given number does not fit in " + numBytes);
+        int srcPos = isFirstByteOnlyForSign ? 1 : 0;
+        int destPos = numBytes - length;
+        System.arraycopy(src, srcPos, dest, destPos, length);
+        return dest;
     }
 
     public static void uint32ToByteArrayBE(long val, byte[] out, int offset) {

core/src/test/java/org/bitcoinj/core/UtilsTest.java

@@ -1,6 +1,7 @@
 /*
  * Copyright 2011 Thilo Planz
  * Copyright 2014 Andreas Schildbach
+ * Copyright 2017 Nicola Atzei
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,6 +19,7 @@
 package org.bitcoinj.core;
 
 import java.math.BigInteger;
+import java.util.Arrays;
 import java.util.Date;
 
 import org.junit.Test;
@@ -61,4 +63,60 @@ public class UtilsTest {
         assertEquals("2014-11-16T10:54:33Z", Utils.dateTimeFormat(1416135273781L));
         assertEquals("2014-11-16T10:54:33Z", Utils.dateTimeFormat(new Date(1416135273781L)));
     }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void bigIntegerToBytes_convertNegativeNumber() {
+        BigInteger b = BigInteger.valueOf(-1);
+        Utils.bigIntegerToBytes(b, 32);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void bigIntegerToBytes_convertWithNegativeLength() {
+        BigInteger b = BigInteger.valueOf(10);
+        Utils.bigIntegerToBytes(b, -1);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void bigIntegerToBytes_convertWithZeroLength() {
+        BigInteger b = BigInteger.valueOf(10);
+        Utils.bigIntegerToBytes(b, 0);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void bigIntegerToBytes_insufficientLength() {
+        BigInteger b = BigInteger.valueOf(0b1000__0000_0000);   // base 2
+        Utils.bigIntegerToBytes(b, 1);
+    }
+
+    @Test
+    public void bigIntegerToBytes_convertZero() {
+        BigInteger b = BigInteger.valueOf(0);
+        byte[] expected = new byte[]{0b0000_0000};
+        byte[] actual = Utils.bigIntegerToBytes(b, 1);
+        assertTrue(Arrays.equals(expected, actual));
+    }
+
+    @Test
+    public void bigIntegerToBytes_singleByteSignFit() {
+        BigInteger b = BigInteger.valueOf(0b0000_1111);
+        byte[] expected = new byte[]{0b0000_1111};
+        byte[] actual = Utils.bigIntegerToBytes(b, 1);
+        assertTrue(Arrays.equals(expected, actual));
+    }
+
+    @Test
+    public void bigIntegerToBytes_paddedSingleByte() {
+        BigInteger b = BigInteger.valueOf(0b0000_1111);
+        byte[] expected = new byte[]{0, 0b0000_1111};
+        byte[] actual = Utils.bigIntegerToBytes(b, 2);
+        assertTrue(Arrays.equals(expected, actual));
+    }
+
+    @Test
+    public void bigIntegerToBytes_singleByteSignDoesNotFit() {
+        BigInteger b = BigInteger.valueOf(0b1000_0000);     // 128 (2-compl does not fit in one byte)
+        byte[] expected = new byte[]{-128};                 // -128 == 1000_0000 (compl-2)
+        byte[] actual = Utils.bigIntegerToBytes(b, 1);
+        assertTrue(Arrays.equals(expected, actual));
+    }
 }