mirrors/bisq
1
0
Fork 0
mirror of https://github.com/bisq-network/bisq.git synced 2025-02-24 15:10:44 +01:00
Code Issues Projects Releases Packages Wiki Activity

Add payload safety checks in ProtectedStorageEntry

Browse source 
It is currently possible to construct a valid Payload object
that implements both the ProtectedStoragePayload and
PersistableNetworkPayload interfaces even though this combination is
invalid.

Instead of depending on future reviewers to catch an error, assert that
ProtectedStoragePayloads and PersistableNetworkPayloads are incompatible
as objects inside a ProtectedStorageEntry.

This allows cleanup of removeExpiredEntries that branched on this
behavior.
This commit is contained in:
Julian Knutsen 2019-11-14 08:42:47 -08:00
parent c81f8a6da3
commit bdef1e46ea
No known key found for this signature in database
GPG key ID: D85F536DB3615B2D
5 changed files with 59 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java
View file

@ -192,11 +192,9 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers
.forEach(entry -> {
ByteArray hashOfPayload = entry.getKey();
ProtectedStorageEntry protectedStorageEntry = map.get(hashOfPayload);
if (!(protectedStorageEntry.getProtectedStoragePayload() instanceof PersistableNetworkPayload)) {
toRemoveSet.add(protectedStorageEntry);
log.debug("We found an expired data entry. We remove the protectedData:\n\t" + Utilities.toTruncatedString(protectedStorageEntry));
map.remove(hashOfPayload);
}
toRemoveSet.add(protectedStorageEntry);
log.debug("We found an expired data entry. We remove the protectedData:\n\t" + Utilities.toTruncatedString(protectedStorageEntry));
map.remove(hashOfPayload);
});
// Batch processing can cause performance issues, so we give listeners a chance to deal with it by notifying
@ -570,7 +568,7 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers
}
}
}
private void maybeAddToRemoveAddOncePayloads(ProtectedStoragePayload protectedStoragePayload,
ByteArray hashOfData) {
if (protectedStoragePayload instanceof AddOncePayload) {

4
p2p/src/main/java/bisq/network/p2p/storage/payload/ProtectedStorageEntry.java
View file

@ -29,6 +29,8 @@ import bisq.common.util.Utilities;
import com.google.protobuf.ByteString;
import com.google.protobuf.Message;
import com.google.common.base.Preconditions;
import java.security.PublicKey;
import java.time.Clock;
@ -70,6 +72,8 @@ public class ProtectedStorageEntry implements NetworkPayload, PersistablePayload
long creationTimeStamp,
Clock clock) {
Preconditions.checkArgument(!(protectedStoragePayload instanceof PersistableNetworkPayload));
this.protectedStoragePayload = protectedStoragePayload;
this.ownerPubKeyBytes = ownerPubKeyBytes;
this.ownerPubKey = ownerPubKey;

14
p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageRemoveExpiredTest.java
View file

@ -20,7 +20,9 @@ package bisq.network.p2p.storage;
import bisq.network.p2p.TestUtils;
import bisq.network.p2p.storage.mocks.ExpirableProtectedStoragePayloadStub;
import bisq.network.p2p.storage.mocks.PersistableExpirableProtectedStoragePayloadStub;
import bisq.network.p2p.storage.mocks.PersistableNetworkPayloadStub;
import bisq.network.p2p.storage.mocks.ProtectedStoragePayloadStub;
import bisq.network.p2p.storage.payload.PersistableNetworkPayload;
import bisq.network.p2p.storage.payload.ProtectedStorageEntry;
import bisq.network.p2p.storage.payload.ProtectedStoragePayload;
@ -67,6 +69,18 @@ public class P2PDataStorageRemoveExpiredTest {
this.testState.verifyProtectedStorageRemove(beforeState, protectedStorageEntry, false, false, false, false);
}
// TESTCASE: Correctly skips all PersistableNetworkPayloads since they are not expirable
@Test
public void removeExpiredEntries_skipsPersistableNetworkPayload() {
PersistableNetworkPayload persistableNetworkPayload = new PersistableNetworkPayloadStub(true);
Assert.assertTrue(this.testState.mockedStorage.addPersistableNetworkPayload(persistableNetworkPayload,getTestNodeAddress(), true, true, false, false));
this.testState.mockedStorage.removeExpiredEntries();
Assert.assertTrue(this.testState.mockedStorage.getAppendOnlyDataStoreMap().containsKey(new P2PDataStorage.ByteArray(persistableNetworkPayload.getHash())));
}
// TESTCASE: Correctly skips non-persistable entries that are not expired
@Test
public void removeExpiredEntries_SkipNonExpiredExpirableEntries() throws CryptoException, NoSuchAlgorithmException {

2
p2p/src/test/java/bisq/network/p2p/storage/mocks/ProtectedStoragePayloadStub.java
View file

@ -45,7 +45,7 @@ public class ProtectedStoragePayloadStub implements ProtectedStoragePayload {
@Getter
private PublicKey ownerPubKey;
private Message messageMock;
protected Message messageMock;
public ProtectedStoragePayloadStub(PublicKey ownerPubKey) {
this.ownerPubKey = ownerPubKey;

36
p2p/src/test/java/bisq/network/p2p/storage/payload/ProtectedStorageEntryTest.java
View file

@ -213,4 +213,40 @@ public class ProtectedStorageEntryTest {
Assert.assertFalse(protectedStorageEntryOne.matchesRelevantPubKey(protectedStorageEntryTwo));
}
// TESTCASE: Payload implementing ProtectedStoragePayload & PersistableNetworkPayload is invalid
// We rely on the fact that a payload is either a ProtectedStoragePayload OR PersistableNetworkPayload, but Java
// does not have a clean way to specify mutually exclusive interfaces.
//
// We also want to guarantee that ONLY ProtectedStoragePayload objects are valid as payloads in
// ProtectedStorageEntrys. This test will give a defense in case future development work breaks that expectation.
@Test(expected = IllegalArgumentException.class)
public void ProtectedStoragePayload_PersistableNetworkPayload_incompatible() throws NoSuchAlgorithmException {
class IncompatiblePayload extends ProtectedStoragePayloadStub implements PersistableNetworkPayload {
private IncompatiblePayload(PublicKey ownerPubKey) {
super(ownerPubKey);
}
@Override
public byte[] getHash() {
return new byte[0];
}
@Override
public boolean verifyHashSize() {
return true;
}
@Override
public protobuf.PersistableNetworkPayload toProtoMessage() {
return (protobuf.PersistableNetworkPayload) this.messageMock;
}
}
KeyPair ownerKeys = TestUtils.generateKeyPair();
IncompatiblePayload incompatiblePayload = new IncompatiblePayload(ownerKeys.getPublic());
new ProtectedStorageEntry(incompatiblePayload,ownerKeys.getPublic(), 1,
new byte[] { 0 }, Clock.systemDefaultZone());
}
}