Skip to content

Commit

Permalink
GenericKeyState clear text byteArray if it has been dereferenced
Browse files Browse the repository at this point in the history
Previously in clear() we only cleared the bytes dereferenced flag without
actually clearing the byteArray. This meant we could overwrite the underlying
byte array of some value that we had returned previously.
  • Loading branch information
burqen committed Sep 20, 2018
1 parent 06dd183 commit 10c7164
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,11 @@ public class GenericKeyState extends TemporalValueWriterAdapter<RuntimeException
/* <initializers> */
void clear()
{
if ( type == Type.TEXT && long1 == TRUE )
{
// Clear byteArray if it has been dereferenced
byteArray = null;
}
type = null;
long0 = 0;
long1 = 0;
Expand Down Expand Up @@ -667,7 +672,7 @@ Value asValue()
return Values.of( populateValueArray( new DurationValue[arrayLength],
i -> durationAsValue( long0Array[i], long1Array[i], long2Array[i], long3Array[i] ) ) );
case TEXT_ARRAY:
// no need to set bytes dereferenced because byte[][] owned by this class will deserialized into String objects.
// no need to set bytes dereferenced because byte[][] owned by this class will be deserialized into String objects.
return Values.of( populateValueArray( new String[arrayLength], i -> textAsValueRaw( byteArrayArray[i], long0Array[i] ) ) );
case BOOLEAN_ARRAY:
return booleanArrayAsValue();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;

import java.nio.charset.StandardCharsets;
import java.time.Duration;
import java.time.LocalDate;
import java.time.LocalDateTime;
Expand Down Expand Up @@ -58,6 +59,7 @@
import org.neo4j.values.storable.TimeValue;
import org.neo4j.values.storable.Value;
import org.neo4j.values.storable.ValueGroup;
import org.neo4j.values.storable.Values;

import static java.util.Arrays.asList;
import static org.junit.jupiter.api.Assertions.assertEquals;
Expand Down Expand Up @@ -399,6 +401,40 @@ void highestMustBeHighest()
assertHighest( doubleArray( new double[]{Double.POSITIVE_INFINITY} ) );
}

@Test
void shouldNeverDereferencedTextValues()
{
// Given a value that we dereference
Value srcValue = Values.utf8Value( "First string".getBytes( StandardCharsets.UTF_8 ) );
GenericKeyState genericKeyState = newKeyState();
genericKeyState.writeValue( srcValue, NEUTRAL );
Value dereferencedValue = genericKeyState.asValue();
assertEquals( srcValue, dereferencedValue );

// and write to page
PageCursor cursor = newPageCursor();
int offset = cursor.getOffset();
genericKeyState.put( cursor );
int keySize = cursor.getOffset() - offset;
cursor.setOffset( offset );

// we should not overwrite the first dereferenced value when initializing from a new value
genericKeyState.clear();
Value srcValue2 = Values.utf8Value( "Secondstring".getBytes( StandardCharsets.UTF_8 ) ); // <- Same length as first string
genericKeyState.writeValue( srcValue2, NEUTRAL );
Value dereferencedValue2 = genericKeyState.asValue();
assertEquals( srcValue2, dereferencedValue2 );
assertEquals( srcValue, dereferencedValue );

// and we should not overwrite the second value when we read back the first value from page
genericKeyState.clear();
genericKeyState.read( cursor, keySize );
Value dereferencedValue3 = genericKeyState.asValue();
assertEquals( srcValue, dereferencedValue3 );
assertEquals( srcValue2, dereferencedValue2 );
assertEquals( srcValue, dereferencedValue );
}

private void assertHighestStringArray()
{
for ( int i = 0; i < 1000; i++ )
Expand Down

0 comments on commit 10c7164

Please sign in to comment.