Merge pull request #2732 from rsksmart/vovchyk/rpc-params-parsing-enh

refactor: enhance input params parsing
rsksmart · Oct 10, 2024 · d5423a9 · d5423a9
2 parents e8fae59 + be9de73
commit d5423a9
Show file tree

Hide file tree

Showing 10 changed files with 66 additions and 32 deletions.
diff --git a/rskj-core/src/main/java/co/rsk/rpc/Web3InformationRetriever.java b/rskj-core/src/main/java/co/rsk/rpc/Web3InformationRetriever.java
@@ -21,19 +21,22 @@
 import co.rsk.core.bc.AccountInformationProvider;
 import co.rsk.db.RepositoryLocator;
 import co.rsk.util.HexUtils;
+import co.rsk.util.StringUtils;
 import org.bouncycastle.util.encoders.DecoderException;
 import org.ethereum.core.Block;
 import org.ethereum.core.Blockchain;
 import org.ethereum.core.Transaction;
 import org.ethereum.core.TransactionPool;
 import org.ethereum.rpc.exception.RskJsonRpcRequestException;
+import org.ethereum.rpc.parameters.HashParam32;
+import org.ethereum.rpc.parameters.HexNumberParam;
 
 import java.util.List;
 import java.util.Optional;
 
-import static co.rsk.crypto.Keccak256.HASH_LEN;
 import static org.ethereum.rpc.exception.RskJsonRpcRequestException.blockNotFound;
 import static org.ethereum.rpc.exception.RskJsonRpcRequestException.invalidParamError;
+import static org.ethereum.rpc.parameters.HashParam32.HASH_BYTE_LENGTH;
 
 /**
  * Retrieves information requested by web3 based on the block identifier:
@@ -84,10 +87,18 @@ public Optional<Block> getBlock(String identifier) {
                 block = blockchain.getBlockByNumber(0);
                 break;
             default:
-                byte[] hash = getBlockHash(identifier);
-                block = hash.length == HASH_LEN ?
-                        blockchain.getBlockByHash(hash)
-                        : blockchain.getBlockByNumber(getBlockNumber(identifier));
+                if (HashParam32.isHash32HexLengthValid(identifier)
+                        && HexUtils.isHex(identifier, HexUtils.hasHexPrefix(identifier) ? 2 : 0)) {
+                    byte[] hash = getBlockHash(identifier);
+                    if (hash.length != HASH_BYTE_LENGTH) {
+                        throw invalidParamError(String.format("invalid block hash %s", identifier));
+                    }
+                    block = blockchain.getBlockByHash(hash);
+                } else if (HexNumberParam.isHexNumberLengthValid(identifier)) {
+                    block = blockchain.getBlockByNumber(getBlockNumber(identifier));
+                } else {
+                    throw invalidParamError(String.format("invalid block identifier %s", StringUtils.trim(identifier)));
+                }
         }
 
         return Optional.ofNullable(block);

diff --git a/rskj-core/src/main/java/co/rsk/util/HexUtils.java b/rskj-core/src/main/java/co/rsk/util/HexUtils.java
@@ -361,15 +361,4 @@ public static int jsonHexToInt(final String param) {
 
         return Integer.parseInt(preResult, 16);
     }
-
-    public static int jsonHexToIntOptionalPrefix(final String param) {
-        if (!hasHexPrefix(param) && !HexUtils.isHex(param)) {
-            throw invalidParamError(INCORRECT_HEX_SYNTAX);
-        }
-
-        String preResult = removeHexPrefix(param);
-
-        return Integer.parseInt(preResult, 16);
-    }
-
 }
diff --git a/rskj-core/src/main/java/co/rsk/util/StringUtils.java b/rskj-core/src/main/java/co/rsk/util/StringUtils.java
@@ -22,7 +22,7 @@
 
 public class StringUtils {
 
-    private static final int DEFAULT_MAX_LEN = 64;
+    private static final int DEFAULT_MAX_LEN = 66; // 0x + 32 bytes, where each byte is represented by 2 hex characters
 
     private StringUtils() { /* hidden */ }
 

