From 34db2275c87836a80eb839478b0b1eac83b49fa1 Mon Sep 17 00:00:00 2001 From: lutovich Date: Tue, 14 Jun 2016 13:34:02 +0200 Subject: [PATCH] Fix relationship group type overflow in command readers 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. --- .../PhysicalLogCommandReaderV2_2_10.java | 3 +- .../PhysicalLogNeoCommandReaderV2_1.java | 3 +- .../PhysicalLogNeoCommandReaderV2_2.java | 3 +- .../PhysicalLogNeoCommandReaderV2_2_4.java | 8 +- .../kernel/impl/util/IoPrimitiveUtils.java | 5 + .../PhysicalLogCommandReadersTest.java | 151 ++++++++++++++++++ 6 files changed, 164 insertions(+), 9 deletions(-) create mode 100644 community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/command/PhysicalLogCommandReadersTest.java diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/PhysicalLogCommandReaderV2_2_10.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/PhysicalLogCommandReaderV2_2_10.java index be27cc2b2be26..e3b455110c6d7 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/PhysicalLogCommandReaderV2_2_10.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/PhysicalLogCommandReaderV2_2_10.java @@ -65,6 +65,7 @@ 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.read3bLengthAndString; +import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.shortToUnsignedInt; public class PhysicalLogCommandReaderV2_2_10 implements CommandReader, NeoCommandHandler { @@ -252,7 +253,7 @@ public boolean visitRelationshipGroupCommand( Command.RelationshipGroupCommand c { throw new IOException( "Illegal in use flag: " + inUseByte ); } - int type = channel.getShort(); + int type = shortToUnsignedInt( channel.getShort() ); RelationshipGroupRecord record = new RelationshipGroupRecord( id, type ); record.setInUse( inUse ); record.setNext( channel.getLong() ); diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/PhysicalLogNeoCommandReaderV2_1.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/PhysicalLogNeoCommandReaderV2_1.java index 1ab2521d18811..abede7cd58bd5 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/PhysicalLogNeoCommandReaderV2_1.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/PhysicalLogNeoCommandReaderV2_1.java @@ -65,6 +65,7 @@ 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.read3bLengthAndString; +import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.shortToUnsignedInt; public class PhysicalLogNeoCommandReaderV2_1 implements CommandReader { @@ -309,7 +310,7 @@ public boolean visitRelationshipGroupCommand( Command.RelationshipGroupCommand c { throw new IOException( "Illegal in use flag: " + inUseByte ); } - int type = channel.getShort(); + int type = shortToUnsignedInt( channel.getShort() ); RelationshipGroupRecord record = new RelationshipGroupRecord( id, type ); record.setInUse( inUse ); record.setNext( channel.getLong() ); diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/PhysicalLogNeoCommandReaderV2_2.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/PhysicalLogNeoCommandReaderV2_2.java index 820b3c393e88e..63ba0ab51f256 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/PhysicalLogNeoCommandReaderV2_2.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/PhysicalLogNeoCommandReaderV2_2.java @@ -65,6 +65,7 @@ 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.read3bLengthAndString; +import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.shortToUnsignedInt; public class PhysicalLogNeoCommandReaderV2_2 implements CommandReader { @@ -304,7 +305,7 @@ public boolean visitRelationshipGroupCommand( Command.RelationshipGroupCommand c { throw new IOException( "Illegal in use flag: " + inUseByte ); } - int type = channel.getShort(); + int type = shortToUnsignedInt( channel.getShort() ); RelationshipGroupRecord record = new RelationshipGroupRecord( id, type ); record.setInUse( inUse ); record.setNext( channel.getLong() ); diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/PhysicalLogNeoCommandReaderV2_2_4.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/PhysicalLogNeoCommandReaderV2_2_4.java index d0ece7f3ce650..ff15f5ecc6cd7 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/PhysicalLogNeoCommandReaderV2_2_4.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/PhysicalLogNeoCommandReaderV2_2_4.java @@ -65,6 +65,7 @@ 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.read3bLengthAndString; +import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.shortToUnsignedInt; public class PhysicalLogNeoCommandReaderV2_2_4 implements CommandReader, NeoCommandHandler { @@ -252,7 +253,7 @@ public boolean visitRelationshipGroupCommand( Command.RelationshipGroupCommand c { throw new IOException( "Illegal in use flag: " + inUseByte ); } - int type = getUnsignedShort( channel.getShort() ); + int type = shortToUnsignedInt( channel.getShort() ); RelationshipGroupRecord record = new RelationshipGroupRecord( id, type ); record.setInUse( inUse ); record.setNext( channel.getLong() ); @@ -752,9 +753,4 @@ public void apply() public void close() { // Nothing to close } - - private int getUnsignedShort( short value ) - { - return value & 0xFFFF; - } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/util/IoPrimitiveUtils.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/util/IoPrimitiveUtils.java index 7b1ec7129641b..ee5a2e227f2ef 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/util/IoPrimitiveUtils.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/util/IoPrimitiveUtils.java @@ -266,4 +266,9 @@ public static int safeCastLongToInt( long value ) } return (int) value; } + + public static int shortToUnsignedInt( short value ) + { + return value & 0xFFFF; + } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/command/PhysicalLogCommandReadersTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/command/PhysicalLogCommandReadersTest.java new file mode 100644 index 0000000000000..e426c41e5ba4d --- /dev/null +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/command/PhysicalLogCommandReadersTest.java @@ -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 . + */ +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; + } +}