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

[PAN-2811] Be more lenient with discovery message deserialization #1580

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public static FindNeighborsPacketData readFrom(final RLPInput in) {
in.enterList();
final BytesValue target = in.readBytesValue();
final long expiration = in.readLongScalar();
in.leaveList();
in.leaveList(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

(optional) make leaveList parameter an enum. Much easier to understand what
in.leaveList(RLPInput.IgnoreRest) does as opposed to in.leaveList(true)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd say either @RatanRSur's suggestion or at least leave a comment that (true) means to ignore the rest of the list, for all the leaveList(true) calls.

Copy link
Contributor

Choose a reason for hiding this comment

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

Third option would be to have two methods - the usual leaveList and the new leaveListSkippingRemainingElements

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed this to leaveListLenient()

return new FindNeighborsPacketData(target, expiration);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public static NeighborsPacketData readFrom(final RLPInput in) {
in.enterList();
final List<DiscoveryPeer> peers = in.readList(DiscoveryPeer::readFrom);
final long expiration = in.readLongScalar();
in.leaveList();
in.leaveList(true);
return new NeighborsPacketData(peers, expiration);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,11 @@
package tech.pegasys.pantheon.ethereum.p2p.discovery.internal;

import static com.google.common.base.Preconditions.checkArgument;
import static tech.pegasys.pantheon.util.Preconditions.checkGuard;

import tech.pegasys.pantheon.ethereum.p2p.discovery.Endpoint;
import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryPacketDecodingException;
import tech.pegasys.pantheon.ethereum.rlp.RLPInput;
import tech.pegasys.pantheon.ethereum.rlp.RLPOutput;

import java.math.BigInteger;

public class PingPacketData implements PacketData {

/* Fixed value that represents we're using v4 of the P2P discovery protocol. */
Expand Down Expand Up @@ -53,18 +49,12 @@ public static PingPacketData create(final Endpoint from, final Endpoint to) {

public static PingPacketData readFrom(final RLPInput in) {
in.enterList();
final BigInteger version = in.readBigIntegerScalar();
checkGuard(
version.intValue() == VERSION,
PeerDiscoveryPacketDecodingException::new,
"Version mismatch in ping packet. Expected: %s, got: %s.",
VERSION,
version);

// The first element signifies the "version", but this value is ignored as of EIP-8
in.readBigIntegerScalar();
final Endpoint from = Endpoint.decodeStandalone(in);
final Endpoint to = Endpoint.decodeStandalone(in);
final long expiration = in.readLongScalar();
in.leaveList();
in.leaveList(true);
return new PingPacketData(from, to, expiration);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public static PongPacketData readFrom(final RLPInput in) {
final Endpoint to = Endpoint.decodeStandalone(in);
final BytesValue hash = in.readBytesValue();
final long expiration = in.readLongScalar();
in.leaveList();
in.leaveList(true);
return new PongPacketData(to, hash, expiration);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
* Copyright 2019 ConsenSys AG.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*/
package tech.pegasys.pantheon.ethereum.p2p.discovery.internal;

import static org.assertj.core.api.Assertions.assertThat;

import tech.pegasys.pantheon.ethereum.p2p.peers.Peer;
import tech.pegasys.pantheon.ethereum.rlp.BytesValueRLPOutput;
import tech.pegasys.pantheon.ethereum.rlp.RLP;
import tech.pegasys.pantheon.util.bytes.BytesValue;

import org.junit.Test;

public class FindNeighborsPacketDataTest {
@Test
public void serializeDeserialize() {
final long time = System.currentTimeMillis();
final BytesValue target = Peer.randomId();

final FindNeighborsPacketData packet = FindNeighborsPacketData.create(target);
final BytesValue serialized = RLP.encode(packet::writeTo);
final FindNeighborsPacketData deserialized =
FindNeighborsPacketData.readFrom(RLP.input(serialized));

assertThat(deserialized.getTarget()).isEqualTo(target);
assertThat(deserialized.getExpiration()).isGreaterThan(time);
}

@Test
public void readFrom() {
final long time = System.currentTimeMillis();
final BytesValue target = Peer.randomId();

BytesValueRLPOutput out = new BytesValueRLPOutput();
out.startList();
out.writeBytesValue(target);
out.writeLongScalar(time);
out.endList();
final BytesValue encoded = out.encoded();

final FindNeighborsPacketData deserialized =
FindNeighborsPacketData.readFrom(RLP.input(encoded));
assertThat(deserialized.getTarget()).isEqualTo(target);
assertThat(deserialized.getExpiration()).isEqualTo(time);
}

@Test
public void readFrom_withExtraFields() {
final long time = System.currentTimeMillis();
final BytesValue target = Peer.randomId();

BytesValueRLPOutput out = new BytesValueRLPOutput();
out.startList();
out.writeBytesValue(target);
out.writeLongScalar(time);
// Add extra list elements
out.writeLong(123L);
out.endList();
final BytesValue encoded = out.encoded();

final FindNeighborsPacketData deserialized =
FindNeighborsPacketData.readFrom(RLP.input(encoded));
assertThat(deserialized.getTarget()).isEqualTo(target);
assertThat(deserialized.getExpiration()).isEqualTo(time);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/*
* Copyright 2019 ConsenSys AG.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*/
package tech.pegasys.pantheon.ethereum.p2p.discovery.internal;

import static org.assertj.core.api.Assertions.assertThat;
import static tech.pegasys.pantheon.ethereum.p2p.peers.PeerTestHelper.enode;

import tech.pegasys.pantheon.ethereum.p2p.discovery.DiscoveryPeer;
import tech.pegasys.pantheon.ethereum.rlp.BytesValueRLPOutput;
import tech.pegasys.pantheon.ethereum.rlp.RLP;
import tech.pegasys.pantheon.util.bytes.BytesValue;

import java.util.Arrays;
import java.util.List;

import org.junit.Test;

public class NeighborsPacketDataTest {

@Test
public void serializeDeserialize() {
final long time = System.currentTimeMillis();
final List<DiscoveryPeer> peers =
Arrays.asList(DiscoveryPeer.fromEnode(enode()), DiscoveryPeer.fromEnode(enode()));

final NeighborsPacketData packet = NeighborsPacketData.create(peers);
final BytesValue serialized = RLP.encode(packet::writeTo);
final NeighborsPacketData deserialized = NeighborsPacketData.readFrom(RLP.input(serialized));

assertThat(deserialized.getNodes()).isEqualTo(peers);
assertThat(deserialized.getExpiration()).isGreaterThan(time);
}

@Test
public void readFrom() {
final long time = System.currentTimeMillis();
final List<DiscoveryPeer> peers =
Arrays.asList(DiscoveryPeer.fromEnode(enode()), DiscoveryPeer.fromEnode(enode()));

BytesValueRLPOutput out = new BytesValueRLPOutput();
out.startList();
out.writeList(peers, DiscoveryPeer::writeTo);
out.writeLongScalar(time);
out.endList();
BytesValue encoded = out.encoded();

final NeighborsPacketData deserialized = NeighborsPacketData.readFrom(RLP.input(encoded));
assertThat(deserialized.getNodes()).isEqualTo(peers);
assertThat(deserialized.getExpiration()).isEqualTo(time);
}

@Test
public void readFrom_extraFields() {
final long time = System.currentTimeMillis();
final List<DiscoveryPeer> peers =
Arrays.asList(DiscoveryPeer.fromEnode(enode()), DiscoveryPeer.fromEnode(enode()));

BytesValueRLPOutput out = new BytesValueRLPOutput();
out.startList();
out.writeList(peers, DiscoveryPeer::writeTo);
out.writeLongScalar(time);
out.endList();
BytesValue encoded = out.encoded();

final NeighborsPacketData deserialized = NeighborsPacketData.readFrom(RLP.input(encoded));
assertThat(deserialized.getNodes()).isEqualTo(peers);
assertThat(deserialized.getExpiration()).isEqualTo(time);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/*
* Copyright 2019 ConsenSys AG.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*/
package tech.pegasys.pantheon.ethereum.p2p.discovery.internal;

import static org.assertj.core.api.Assertions.assertThat;

import tech.pegasys.pantheon.ethereum.p2p.discovery.Endpoint;
import tech.pegasys.pantheon.ethereum.rlp.BytesValueRLPOutput;
import tech.pegasys.pantheon.ethereum.rlp.RLP;
import tech.pegasys.pantheon.util.bytes.BytesValue;

import java.util.OptionalInt;

import org.junit.Test;

public class PingPacketDataTest {

@Test
public void serializeDeserialize() {
final long currentTime = System.currentTimeMillis();

final Endpoint from = new Endpoint("127.0.0.1", 30303, OptionalInt.of(30303));
final Endpoint to = new Endpoint("127.0.0.2", 30303, OptionalInt.empty());
final PingPacketData packet = PingPacketData.create(from, to);
final BytesValue serialized = RLP.encode(packet::writeTo);
final PingPacketData deserialized = PingPacketData.readFrom(RLP.input(serialized));

assertThat(deserialized.getFrom()).isEqualTo(from);
assertThat(deserialized.getTo()).isEqualTo(to);
assertThat(deserialized.getExpiration()).isGreaterThan(currentTime);
}

@Test
public void readFrom() {
final int version = 4;
final Endpoint from = new Endpoint("127.0.0.1", 30303, OptionalInt.of(30303));
final Endpoint to = new Endpoint("127.0.0.2", 30303, OptionalInt.empty());
final long time = System.currentTimeMillis();

final BytesValueRLPOutput out = new BytesValueRLPOutput();
out.startList();
out.writeIntScalar(version);
from.encodeStandalone(out);
to.encodeStandalone(out);
out.writeLongScalar(time);
out.endList();

final BytesValue serialized = out.encoded();
final PingPacketData deserialized = PingPacketData.readFrom(RLP.input(serialized));

assertThat(deserialized.getFrom()).isEqualTo(from);
assertThat(deserialized.getTo()).isEqualTo(to);
assertThat(deserialized.getExpiration()).isEqualTo(time);
}

@Test
public void readFrom_withExtraFields() {
final int version = 4;
final Endpoint from = new Endpoint("127.0.0.1", 30303, OptionalInt.of(30303));
final Endpoint to = new Endpoint("127.0.0.2", 30303, OptionalInt.empty());
final long time = System.currentTimeMillis();

final BytesValueRLPOutput out = new BytesValueRLPOutput();
out.startList();
out.writeIntScalar(version);
from.encodeStandalone(out);
to.encodeStandalone(out);
out.writeLongScalar(time);
// Add extra field
out.writeLongScalar(11);
out.endList();

final BytesValue serialized = out.encoded();
final PingPacketData deserialized = PingPacketData.readFrom(RLP.input(serialized));

assertThat(deserialized.getFrom()).isEqualTo(from);
assertThat(deserialized.getTo()).isEqualTo(to);
assertThat(deserialized.getExpiration()).isEqualTo(time);
}

@Test
public void readFrom_unknownVersion() {
final int version = 99;
final Endpoint from = new Endpoint("127.0.0.1", 30303, OptionalInt.of(30303));
final Endpoint to = new Endpoint("127.0.0.2", 30303, OptionalInt.empty());
final long time = System.currentTimeMillis();

final BytesValueRLPOutput out = new BytesValueRLPOutput();
out.startList();
out.writeIntScalar(version);
from.encodeStandalone(out);
to.encodeStandalone(out);
out.writeLongScalar(time);
out.endList();

final BytesValue serialized = out.encoded();
final PingPacketData deserialized = PingPacketData.readFrom(RLP.input(serialized));

assertThat(deserialized.getFrom()).isEqualTo(from);
assertThat(deserialized.getTo()).isEqualTo(to);
assertThat(deserialized.getExpiration()).isEqualTo(time);
}
}
Loading