Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

[MINOR] added fromHexStringStrict to check for exactly 20 byte addresses #1128

Merged
merged 14 commits into from
Mar 21, 2019
Merged

[MINOR] added fromHexStringStrict to check for exactly 20 byte addresses #1128

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
6b04fae
added fromHexStringStrict to check for exactly 20 byte addresses
macfarla Mar 19, 2019
8d1cfd1
Merge branch 'master' of github.com:PegaSysEng/pantheon into cli-stri…
macfarla Mar 19, 2019
5cb8d2f
IllegalArgumentException if null
macfarla Mar 19, 2019
a0bc882
Merge branch 'master' into cli-strict-address
macfarla Mar 19, 2019
581667d
Merge branch 'master' into cli-strict-address
macfarla Mar 20, 2019
ddcda8b
Merge branch 'master' of github.com:PegaSysEng/pantheon into cli-stri…
macfarla Mar 20, 2019
6e20fd2
formatting
macfarla Mar 20, 2019
807022b
throw IllegalArgumentException instead of returning null
macfarla Mar 20, 2019
05b78ee
Merge branch 'cli-strict-address' of github.com:macfarla/pantheon int…
macfarla Mar 20, 2019
c8e378a
Merge branch 'master' of github.com:PegaSysEng/pantheon into cli-stri…
macfarla Mar 20, 2019
eb1ece3
return null if address is null
macfarla Mar 20, 2019
c175cdc
fixed javadoc
macfarla Mar 20, 2019
623b41c
Merge branch 'master' of github.com:PegaSysEng/pantheon into cli-stri…
macfarla Mar 20, 2019
c615609
Merge branch 'master' into cli-strict-address
macfarla Mar 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Address.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you return null rather than throw a null pointer exception when the hexstring is null

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and thinking further probably better to throw an illegal argument exception from checkArgument rather than a null pointer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is also true for the existing fromHexString method. Do I leave that one as is?

* @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);
Expand Down
5 changes: 5 additions & 0 deletions ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/AddressTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
2 changes: 1 addition & 1 deletion pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
13 changes: 12 additions & 1 deletion pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,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() {

Expand Down Expand Up @@ -1783,7 +1794,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<MiningParameters> miningArg =
Expand Down