Skip to content

Commit

Permalink
removed SafeUsageChecker from BaseDataObject, deprecated IBDO.checkFo…
Browse files Browse the repository at this point in the history 
…rUnsafeDataChanges (#1017)
  • Loading branch information
@drivenflywheel
drivenflywheel authored Dec 5, 2024
1 parent 5f26666 commit 504c492
Show file tree
Hide file tree
Showing 4 changed files with 4 additions and 165 deletions.
30 changes: 1 addition & 29 deletions src/main/java/emissary/core/BaseDataObject.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,6 @@ public class BaseDataObject implements Serializable, Cloneable, Remote, IBaseDat
@Nullable
protected SeekableByteChannelFactory seekableByteChannelFactory;

final SafeUsageChecker safeUsageChecker = new SafeUsageChecker();

protected enum DataState {
NO_DATA, CHANNEL_ONLY, BYTE_ARRAY_ONLY, BYTE_ARRAY_AND_CHANNEL
Expand Down Expand Up @@ -233,18 +232,6 @@ protected DataState getDataState() {
}
}

/**
* {@inheritDoc}
*/
@Override
public void checkForUnsafeDataChanges() {
safeUsageChecker.checkForUnsafeDataChanges();

if (theData != null) {
safeUsageChecker.recordSnapshot(theData);
}
}

/**
* Create an empty BaseDataObject.
*/
Expand Down Expand Up @@ -351,10 +338,6 @@ public void setChannelFactory(final SeekableByteChannelFactory sbcf) {
Validate.notNull(sbcf, "Required: SeekableByteChannelFactory not null");
this.theData = null;
this.seekableByteChannelFactory = sbcf;

// calls to setData clear the unsafe state by definition
// reset the usage checker but don't capture a snapshot until someone requests the data in byte[] form
safeUsageChecker.reset();
}

/**
Expand Down Expand Up @@ -421,12 +404,7 @@ public byte[] data() {
return theData;
case CHANNEL_ONLY:
// Max size here is slightly less than the true max size to avoid memory issues
final byte[] bytes = SeekableByteChannelHelper.getByteArrayFromBdo(this, MAX_BYTE_ARRAY_SIZE);

// capture a reference to the returned byte[] so we can test for unsafe modifications of its contents
safeUsageChecker.recordSnapshot(bytes);

return bytes;
return SeekableByteChannelHelper.getByteArrayFromBdo(this, MAX_BYTE_ARRAY_SIZE);
case NO_DATA:
default:
return null; // NOSONAR maintains backwards compatibility
Expand All @@ -440,9 +418,6 @@ public byte[] data() {
public void setData(@Nullable final byte[] newData) {
this.seekableByteChannelFactory = null;
this.theData = newData == null ? new byte[0] : newData;

// calls to setData clear the unsafe state by definition, but we need to capture a new snapshot
safeUsageChecker.resetCacheThenRecordSnapshot(theData);
}

/**
Expand All @@ -469,9 +444,6 @@ public void setData(@Nullable final byte[] newData, final int offset, final int
this.theData = new byte[length];
System.arraycopy(newData, offset, this.theData, 0, length);
}

// calls to setData clear the unsafe state by definition, but we need to capture a new snapshot
safeUsageChecker.resetCacheThenRecordSnapshot(theData);
}

/**
Expand Down
8 changes: 3 additions & 5 deletions src/main/java/emissary/core/IBaseDataObject.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,10 @@ enum MergePolicy {
String DEFAULT_PARAM_SEPARATOR = ";";

/**
* Checks to see if payload byte arrays visible to external classes have any changes not explicitly saved via a call to
* the {@link IBaseDataObject#setData(byte[]) setData(byte[])}, {@link IBaseDataObject#setData(byte[], int, int)
* setData(byte[], int, int)}, or {@link IBaseDataObject#setChannelFactory(SeekableByteChannelFactory)
* setChannelFactory(SeekableByteChannelFactory)} method.
* @deprecated As of emissary 8.18.0, this method performs no operations
*/
void checkForUnsafeDataChanges();
@Deprecated(forRemoval = true)
default void checkForUnsafeDataChanges() {};

/**
* Return the data as a byte array. If using a channel to the data, calling this method will only return up to
Expand Down
1 change: 0 additions & 1 deletion src/main/java/emissary/place/ServiceProviderPlace.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,6 @@ public List<IBaseDataObject> agentProcessHeavyDuty(IBaseDataObject payload) thro
MDC.put(MDCConstants.SERVICE_LOCATION, this.getKey());
try {
List<IBaseDataObject> l = processHeavyDuty(payload);
payload.checkForUnsafeDataChanges();
rehash(payload);
return l;
} catch (Exception e) {
Expand Down
130 changes: 0 additions & 130 deletions src/test/java/emissary/core/BaseDataObjectTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@
import emissary.core.channels.SeekableByteChannelHelper;
import emissary.directory.DirectoryEntry;
import emissary.pickup.Priority;
import emissary.test.core.junit5.LogbackTester;
import emissary.test.core.junit5.UnitTest;

import ch.qos.logback.classic.Level;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ListMultimap;
import org.apache.commons.io.IOUtils;
Expand All @@ -27,7 +25,6 @@
import java.lang.reflect.Field;
import java.nio.ByteBuffer;
import java.nio.channels.SeekableByteChannel;
import java.nio.charset.StandardCharsets;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -42,7 +39,6 @@
import java.util.stream.Stream;
import javax.annotation.Nullable;

import static emissary.core.SafeUsageChecker.UNSAFE_MODIFICATION_DETECTED;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
Expand Down Expand Up @@ -1308,132 +1304,6 @@ void testExtractedRecordClone() {
}
}

static final byte[] DATA_MODIFICATION_BYTES = "These are the test bytes!".getBytes(StandardCharsets.US_ASCII);
static final Level LEVELS_ONE_WARN = Level.WARN;

@Nullable
@SuppressWarnings("StaticAssignmentOfThrowable")
static final Throwable NO_THROWABLES = null;
List<LogbackTester.SimplifiedLogEvent> events = new ArrayList<>();
LogbackTester.SimplifiedLogEvent event1 =
new LogbackTester.SimplifiedLogEvent(LEVELS_ONE_WARN, UNSAFE_MODIFICATION_DETECTED, NO_THROWABLES);

@Test
void testChannelFactoryInArrayOutNoSet() throws IOException {
try (LogbackTester logbackTester = new LogbackTester(SafeUsageChecker.class.getName())) {
final IBaseDataObject ibdo = new BaseDataObject();

ibdo.setChannelFactory(InMemoryChannelFactory.create(DATA_MODIFICATION_BYTES));

final byte[] data = ibdo.data();

Arrays.fill(data, (byte) 0);

ibdo.checkForUnsafeDataChanges();

assertArrayEquals(DATA_MODIFICATION_BYTES, ibdo.data());
events.add(event1);
logbackTester.checkLogList(events);
}
}

@Test
void shouldDetectUnsafeChangesIfArrayChangesNotFollowedByOneSet() throws IOException {
try (LogbackTester logbackTester = new LogbackTester(SafeUsageChecker.class.getName())) {
final IBaseDataObject ibdo = new BaseDataObject();

ibdo.setChannelFactory(InMemoryChannelFactory.create(DATA_MODIFICATION_BYTES));

byte[] data = ibdo.data();

Arrays.fill(data, (byte) 0);

ibdo.checkForUnsafeDataChanges();

assertArrayEquals(DATA_MODIFICATION_BYTES, ibdo.data());
events.add(event1);
logbackTester.checkLogList(events);
}
}

@Test
void shouldDetectUnsafeChangesIfArrayChangesNotFollowedByBothSet() throws IOException {
try (LogbackTester logbackTester = new LogbackTester(SafeUsageChecker.class.getName())) {
final IBaseDataObject ibdo = new BaseDataObject();

ibdo.setChannelFactory(InMemoryChannelFactory.create(DATA_MODIFICATION_BYTES));

final byte[] data0 = ibdo.data();
final byte[] data1 = ibdo.data();

Arrays.fill(data0, (byte) 0);
Arrays.fill(data1, (byte) 0);

ibdo.checkForUnsafeDataChanges();

assertArrayEquals(DATA_MODIFICATION_BYTES, ibdo.data());
events.add(event1);
logbackTester.checkLogList(events);
}
}

@Test
void shouldDetectNoUnsafeChangesImmediatelyAfterSetChannelFactory() throws IOException {
try (LogbackTester logbackTester = new LogbackTester(SafeUsageChecker.class.getName())) {
final IBaseDataObject ibdo = new BaseDataObject();

ibdo.setChannelFactory(InMemoryChannelFactory.create(DATA_MODIFICATION_BYTES));

final byte[] data = ibdo.data();

Arrays.fill(data, (byte) 0);
ibdo.setChannelFactory(InMemoryChannelFactory.create(data));

ibdo.checkForUnsafeDataChanges();

assertArrayEquals(new byte[DATA_MODIFICATION_BYTES.length], ibdo.data());
logbackTester.checkLogList(Collections.emptyList());
}
}

@Test
void shouldDetectNoUnsafeChangesImmediatelyAfterSetData() throws IOException {
try (LogbackTester logbackTester = new LogbackTester(SafeUsageChecker.class.getName())) {
final IBaseDataObject ibdo = new BaseDataObject();

ibdo.setChannelFactory(InMemoryChannelFactory.create(DATA_MODIFICATION_BYTES));

final byte[] data = ibdo.data();

Arrays.fill(data, (byte) 0);
ibdo.setData(data);

ibdo.checkForUnsafeDataChanges();

assertArrayEquals(new byte[DATA_MODIFICATION_BYTES.length], ibdo.data());
logbackTester.checkLogList(Collections.emptyList());
}
}

@Test
void testArrayInArrayOutNoSet() throws IOException {
try (LogbackTester logbackTester = new LogbackTester(SafeUsageChecker.class.getName())) {
final IBaseDataObject ibdo = new BaseDataObject();

ibdo.setData(DATA_MODIFICATION_BYTES);

final byte[] data = ibdo.data();

Arrays.fill(data, (byte) 0);

ibdo.checkForUnsafeDataChanges();

assertArrayEquals(DATA_MODIFICATION_BYTES, ibdo.data());
events.add(event1);
logbackTester.checkLogList(events);
}
}

@Test
void testNewInputStream() throws IOException {
final IBaseDataObject ibdo = new BaseDataObject();
Expand Down

0 comments on commit 504c492

Please sign in to comment.