Skip to content

Commit 19eef23

Browse files
Gabriel-TrintinaliaGabriel-Trintinalia
authored
Fix issues with RLPExceptions (#5140)
Signed-off-by: Gabriel-Trintinalia <[email protected]>
1 parent 81e7d5d commit 19eef23

File tree

8 files changed

+122
-58
lines changed

8 files changed

+122
-58
lines changed

‎ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/BlockBody.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,19 @@ public void writeTo(final RLPOutput output) {
9797

9898
public static BlockBody readFrom(
9999
final RLPInput input, final BlockHeaderFunctions blockHeaderFunctions) {
100+
return readFrom(input, blockHeaderFunctions, false);
101+
}
102+
103+
public static BlockBody readFrom(
104+
final RLPInput input,
105+
final BlockHeaderFunctions blockHeaderFunctions,
106+
final boolean allowEmptyBody) {
100107
input.enterList();
108+
if (input.isEndOfCurrentList() && allowEmptyBody) {
109+
// empty block [] -> Return empty body.
110+
input.leaveList();
111+
return empty();
112+
}
101113
// TODO: Support multiple hard fork transaction formats.
102114
final BlockBody body =
103115
new BlockBody(
@@ -125,6 +137,10 @@ public int hashCode() {
125137
return Objects.hash(transactions, ommers, withdrawals);
126138
}
127139

140+
public boolean isEmpty() {
141+
return transactions.isEmpty() && ommers.isEmpty() && withdrawals.isEmpty();
142+
}
143+
128144
@Override
129145
public String toString() {
130146
return "BlockBody{"

‎ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/encoding/TransactionAnnouncementDecoder.java‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.hyperledger.besu.ethereum.rlp.RLPInput;
2323
import org.hyperledger.besu.plugin.data.TransactionType;
2424

25+
import java.util.ArrayList;
2526
import java.util.List;
2627
import java.util.stream.Collectors;
2728

@@ -69,13 +70,14 @@ private static List<TransactionAnnouncement> decodeForEth66(final RLPInput input
6970
*/
7071
private static List<TransactionAnnouncement> decodeForEth68(final RLPInput input) {
7172
input.enterList();
72-
final List<TransactionType> types =
73-
input.readList(
74-
rlp -> {
75-
final int type = rlp.readByte() & 0xff;
76-
return type == 0 ? TransactionType.FRONTIER : TransactionType.of(type);
77-
});
78-
final List<Long> sizes = input.readList(RLPInput::readUnsignedInt);
73+
74+
final List<TransactionType> types = new ArrayList<>();
75+
final byte[] bytes = input.readBytes().toArray();
76+
for (final byte b : bytes) {
77+
types.add(b == 0 ? TransactionType.FRONTIER : TransactionType.of(b));
78+
}
79+
80+
final List<Long> sizes = input.readList(in -> (long) in.readBytes().toInt());
7981
final List<Hash> hashes = input.readList(rlp -> Hash.wrap(rlp.readBytes32()));
8082
input.leaveList();
8183
if (!(types.size() == hashes.size() && hashes.size() == sizes.size())) {

‎ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/encoding/TransactionAnnouncementEncoder.java‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -76,30 +76,42 @@ private static Bytes encodeForEth66(final List<Transaction> transactions) {
7676
*/
7777
private static Bytes encodeForEth68(final List<Transaction> transactions) {
7878
final List<Integer> sizes = new ArrayList<>(transactions.size());
79-
final List<TransactionType> types = new ArrayList<>(transactions.size());
79+
final byte[] types = new byte[transactions.size()];
8080
final List<Hash> hashes = new ArrayList<>(transactions.size());
81-
transactions.forEach(
82-
transaction -> {
83-
types.add(transaction.getType());
84-
sizes.add(transaction.getSize());
85-
hashes.add(transaction.getHash());
86-
});
81+
82+
for (int i = 0; i < transactions.size(); i++) {
83+
final TransactionType type = transactions.get(i).getType();
84+
types[i] = type == TransactionType.FRONTIER ? 0x00 : type.getSerializedType();
85+
sizes.add(transactions.get(i).getSize());
86+
hashes.add(transactions.get(i).getHash());
87+
}
8788

8889
return encodeForEth68(types, sizes, hashes);
8990
}
9091

9192
@VisibleForTesting
9293
public static Bytes encodeForEth68(
9394
final List<TransactionType> types, final List<Integer> sizes, final List<Hash> hashes) {
95+
96+
final byte[] byteTypes = new byte[types.size()];
97+
for (int i = 0; i < types.size(); i++) {
98+
final TransactionType type = types.get(i);
99+
byteTypes[i] = type == TransactionType.FRONTIER ? 0x00 : type.getSerializedType();
100+
}
101+
return encodeForEth68(byteTypes, sizes, hashes);
102+
}
103+
104+
@VisibleForTesting
105+
public static Bytes encodeForEth68(
106+
final byte[] types, final List<Integer> sizes, final List<Hash> hashes) {
94107
final BytesValueRLPOutput out = new BytesValueRLPOutput();
95108
// Check if lists have the same size
96-
if (!(types.size() == hashes.size() && hashes.size() == sizes.size())) {
109+
if (!(types.length == hashes.size() && hashes.size() == sizes.size())) {
97110
throw new IllegalArgumentException(
98111
"Hashes, sizes and types must have the same number of elements");
99112
}
100113
out.startList();
101-
out.writeList(
102-
types, (h, w) -> w.writeByte(h == TransactionType.FRONTIER ? 0x00 : h.getSerializedType()));
114+
out.writeBytes(Bytes.wrap((types)));
103115
out.writeList(sizes, (h, w) -> w.writeInt(h));
104116
out.writeList(hashes, (h, w) -> w.writeBytes(h));
105117
out.endList();

‎ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractPeerRequestTask.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ private void handleMessage(
109109
} catch (final RLPException e) {
110110
// Peer sent us malformed data - disconnect
111111
LOG.debug("Disconnecting with BREACH_OF_PROTOCOL due to malformed message: {}", peer, e);
112+
LOG.trace("Message data: {}", message.getData());
112113
peer.disconnect(DisconnectReason.BREACH_OF_PROTOCOL);
113114
promise.completeExceptionally(new PeerBreachedProtocolException());
114115
}

‎ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/messages/BlockBodiesMessage.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,6 @@ public List<BlockBody> bodies(final ProtocolSchedule protocolSchedule) {
7171
final BlockHeaderFunctions blockHeaderFunctions =
7272
ScheduleBasedBlockHeaderFunctions.create(protocolSchedule);
7373
return new BytesValueRLPInput(data, false)
74-
.readList(rlp -> BlockBody.readFrom(rlp, blockHeaderFunctions));
74+
.readList(rlp -> BlockBody.readFrom(rlp, blockHeaderFunctions, true));
7575
}
7676
}

‎ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessor.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ private void processNewPooledTransactionHashesMessage(
138138
"Malformed pooled transaction hashes message received (BREACH_OF_PROTOCOL), disconnecting: {}",
139139
peer,
140140
ex);
141+
LOG.trace("Message data: {}", transactionsMessage.getData());
141142
peer.disconnect(DisconnectReason.BREACH_OF_PROTOCOL);
142143
}
143144
}

‎ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/messages/BlockBodiesMessageTest.java‎

Lines changed: 45 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,22 @@
1414
*/
1515
package org.hyperledger.besu.ethereum.eth.messages;
1616

17+
import static org.junit.jupiter.api.Assertions.assertThrows;
18+
1719
import org.hyperledger.besu.config.GenesisConfigFile;
1820
import org.hyperledger.besu.ethereum.core.BlockBody;
1921
import org.hyperledger.besu.ethereum.core.BlockHeader;
22+
import org.hyperledger.besu.ethereum.core.BlockHeaderFunctions;
2023
import org.hyperledger.besu.ethereum.core.Transaction;
2124
import org.hyperledger.besu.ethereum.difficulty.fixed.FixedDifficultyProtocolSchedule;
2225
import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderFunctions;
26+
import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
27+
import org.hyperledger.besu.ethereum.mainnet.ScheduleBasedBlockHeaderFunctions;
2328
import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData;
2429
import org.hyperledger.besu.ethereum.p2p.rlpx.wire.RawMessage;
2530
import org.hyperledger.besu.ethereum.rlp.BytesValueRLPInput;
2631
import org.hyperledger.besu.ethereum.rlp.RLP;
32+
import org.hyperledger.besu.ethereum.rlp.RLPException;
2733
import org.hyperledger.besu.ethereum.rlp.RLPInput;
2834
import org.hyperledger.besu.evm.internal.EvmConfiguration;
2935

@@ -36,11 +42,21 @@
3642
import com.google.common.io.Resources;
3743
import org.apache.tuweni.bytes.Bytes;
3844
import org.assertj.core.api.Assertions;
45+
import org.junit.Before;
3946
import org.junit.Test;
4047

4148
/** Tests for {@link BlockBodiesMessage}. */
4249
public final class BlockBodiesMessageTest {
4350

51+
private ProtocolSchedule protocolSchedule;
52+
53+
@Before
54+
public void setup() {
55+
protocolSchedule =
56+
FixedDifficultyProtocolSchedule.create(
57+
GenesisConfigFile.development().getConfigOptions(), false, EvmConfiguration.DEFAULT);
58+
}
59+
4460
@Test
4561
public void blockBodiesRoundTrip() throws IOException {
4662
final List<BlockBody> bodies = new ArrayList<>();
@@ -65,16 +81,37 @@ public void blockBodiesRoundTrip() throws IOException {
6581
final MessageData initialMessage = BlockBodiesMessage.create(bodies);
6682
final MessageData raw = new RawMessage(EthPV62.BLOCK_BODIES, initialMessage.getData());
6783
final BlockBodiesMessage message = BlockBodiesMessage.readFrom(raw);
68-
final Iterator<BlockBody> readBodies =
69-
message
70-
.bodies(
71-
FixedDifficultyProtocolSchedule.create(
72-
GenesisConfigFile.development().getConfigOptions(),
73-
false,
74-
EvmConfiguration.DEFAULT))
75-
.iterator();
84+
final Iterator<BlockBody> readBodies = message.bodies(protocolSchedule).iterator();
7685
for (int i = 0; i < 50; ++i) {
7786
Assertions.assertThat(readBodies.next()).isEqualTo(bodies.get(i));
7887
}
7988
}
89+
90+
@Test
91+
public void shouldEncodeEmptyBlocksInBlockBodiesMessage() {
92+
final Bytes bytes = Bytes.fromHexString("0xc2c0c0");
93+
final MessageData raw = new RawMessage(EthPV62.BLOCK_BODIES, bytes);
94+
final BlockBodiesMessage message = BlockBodiesMessage.readFrom(raw);
95+
final List<BlockBody> bodies = message.bodies(protocolSchedule);
96+
bodies.forEach(blockBody -> Assertions.assertThat(blockBody.isEmpty()).isTrue());
97+
}
98+
99+
@Test
100+
public void shouldNotThrowRLPExceptionIfAllowedEmptyBody() {
101+
final Bytes bytes = Bytes.fromHexString("0xc0");
102+
final BlockBody empty = BlockBody.readFrom(RLP.input(bytes), null, true);
103+
Assertions.assertThat(empty.isEmpty()).isTrue();
104+
}
105+
106+
@Test
107+
public void shouldThrowRLPExceptionIfNotAllowedEmptyBody() {
108+
final Bytes bytes = Bytes.fromHexString("0xc0");
109+
final BlockHeaderFunctions blockHeaderFunctions =
110+
ScheduleBasedBlockHeaderFunctions.create(protocolSchedule);
111+
assertThrows(
112+
RLPException.class,
113+
() -> {
114+
BlockBody.readFrom(RLP.input(bytes), blockHeaderFunctions, false);
115+
});
116+
}
80117
}

‎ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessorTest.java‎

Lines changed: 27 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ public void shouldNotScheduleGetPooledTransactionsTaskTwice() {
212212
public void shouldCreateAndDecodeForEth66() {
213213

214214
final List<TransactionAnnouncement> expectedAnnouncementList =
215-
transactionList.stream().map(TransactionAnnouncement::new).collect(Collectors.toList());
215+
transactionList.stream().map(TransactionAnnouncement::new).toList();
216216

217217
final NewPooledTransactionHashesMessage message =
218218
NewPooledTransactionHashesMessage.create(transactionList, EthProtocol.ETH66);
@@ -268,7 +268,7 @@ public void shouldThrowRLPExceptionIfIncorrectVersion() {
268268
public void shouldEncodeTransactionsCorrectly_Eth68() {
269269

270270
final String expected =
271-
"0xf879c3000102cf840000000184000000028400000003f863a00000000000000000000000000000000000000000000000000000000000000001a00000000000000000000000000000000000000000000000000000000000000002a00000000000000000000000000000000000000000000000000000000000000003";
271+
"0xf87983000102cf840000000184000000028400000003f863a00000000000000000000000000000000000000000000000000000000000000001a00000000000000000000000000000000000000000000000000000000000000002a00000000000000000000000000000000000000000000000000000000000000003";
272272
final List<Hash> hashes =
273273
List.of(
274274
Hash.fromHexString(
@@ -289,7 +289,7 @@ public void shouldEncodeTransactionsCorrectly_Eth68() {
289289
public void shouldDecodeBytesCorrectly_Eth68() {
290290
/*
291291
* [
292-
* ["0x00","0x01","0x02"]
292+
* "0x0000102"]
293293
* ["0x00000001","0x00000002","0x00000003"],
294294
* ["0x0000000000000000000000000000000000000000000000000000000000000001",
295295
* "0x0000000000000000000000000000000000000000000000000000000000000002",
@@ -299,7 +299,7 @@ public void shouldDecodeBytesCorrectly_Eth68() {
299299

300300
final Bytes bytes =
301301
Bytes.fromHexString(
302-
"0xf879c3000102cf840000000184000000028400000003f863a00000000000000000000000000000000000000000000000000000000000000001a00000000000000000000000000000000000000000000000000000000000000002a00000000000000000000000000000000000000000000000000000000000000003");
302+
"0xf87983000102cf840000000184000000028400000003f863a00000000000000000000000000000000000000000000000000000000000000001a00000000000000000000000000000000000000000000000000000000000000002a00000000000000000000000000000000000000000000000000000000000000003");
303303

304304
final List<TransactionAnnouncement> announcementList =
305305
getDecoder(EthProtocol.ETH68).decode(RLP.input(bytes));
@@ -375,9 +375,8 @@ public void shouldThrowInvalidArgumentExceptionWhenCreatingListsWithDifferentSiz
375375
final Exception exception =
376376
assertThrows(
377377
IllegalArgumentException.class,
378-
() -> {
379-
TransactionAnnouncement.create(new ArrayList<>(), List.of(1L), new ArrayList<>());
380-
});
378+
() ->
379+
TransactionAnnouncement.create(new ArrayList<>(), List.of(1L), new ArrayList<>()));
381380
final String expectedMessage = "Hashes, sizes and types must have the same number of elements";
382381
final String actualMessage = exception.getMessage();
383382
assertThat(actualMessage).isEqualTo(expectedMessage);
@@ -388,10 +387,9 @@ public void shouldThrowInvalidArgumentExceptionWhenEncodingListsWithDifferentSiz
388387
final Exception exception =
389388
assertThrows(
390389
IllegalArgumentException.class,
391-
() -> {
392-
TransactionAnnouncementEncoder.encodeForEth68(
393-
new ArrayList<>(), List.of(1), new ArrayList<>());
394-
});
390+
() ->
391+
TransactionAnnouncementEncoder.encodeForEth68(
392+
new ArrayList<>(), List.of(1), new ArrayList<>()));
395393
final String expectedMessage = "Hashes, sizes and types must have the same number of elements";
396394
final String actualMessage = exception.getMessage();
397395
assertThat(actualMessage).isEqualTo(expectedMessage);
@@ -401,18 +399,17 @@ public void shouldThrowInvalidArgumentExceptionWhenEncodingListsWithDifferentSiz
401399
@SuppressWarnings("UnusedVariable")
402400
public void shouldThrowRLPExceptionWhenDecodingListsWithDifferentSizes() {
403401

404-
// [[],[],["0x881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351"]]
402+
// ["0x000102",[],["0x881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351"]]
405403
final Bytes invalidMessageBytes =
406404
Bytes.fromHexString(
407-
"0xe4c0c0e1a0881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351");
405+
"0xe783000102c0e1a0881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351");
408406

409407
final Exception exception =
410408
assertThrows(
411409
RLPException.class,
412-
() -> {
413-
TransactionAnnouncementDecoder.getDecoder(EthProtocol.ETH68)
414-
.decode(RLP.input(invalidMessageBytes));
415-
});
410+
() ->
411+
TransactionAnnouncementDecoder.getDecoder(EthProtocol.ETH68)
412+
.decode(RLP.input(invalidMessageBytes)));
416413

417414
final String expectedMessage = "Hashes, sizes and types must have the same number of elements";
418415
final String actualMessage = exception.getMessage();
@@ -423,38 +420,36 @@ public void shouldThrowRLPExceptionWhenDecodingListsWithDifferentSizes() {
423420
public void shouldThrowRLPExceptionWhenTypeIsInvalid() {
424421
final Bytes invalidMessageBytes =
425422
Bytes.fromHexString(
426-
// [["0x09"],["0x00000002"],["0x881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351"]]
427-
"0xeac109c58400000002e1a0881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351");
423+
// ["0x07",["0x00000002"],["0x881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351"]]
424+
"0xe907c58400000002e1a0881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351");
428425

429426
final Exception exception =
430427
assertThrows(
431-
RLPException.class,
432-
() -> {
433-
TransactionAnnouncementDecoder.getDecoder(EthProtocol.ETH68)
434-
.decode(RLP.input(invalidMessageBytes));
435-
});
428+
IllegalArgumentException.class,
429+
() ->
430+
TransactionAnnouncementDecoder.getDecoder(EthProtocol.ETH68)
431+
.decode(RLP.input(invalidMessageBytes)));
436432

437433
final String expectedMessage = "Unsupported transaction type";
438-
final String actualMessage = exception.getCause().getMessage();
434+
final String actualMessage = exception.getMessage();
439435
assertThat(actualMessage).contains(expectedMessage);
440436
}
441437

442438
@Test
443439
public void shouldThrowRLPExceptionWhenSizeSizeGreaterThanFourBytes() {
444440
final Bytes invalidMessageBytes =
445441
Bytes.fromHexString(
446-
// [["0x02"],["0xffffffff01"],["0x881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351"]]
447-
"0xebc102c685ffffffff00e1a0881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351");
442+
// ["0x02",["0xffffffff01"],["0x881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351"]]
443+
"0xea02c685ffffffff00e1a0881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351");
448444

449445
final Exception exception =
450446
assertThrows(
451447
RLPException.class,
452-
() -> {
453-
TransactionAnnouncementDecoder.getDecoder(EthProtocol.ETH68)
454-
.decode(RLP.input(invalidMessageBytes));
455-
});
448+
() ->
449+
TransactionAnnouncementDecoder.getDecoder(EthProtocol.ETH68)
450+
.decode(RLP.input(invalidMessageBytes)));
456451

457-
final String expectedMessage = "Cannot read a 4-byte int";
452+
final String expectedMessage = "Value of size 5 has more than 4 bytes";
458453
final String actualMessage = exception.getCause().getMessage();
459454
assertThat(actualMessage).contains(expectedMessage);
460455
}

0 commit comments

Comments
 (0)