From 6b04fae89eb72e907092edfd1b16ffd6f821868c Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Tue, 19 Mar 2019 12:33:04 +1000 Subject: [PATCH 1/6] added fromHexStringStrict to check for exactly 20 byte addresses --- .../pegasys/pantheon/ethereum/core/Address.java | 15 +++++++++++++++ .../pegasys/pantheon/ethereum/vm/AddressTest.java | 5 +++++ .../pegasys/pantheon/cli/PantheonCommand.java | 2 +- .../pegasys/pantheon/cli/PantheonCommandTest.java | 13 ++++++++++++- 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Address.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Address.java index 877c19f534..6ea77dbcc2 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Address.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Address.java @@ -96,6 +96,21 @@ public static Address fromHexString(final String str) { return new Address(BytesValue.fromHexStringLenient(str, SIZE)); } + /** + * Parse an hexadecimal string representing an account address. + * + * @param str An hexadecimal string representing a valid account address (strictly 20 bytes). + * @return The parsed address. + * @throws NullPointerException if the provided string is {@code null}. + * @throws IllegalArgumentException if the string is either not hexadecimal, or not the valid + * representation of a 20 byte address. + */ + public static Address fromHexStringStrict(final String str) { + if (str == null) return null; + + return new Address(BytesValue.fromHexString(str)); + } + private static Address precompiled(final int value) { // Keep it simple while we don't need precompiled above 127. checkArgument(value < Byte.MAX_VALUE); diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/AddressTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/AddressTest.java index 2aa4354d44..f69d10fa59 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/AddressTest.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/AddressTest.java @@ -51,4 +51,9 @@ public void accountAddresHashCode() { public void invalidAccountAddress() { Address.wrap(BytesValue.fromHexString("0x00101010")); } + + @Test(expected = IllegalArgumentException.class) + public void tooShortAccountAddress() { + Address.fromHexStringStrict("0x00101010"); + } } diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java index 4256785010..da8fb34e00 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java @@ -564,7 +564,7 @@ public void parse( commandLine.addSubcommand( RLPSubCommand.COMMAND_NAME, new RLPSubCommand(resultHandler.out(), in)); - commandLine.registerConverter(Address.class, Address::fromHexString); + commandLine.registerConverter(Address.class, Address::fromHexStringStrict); commandLine.registerConverter(BytesValue.class, BytesValue::fromHexString); commandLine.registerConverter(Level.class, Level::valueOf); commandLine.registerConverter(SyncMode.class, SyncMode::fromString); diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java index e0f713391a..6be14f0da1 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java @@ -342,6 +342,17 @@ public void permissionsEnabledWithInvalidContractAddressMustError() { assertThat(commandOutput.toString()).isEmpty(); } + @Test + public void permissionsEnabledWithTooShortContractAddressMustError() { + parseCommand( + "--permissions-nodes-contract-enabled", "--permissions-nodes-contract-address", "0x1234"); + + verifyZeroInteractions(mockRunnerBuilder); + + assertThat(commandErrorOutput.toString()).contains("Invalid value"); + assertThat(commandOutput.toString()).isEmpty(); + } + @Test public void permissionsSmartContractMustUseOption() { @@ -1702,7 +1713,7 @@ public void pantheonDoesNotStartInMiningModeIfCoinbaseNotSet() { @Test public void miningIsEnabledWhenSpecified() throws Exception { - final String coinbaseStr = String.format("%020x", 1); + final String coinbaseStr = String.format("%040x", 1); parseCommand("--miner-enabled", "--miner-coinbase=" + coinbaseStr); final ArgumentCaptor miningArg = From 5cb8d2fc847bc083da46f3dc3880492ffa1cbf98 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Tue, 19 Mar 2019 16:00:22 +1000 Subject: [PATCH 2/6] IllegalArgumentException if null --- .../java/tech/pegasys/pantheon/ethereum/core/Address.java | 4 ++-- .../java/tech/pegasys/pantheon/ethereum/vm/AddressTest.java | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Address.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Address.java index 6ea77dbcc2..62a90fef08 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Address.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Address.java @@ -101,12 +101,12 @@ public static Address fromHexString(final String str) { * * @param str An hexadecimal string representing a valid account address (strictly 20 bytes). * @return The parsed address. - * @throws NullPointerException if the provided string is {@code null}. + * @throws IllegalArgumentException if the provided string is {@code null}. * @throws IllegalArgumentException if the string is either not hexadecimal, or not the valid * representation of a 20 byte address. */ public static Address fromHexStringStrict(final String str) { - if (str == null) return null; + checkArgument(str != null); return new Address(BytesValue.fromHexString(str)); } diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/AddressTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/AddressTest.java index f69d10fa59..5e8db03e9a 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/AddressTest.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/AddressTest.java @@ -56,4 +56,8 @@ public void invalidAccountAddress() { public void tooShortAccountAddress() { Address.fromHexStringStrict("0x00101010"); } + @Test(expected = IllegalArgumentException.class) + public void nullAccountAddress() { + Address.fromHexStringStrict(null); + } } From 6e20fd228bbc0c0825fd779f04d281c6a980badd Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Wed, 20 Mar 2019 11:18:29 +1000 Subject: [PATCH 3/6] formatting --- .../test/java/tech/pegasys/pantheon/ethereum/vm/AddressTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/AddressTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/AddressTest.java index 5e8db03e9a..2aff174be1 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/AddressTest.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/AddressTest.java @@ -56,6 +56,7 @@ public void invalidAccountAddress() { public void tooShortAccountAddress() { Address.fromHexStringStrict("0x00101010"); } + @Test(expected = IllegalArgumentException.class) public void nullAccountAddress() { Address.fromHexStringStrict(null); From 807022bcf80de34afd4724136e5c74e38acec2c9 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Wed, 20 Mar 2019 11:27:18 +1000 Subject: [PATCH 4/6] throw IllegalArgumentException instead of returning null --- .../java/tech/pegasys/pantheon/ethereum/core/Address.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Address.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Address.java index 62a90fef08..0459f8e6ec 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Address.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Address.java @@ -85,13 +85,13 @@ public static Address extract(final Hash hash) { * @param str An hexadecimal string (with or without the leading '0x') representing a valid * account address. * @return The parsed address. - * @throws NullPointerException if the provided string is {@code null}. + * @throws IllegalArgumentException if the provided string is {@code null}. * @throws IllegalArgumentException if the string is either not hexadecimal, or not the valid * representation of an address. */ @JsonCreator public static Address fromHexString(final String str) { - if (str == null) return null; + checkArgument(str != null); return new Address(BytesValue.fromHexStringLenient(str, SIZE)); } From eb1ece32a06b60cef140bb9efe117a35795f2ac7 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Wed, 20 Mar 2019 12:53:43 +1000 Subject: [PATCH 5/6] return null if address is null --- .../java/tech/pegasys/pantheon/ethereum/core/Address.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Address.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Address.java index 0459f8e6ec..dd2f2a616d 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Address.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Address.java @@ -85,13 +85,13 @@ public static Address extract(final Hash hash) { * @param str An hexadecimal string (with or without the leading '0x') representing a valid * account address. * @return The parsed address. - * @throws IllegalArgumentException if the provided string is {@code null}. + * @return The parsed address: {@code null} if the provided string is {@code null}. * @throws IllegalArgumentException if the string is either not hexadecimal, or not the valid * representation of an address. */ @JsonCreator public static Address fromHexString(final String str) { - checkArgument(str != null); + if (str == null) return null; return new Address(BytesValue.fromHexStringLenient(str, SIZE)); } From c175cdcb6b11106d676125d843110fbc79d71d36 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Wed, 20 Mar 2019 13:25:40 +1000 Subject: [PATCH 6/6] fixed javadoc --- .../main/java/tech/pegasys/pantheon/ethereum/core/Address.java | 1 - 1 file changed, 1 deletion(-) diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Address.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Address.java index dd2f2a616d..7ee773b860 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Address.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Address.java @@ -84,7 +84,6 @@ public static Address extract(final Hash hash) { * * @param str An hexadecimal string (with or without the leading '0x') representing a valid * account address. - * @return The parsed address. * @return The parsed address: {@code null} if the provided string is {@code null}. * @throws IllegalArgumentException if the string is either not hexadecimal, or not the valid * representation of an address.