diff --git a/rskj-core/src/main/java/org/ethereum/rpc/parameters/HashParam32.java b/rskj-core/src/main/java/org/ethereum/rpc/parameters/HashParam32.java
@@ -24,12 +24,15 @@
 import static co.rsk.util.HexUtils.stringHexToByteArray;
 
 public abstract class HashParam32 {
-    private static final int HASH_BYTE_LENGTH = 32;
+    public static final int HASH_BYTE_LENGTH = Keccak256.HASH_LEN;
+    public static final int MIN_HASH_HEX_LEN = 2 * HASH_BYTE_LENGTH; // 2 hex characters per byte
+    public static final int MAX_HASH_HEX_LEN = MIN_HASH_HEX_LEN + 2; // 2 bytes for 0x prefix
+
     private final Keccak256 hash;
 
     HashParam32(String hashType, String hash) {
-        if (hash == null || hash.isEmpty()) {
-            throw RskJsonRpcRequestException.invalidParamError("Invalid " + hashType + ": empty or null.");
+        if (!isHash32HexLengthValid(hash)) {
+            throw RskJsonRpcRequestException.invalidParamError("Invalid " + hashType + ": incorrect length.");
         }
 
         byte[] hashBytes;
@@ -50,4 +53,8 @@ public abstract class HashParam32 {
     public Keccak256 getHash() {
         return hash;
     }
+
+    public static boolean isHash32HexLengthValid(String hex) {
+        return hex != null && hex.length() >= MIN_HASH_HEX_LEN && hex.length() <= MAX_HASH_HEX_LEN;
+    }
 }
diff --git a/rskj-core/src/main/java/org/ethereum/rpc/parameters/HexAddressParam.java b/rskj-core/src/main/java/org/ethereum/rpc/parameters/HexAddressParam.java
@@ -31,11 +31,15 @@
 public class HexAddressParam implements Serializable {
     private static final long serialVersionUID = 1L;
 
+    public static final int HEX_ADDR_BYTE_LENGTH = RskAddress.LENGTH_IN_BYTES;
+    public static final int MIN_HEX_ADDR_LEN = 2 * HEX_ADDR_BYTE_LENGTH; // 2 hex characters per byte
+    public static final int MAX_HEX_ADDR_LEN = MIN_HEX_ADDR_LEN + 2; // 2 bytes for 0x prefix
+
     private transient final RskAddress address;
 
     public HexAddressParam(String hexAddress) {
-        if (hexAddress == null || hexAddress.isEmpty()) {
-            throw RskJsonRpcRequestException.invalidParamError("Invalid address: empty or null.");
+        if (!isHexAddressLengthValid(hexAddress)) {
+            throw RskJsonRpcRequestException.invalidParamError("Invalid address: null, empty or invalid hex value.");
         }
 
         try {
@@ -53,6 +57,10 @@ public RskAddress getAddress() {
     public String toString() {
         return address.toString();
     }
+
+    public static boolean isHexAddressLengthValid(String hex) {
+        return hex != null && hex.length() >= MIN_HEX_ADDR_LEN && hex.length() <= MAX_HEX_ADDR_LEN;
+    }
 
     public static class Deserializer extends StdDeserializer<HexAddressParam> {
         private static final long serialVersionUID = 1L;

diff --git a/rskj-core/src/main/java/org/ethereum/rpc/parameters/HexNumberParam.java b/rskj-core/src/main/java/org/ethereum/rpc/parameters/HexNumberParam.java
@@ -18,6 +18,7 @@
 package org.ethereum.rpc.parameters;
 
 import co.rsk.util.HexUtils;
+import co.rsk.util.StringUtils;
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.databind.DeserializationContext;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
@@ -32,9 +33,16 @@
 public class HexNumberParam implements Serializable {
     private static final long serialVersionUID = 1L;
 
+    public static final int HEX_NUM_BYTE_LENGTH = 32;
+    public static final int MAX_HEX_NUM_LEN = 2 + 2 * HEX_NUM_BYTE_LENGTH; // 2 bytes for 0x prefix; 2 hex characters per byte
+
     private final String hexNumber;
 
     public HexNumberParam(String hexNumber) {
+        if (!isHexNumberLengthValid(hexNumber)) {
+            throw RskJsonRpcRequestException.invalidParamError("Invalid param: " + StringUtils.trim(hexNumber));
+        }
+
         boolean hasPrefix = HexUtils.hasHexPrefix(hexNumber);
         if (!HexUtils.isHex(hexNumber.toLowerCase(), hasPrefix ? 2 : 0)) {
             try {
@@ -56,6 +64,10 @@ public String toString() {
         return this.hexNumber;
     }
 
+    public static boolean isHexNumberLengthValid(String hex) {
+        return hex != null && hex.length() <= MAX_HEX_NUM_LEN;
+    }
+
     public static class Deserializer extends StdDeserializer<HexNumberParam> {
         private static final long serialVersionUID = 1L;
 

diff --git a/rskj-core/src/main/java/org/ethereum/rpc/parameters/HexStringParam.java b/rskj-core/src/main/java/org/ethereum/rpc/parameters/HexStringParam.java
@@ -18,14 +18,21 @@
 package org.ethereum.rpc.parameters;
 
 import co.rsk.util.HexUtils;
+import co.rsk.util.StringUtils;
 import org.ethereum.rpc.exception.RskJsonRpcRequestException;
 
+import static org.ethereum.rpc.parameters.HexNumberParam.isHexNumberLengthValid;
+
 public abstract class HexStringParam {
     HexStringParam(String hexString) {
-        if(hexString.isEmpty()) {
+        if (hexString.isEmpty()) {
             return;
         }
 
+        if (!isHexNumberLengthValid(hexString)) {
+            throw RskJsonRpcRequestException.invalidParamError("Invalid argument: " + StringUtils.trim(hexString));
+        }
+
         if (!HexUtils.hasHexPrefix(hexString) || !HexUtils.isHex(hexString,2)) {
             throw RskJsonRpcRequestException.invalidParamError(String.format("Invalid argument \"%s\": param should be a hex value string.", hexString));
         }

diff --git a/rskj-core/src/test/java/co/rsk/pcc/bto/ExtractPublicKeyFromExtendedPublicKeyTest.java b/rskj-core/src/test/java/co/rsk/pcc/bto/ExtractPublicKeyFromExtendedPublicKeyTest.java
@@ -91,7 +91,7 @@ void failsUponInvalidPublicKey() {
             });
             Assertions.fail();
         } catch (NativeContractIllegalArgumentException e) {
-            Assertions.assertEquals("Invalid extended public key 'tpubD6NzVbkrYhZ4YHQqwWz3Tm1ESZ9AidobeyLG4mEezB6hN8gFFWrcjczyF77L...'", e.getMessage());
+            Assertions.assertEquals("Invalid extended public key 'tpubD6NzVbkrYhZ4YHQqwWz3Tm1ESZ9AidobeyLG4mEezB6hN8gFFWrcjczyF77Lw3...'", e.getMessage());
         }
     }
 

diff --git a/rskj-core/src/test/java/co/rsk/pcc/bto/HDWalletUtilsHelperTest.java b/rskj-core/src/test/java/co/rsk/pcc/bto/HDWalletUtilsHelperTest.java
@@ -63,7 +63,7 @@ void validateAndExtractNetworkFromExtendedPublicKeyWithInvalidXpub() {
     void validateAndExtractNetworkFromExtendedPublicKeyWithInvalidLongXpub() {
         NativeContractIllegalArgumentException exception = Assertions.assertThrows(
                 NativeContractIllegalArgumentException.class,
-                () -> helper.validateAndExtractNetworkFromExtendedPublicKey("completelyInvalidLongLongLongLongLongLongLongLongLongLongLonStuff"));
-        assertEquals("Invalid extended public key 'completelyInvalidLongLongLongLongLongLongLongLongLongLongLonStuf...'", exception.getMessage());
+                () -> helper.validateAndExtractNetworkFromExtendedPublicKey("completelyInvalidLongLongLongLongLongLongLongLongLongLongLongLStuff"));
+        assertEquals("Invalid extended public key 'completelyInvalidLongLongLongLongLongLongLongLongLongLongLongLStuf...'", exception.getMessage());
     }
 }
diff --git a/rskj-core/src/test/java/co/rsk/util/StringUtilsTest.java b/rskj-core/src/test/java/co/rsk/util/StringUtilsTest.java
@@ -24,20 +24,20 @@
 
 class StringUtilsTest {
 
-    private static final String STR_64_CHARS = "ee5c851e70650111887bb6c04e18ef4353391abe37846234c17895a9ca2b33d5";
-    private static final String STR_65_CHARS = STR_64_CHARS + "a";
+    private static final String STR_66_CHARS = "0xee5c851e70650111887bb6c04e18ef4353391abe37846234c17895a9ca2b33d5";
+    private static final String STR_67_CHARS = STR_66_CHARS + "a";
 
     @Test
     void testTrim() {
         assertNull(StringUtils.trim(null));
         assertEquals("", StringUtils.trim(""));
         assertEquals("a", StringUtils.trim("a"));
 
-        assertEquals(64, STR_64_CHARS.length());
-        assertEquals(65, STR_65_CHARS.length());
+        assertEquals(66, STR_66_CHARS.length());
+        assertEquals(67, STR_67_CHARS.length());
 
-        assertEquals(STR_64_CHARS, StringUtils.trim(STR_64_CHARS));
-        assertEquals(STR_64_CHARS + "...", StringUtils.trim(STR_65_CHARS));
+        assertEquals(STR_66_CHARS, StringUtils.trim(STR_66_CHARS));
+        assertEquals(STR_66_CHARS + "...", StringUtils.trim(STR_67_CHARS));
     }
 
     @Test