Skip to content

Commit

Permalink
Fix relationship group type overflow in command readers
Browse files Browse the repository at this point in the history
Previously, primitive short, read from a channel, was converted to int using
implicit widening primitive conversion. This could lead to negative
relationship group types because in fact short value represented an unsigned
short. This commit makes sure all command readers interpret relationship group
type as unsigned short.
  • Loading branch information
lutovich committed Jun 14, 2016
1 parent f29dd10 commit 34db227
Show file tree
Hide file tree
Showing 6 changed files with 164 additions and 9 deletions.
Expand Up @@ -65,6 +65,7 @@
import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read2bLengthAndString; import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read2bLengthAndString;
import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read2bMap; import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read2bMap;
import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read3bLengthAndString; import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read3bLengthAndString;
import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.shortToUnsignedInt;


public class PhysicalLogCommandReaderV2_2_10 implements CommandReader, NeoCommandHandler public class PhysicalLogCommandReaderV2_2_10 implements CommandReader, NeoCommandHandler
{ {
Expand Down Expand Up @@ -252,7 +253,7 @@ public boolean visitRelationshipGroupCommand( Command.RelationshipGroupCommand c
{ {
throw new IOException( "Illegal in use flag: " + inUseByte ); throw new IOException( "Illegal in use flag: " + inUseByte );
} }
int type = channel.getShort(); int type = shortToUnsignedInt( channel.getShort() );
RelationshipGroupRecord record = new RelationshipGroupRecord( id, type ); RelationshipGroupRecord record = new RelationshipGroupRecord( id, type );
record.setInUse( inUse ); record.setInUse( inUse );
record.setNext( channel.getLong() ); record.setNext( channel.getLong() );
Expand Down
Expand Up @@ -65,6 +65,7 @@
import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read2bLengthAndString; import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read2bLengthAndString;
import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read2bMap; import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read2bMap;
import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read3bLengthAndString; import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read3bLengthAndString;
import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.shortToUnsignedInt;


public class PhysicalLogNeoCommandReaderV2_1 implements CommandReader public class PhysicalLogNeoCommandReaderV2_1 implements CommandReader
{ {
Expand Down Expand Up @@ -309,7 +310,7 @@ public boolean visitRelationshipGroupCommand( Command.RelationshipGroupCommand c
{ {
throw new IOException( "Illegal in use flag: " + inUseByte ); throw new IOException( "Illegal in use flag: " + inUseByte );
} }
int type = channel.getShort(); int type = shortToUnsignedInt( channel.getShort() );
RelationshipGroupRecord record = new RelationshipGroupRecord( id, type ); RelationshipGroupRecord record = new RelationshipGroupRecord( id, type );
record.setInUse( inUse ); record.setInUse( inUse );
record.setNext( channel.getLong() ); record.setNext( channel.getLong() );
Expand Down
Expand Up @@ -65,6 +65,7 @@
import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read2bLengthAndString; import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read2bLengthAndString;
import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read2bMap; import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read2bMap;
import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read3bLengthAndString; import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read3bLengthAndString;
import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.shortToUnsignedInt;


public class PhysicalLogNeoCommandReaderV2_2 implements CommandReader public class PhysicalLogNeoCommandReaderV2_2 implements CommandReader
{ {
Expand Down Expand Up @@ -304,7 +305,7 @@ public boolean visitRelationshipGroupCommand( Command.RelationshipGroupCommand c
{ {
throw new IOException( "Illegal in use flag: " + inUseByte ); throw new IOException( "Illegal in use flag: " + inUseByte );
} }
int type = channel.getShort(); int type = shortToUnsignedInt( channel.getShort() );
RelationshipGroupRecord record = new RelationshipGroupRecord( id, type ); RelationshipGroupRecord record = new RelationshipGroupRecord( id, type );
record.setInUse( inUse ); record.setInUse( inUse );
record.setNext( channel.getLong() ); record.setNext( channel.getLong() );
Expand Down
Expand Up @@ -65,6 +65,7 @@
import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read2bLengthAndString; import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read2bLengthAndString;
import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read2bMap; import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read2bMap;
import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read3bLengthAndString; import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.read3bLengthAndString;
import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.shortToUnsignedInt;


public class PhysicalLogNeoCommandReaderV2_2_4 implements CommandReader, NeoCommandHandler public class PhysicalLogNeoCommandReaderV2_2_4 implements CommandReader, NeoCommandHandler
{ {
Expand Down Expand Up @@ -252,7 +253,7 @@ public boolean visitRelationshipGroupCommand( Command.RelationshipGroupCommand c
{ {
throw new IOException( "Illegal in use flag: " + inUseByte ); throw new IOException( "Illegal in use flag: " + inUseByte );
} }
int type = getUnsignedShort( channel.getShort() ); int type = shortToUnsignedInt( channel.getShort() );
RelationshipGroupRecord record = new RelationshipGroupRecord( id, type ); RelationshipGroupRecord record = new RelationshipGroupRecord( id, type );
record.setInUse( inUse ); record.setInUse( inUse );
record.setNext( channel.getLong() ); record.setNext( channel.getLong() );
Expand Down Expand Up @@ -752,9 +753,4 @@ public void apply()
public void close() public void close()
{ // Nothing to close { // Nothing to close
} }

private int getUnsignedShort( short value )
{
return value & 0xFFFF;
}
} }
Expand Up @@ -266,4 +266,9 @@ public static int safeCastLongToInt( long value )
} }
return (int) value; return (int) value;
} }

public static int shortToUnsignedInt( short value )
{
return value & 0xFFFF;
}
} }
@@ -0,0 +1,151 @@
/*
* Copyright (c) 2002-2016 "Neo Technology,"
* Network Engine for Objects in Lund AB [http://neotechnology.com]
*
* This file is part of Neo4j.
*
* Neo4j is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package org.neo4j.kernel.impl.transaction.command;

import org.junit.Test;

import java.io.IOException;

import org.neo4j.kernel.impl.store.record.Record;
import org.neo4j.kernel.impl.store.record.RelationshipGroupRecord;
import org.neo4j.kernel.impl.transaction.log.ReadableLogChannel;

import static org.hamcrest.Matchers.instanceOf;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.neo4j.kernel.impl.transaction.command.Command.RelationshipGroupCommand;

public class PhysicalLogCommandReadersTest
{
private static final long ID = 42;
private static final byte IN_USE_FLAG = Record.IN_USE.byteValue();
private static final short TYPE = (short) (Short.MAX_VALUE + 42);
private static final int TYPE_AS_INT = TYPE & 0xFFFF;
private static final long NEXT = 42;
private static final long FIRST_OUT = 42;
private static final long FIRST_IN = 42;
private static final long FIRST_LOOP = 42;
private static final long OWNING_NODE = 42;

@Test
public void readRelGroupWithHugeTypeInV1_9()
{
assertDoesNotKnowAboutRelGroups( new PhysicalLogNeoCommandReaderV1_9() );
}

@Test
public void readRelGroupWithHugeTypeInV2_0()
{
assertDoesNotKnowAboutRelGroups( new PhysicalLogNeoCommandReaderV2_0() );
}

@Test
public void readRelGroupWithHugeTypeInV2_1() throws IOException
{
assertCanReadRelGroup( new PhysicalLogNeoCommandReaderV2_1() );
}

@Test
public void readRelGroupWithHugeTypeInV2_2() throws IOException
{
assertCanReadRelGroup( new PhysicalLogNeoCommandReaderV2_2() );
}

@Test
public void readRelGroupWithHugeTypeInV2_2_4() throws IOException
{
assertCanReadRelGroup( new PhysicalLogNeoCommandReaderV2_2_4() );
}

@Test
public void readRelGroupWithHugeTypeInV2_2_10() throws IOException
{
assertCanReadRelGroup( new PhysicalLogCommandReaderV2_2_10() );
}

private static void assertDoesNotKnowAboutRelGroups( CommandReader reader )
{
try
{
reader.read( channelWithRelGroupRecord() );
fail( "Exception expected" );
}
catch ( IOException e )
{
assertEquals( "Unknown command type[" + NeoCommandType.REL_GROUP_COMMAND + "]", e.getMessage() );
}
}

private void assertCanReadRelGroup( CommandReader reader ) throws IOException
{
Command command = reader.read( channelWithRelGroupRecord() );
assertValidRelGroupCommand( command );
}

private static void assertValidRelGroupCommand( Command command )
{
assertThat( command, instanceOf( RelationshipGroupCommand.class ) );
RelationshipGroupCommand relGroupCommand = (RelationshipGroupCommand) command;
RelationshipGroupRecord record = relGroupCommand.getRecord();

assertEquals( ID, record.getId() );
if ( IN_USE_FLAG == Record.IN_USE.byteValue() )
{
assertTrue( record.inUse() );
}
else if ( IN_USE_FLAG == Record.NOT_IN_USE.byteValue() )
{
assertFalse( record.inUse() );
}
else
{
throw new IllegalStateException( "Illegal inUse flag: " + IN_USE_FLAG );
}
assertEquals( TYPE_AS_INT, record.getType() );
assertEquals( NEXT, record.getNext() );
assertEquals( FIRST_OUT, record.getFirstOut() );
assertEquals( FIRST_IN, record.getFirstIn() );
assertEquals( FIRST_LOOP, record.getNext() );
assertEquals( OWNING_NODE, record.getOwningNode() );
}

private static ReadableLogChannel channelWithRelGroupRecord() throws IOException
{
return channelWithRelGroupRecord( ID, IN_USE_FLAG, TYPE, NEXT, FIRST_OUT, FIRST_IN, FIRST_LOOP, OWNING_NODE );
}

private static ReadableLogChannel channelWithRelGroupRecord( long id, byte inUse, short type, long next,
long firstOut, long firstIn, long firstLoop, long owningNode ) throws IOException
{
ReadableLogChannel channel = mock( ReadableLogChannel.class );

when( channel.get() ).thenReturn( NeoCommandType.REL_GROUP_COMMAND ).thenReturn( inUse );
when( channel.getLong() ).thenReturn( id ).thenReturn( next ).thenReturn( firstOut ).thenReturn( firstIn )
.thenReturn( firstLoop ).thenReturn( owningNode );
when( channel.getShort() ).thenReturn( type );

return channel;
}
}

0 comments on commit 34db227

Please sign in to comment.