Skip to content

Commit

Permalink
[#513] Add consistent numBits check to readers in all runtime libraries
Browse files Browse the repository at this point in the history
  • Loading branch information
@Mi-La
Mi-La committed Jun 19, 2024
1 parent 3908c0b commit c0f9202
Show file tree
Hide file tree
Showing 8 changed files with 190 additions and 99 deletions.
4 changes: 2 additions & 2 deletions compiler/extensions/cpp/runtime/src/zserio/BitStreamReader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ inline void throwNumBitsIsNotValid(uint8_t numBits)
/** Checks numBits validity for 32-bit reads. */
inline void checkNumBits(uint8_t numBits)
{
if (numBits > 32)
if (numBits == 0 || numBits > 32)
{
throwNumBitsIsNotValid(numBits);
}
Expand All @@ -216,7 +216,7 @@ inline void checkNumBits(uint8_t numBits)
/** Checks numBits validity for 64-bit reads. */
inline void checkNumBits64(uint8_t numBits)
{
if (numBits > 64)
if (numBits == 0 || numBits > 64)
{
throwNumBitsIsNotValid(numBits);
}
Expand Down
45 changes: 36 additions & 9 deletions compiler/extensions/cpp/runtime/test/zserio/ArrayTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,10 @@ class ArrayTest : public ::testing::Test
ASSERT_EQ(i + bitSize, writer.getBitPosition());

BitStreamReader reader(m_byteBuffer.data(), writer.getBitPosition(), BitsTag());
ASSERT_EQ(0, reader.readBits(i));
if (i > 0)
{
ASSERT_EQ(0, reader.readBits(i));
}
ArrayT readArray;
arrayRead(readArray, owner, reader, rawArray.size());
ASSERT_EQ(array, readArray);
Expand Down Expand Up @@ -394,7 +397,10 @@ class ArrayTest : public ::testing::Test
ASSERT_EQ(i + bitSize, writer.getBitPosition());

BitStreamReader reader(m_byteBuffer.data(), writer.getBitPosition(), BitsTag());
ASSERT_EQ(0, reader.readBits(i));
if (i > 0)
{
ASSERT_EQ(0, reader.readBits(i));
}
ArrayT readArray;
arrayRead(readArray, owner, reader);
ASSERT_EQ(array, readArray);
Expand Down Expand Up @@ -434,7 +440,10 @@ class ArrayTest : public ::testing::Test
ASSERT_EQ(i + bitSize, writer.getBitPosition());

BitStreamReader reader(m_byteBuffer.data(), writer.getBitPosition(), BitsTag());
ASSERT_EQ(0, reader.readBits(i));
if (i > 0)
{
ASSERT_EQ(0, reader.readBits(i));
}
ArrayT readArray;
arrayRead(readArray, owner, reader, rawArray.size());
ASSERT_EQ(array, readArray);
Expand Down Expand Up @@ -475,7 +484,10 @@ class ArrayTest : public ::testing::Test
ASSERT_EQ(i + bitSize, writer.getBitPosition());

BitStreamReader reader(m_byteBuffer.data(), writer.getBitPosition(), BitsTag());
ASSERT_EQ(0, reader.readBits(i));
if (i > 0)
{
ASSERT_EQ(0, reader.readBits(i));
}
ArrayT readArray;
arrayRead(readArray, owner, reader);
ASSERT_EQ(array, readArray);
Expand Down Expand Up @@ -514,7 +526,10 @@ class ArrayTest : public ::testing::Test
ASSERT_EQ(i + bitSize, writer.getBitPosition());

BitStreamReader reader(m_byteBuffer.data(), writer.getBitPosition(), BitsTag());
ASSERT_EQ(0, reader.readBits(i));
if (i > 0)
{
ASSERT_EQ(0, reader.readBits(i));
}
ArrayT readArray;
arrayRead(readArray, owner, reader);
ASSERT_EQ(array, readArray);
Expand Down Expand Up @@ -557,7 +572,10 @@ class ArrayTest : public ::testing::Test
ASSERT_EQ(i + bitSize, writer.getBitPosition());

BitStreamReader reader(m_byteBuffer.data(), writer.getBitPosition(), BitsTag());
ASSERT_EQ(0, reader.readBits(i));
if (i > 0)
{
ASSERT_EQ(0, reader.readBits(i));
}
ArrayT readArray;
arrayReadPacked(readArray, owner, reader, rawArray.size());
ASSERT_EQ(array, readArray);
Expand Down Expand Up @@ -593,7 +611,10 @@ class ArrayTest : public ::testing::Test
ASSERT_EQ(i + bitSize, writer.getBitPosition());

BitStreamReader reader(m_byteBuffer.data(), writer.getBitPosition(), BitsTag());
ASSERT_EQ(0, reader.readBits(i));
if (i > 0)
{
ASSERT_EQ(0, reader.readBits(i));
}
ArrayT readArray;
arrayReadPacked(readArray, owner, reader);
ASSERT_EQ(array, readArray);
Expand Down Expand Up @@ -629,7 +650,10 @@ class ArrayTest : public ::testing::Test
ASSERT_EQ(i + bitSize, writer.getBitPosition());

BitStreamReader reader(m_byteBuffer.data(), writer.getBitPosition(), BitsTag());
ASSERT_EQ(0, reader.readBits(i));
if (i > 0)
{
ASSERT_EQ(0, reader.readBits(i));
}
ArrayT readArray;
arrayReadPacked(readArray, owner, reader, rawArray.size());
ASSERT_EQ(array, readArray);
Expand Down Expand Up @@ -665,7 +689,10 @@ class ArrayTest : public ::testing::Test
ASSERT_EQ(i + bitSize, writer.getBitPosition());

BitStreamReader reader(m_byteBuffer.data(), writer.getBitPosition(), BitsTag());
ASSERT_EQ(0, reader.readBits(i));
if (i > 0)
{
ASSERT_EQ(0, reader.readBits(i));
}
ArrayT readArray;
arrayReadPacked(readArray, owner, reader);
ASSERT_EQ(array, readArray);
Expand Down
21 changes: 8 additions & 13 deletions compiler/extensions/cpp/runtime/test/zserio/BitStreamReaderTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,10 @@ TEST_F(BitStreamReaderTest, readUnalignedData)
BitStreamReader reader(buffer);

// read offset bits
ASSERT_EQ(0, reader.readBits64(offset));
if (offset > 0)
{
ASSERT_EQ(0, reader.readBits64(offset));
}

// read magic number
ASSERT_EQ(testValue, reader.readBits(8)) << "Offset: " << offset;
Expand All @@ -113,40 +116,32 @@ TEST_F(BitStreamReaderTest, readBits)
{
// check invalid bitlength acceptance
ASSERT_THROW(m_reader.readBits(255), CppRuntimeException);
ASSERT_THROW(m_reader.readBits(0), CppRuntimeException);
ASSERT_THROW(m_reader.readBits(33), CppRuntimeException);

// return 0 for 0 bits
ASSERT_EQ(0, m_reader.readBits(0));
}

TEST_F(BitStreamReaderTest, readBits64)
{
// check invalid bit length acceptance
ASSERT_THROW(m_reader.readBits64(255), CppRuntimeException);
ASSERT_THROW(m_reader.readBits64(0), CppRuntimeException);
ASSERT_THROW(m_reader.readBits64(65), CppRuntimeException);

// return 0 for 0 bits
ASSERT_EQ(0, m_reader.readBits64(0));
}

TEST_F(BitStreamReaderTest, readSignedBits)
{
// check invalid bit length acceptance
ASSERT_THROW(m_reader.readSignedBits(255), CppRuntimeException);
ASSERT_THROW(m_reader.readSignedBits(0), CppRuntimeException);
ASSERT_THROW(m_reader.readSignedBits(33), CppRuntimeException);

// return 0 for 0 bits
ASSERT_EQ(0, m_reader.readSignedBits(0));
}

TEST_F(BitStreamReaderTest, readSignedBits64)
{
// check invalid bit length acceptance
ASSERT_THROW(m_reader.readSignedBits64(255), CppRuntimeException);
ASSERT_THROW(m_reader.readSignedBits64(0), CppRuntimeException);
ASSERT_THROW(m_reader.readSignedBits64(65), CppRuntimeException);

// return 0 for 0 bits
ASSERT_EQ(0, m_reader.readSignedBits64(0));
}

TEST_F(BitStreamReaderTest, readVarSize)
Expand Down
109 changes: 58 additions & 51 deletions compiler/extensions/java/runtime/src/zserio/runtime/io/ByteArrayBitStreamReader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ public ByteArrayBitStreamReader(final byte[] bytes, long bitSize)
@Override
public long readSignedBits(final int numBits) throws IOException
{
long result = readBits(numBits);
checkRange(numBits);
long result = readBitsImpl(numBits);

/*
* Perform a sign extension if needed.
Expand All @@ -69,55 +70,8 @@ public long readSignedBits(final int numBits) throws IOException
@Override
public long readBits(final int numBits) throws IOException
{
checkRange(numBits);

int bitsToRead = bitOffset + numBits;
long accum = nextUnsignedByte() & BIT_MASKS[bitOffset];
bitsToRead -= 8;

if (bitsToRead < 0)
{
// less than already read byte is needed
accum = accum >>> -bitsToRead;
bytePosition--; // consumed only few bits
}
else
{
// full bytes
while (bitsToRead >= 8)
{
accum = (accum << 8) | nextUnsignedByte();
bitsToRead -= 8;
}

// last few bits
if (bitsToRead > 0)
{
accum = (accum << bitsToRead) | (nextUnsignedByte() >>> (8 - bitsToRead));
bytePosition--; // consumed only few bits
}
}

bitOffset = (bitOffset + numBits) & BYTE_MOD_MASK;

if (bytePosition == buffer.length) // consumed full last byte
{
if (lastByteBits < 8) // check if whole byte is available
{
throw new IOException("ByteArrayBitStreamReader: Unable to read bit on offset position " +
lastByteBits + " in the last byte.");
}
}
else if (bytePosition + 1 >= buffer.length) // consumed last byte only partially or not at all
{
if (bitOffset > lastByteBits) // check if we didn't read more bits than available
{
throw new IOException("ByteArrayBitStreamReader: Unable to read bit on offset position " +
bitOffset + " in the last byte.");
}
}

return accum;
checkUnsignedRange(numBits);
return readBitsImpl(numBits);
}

@Override
Expand Down Expand Up @@ -207,14 +161,16 @@ public long readLong() throws IOException
}
else
{
result = readBits(64);
result = readSignedBits(64);
}
return result;
}

@Override
public BigInteger readBigInteger(final int numBits) throws IOException
{
checkRange(numBits);

BigInteger result = BigInteger.ZERO;
int bitsToRead = numBits;
if (bitsToRead > 8)
Expand Down Expand Up @@ -674,6 +630,57 @@ protected byte[] getBuffer()
return buffer;
}

private long readBitsImpl(final int numBits) throws IOException
{
int bitsToRead = bitOffset + numBits;
long accum = nextUnsignedByte() & BIT_MASKS[bitOffset];
bitsToRead -= 8;

if (bitsToRead < 0)
{
// less than already read byte is needed
accum = accum >>> -bitsToRead;
bytePosition--; // consumed only few bits
}
else
{
// full bytes
while (bitsToRead >= 8)
{
accum = (accum << 8) | nextUnsignedByte();
bitsToRead -= 8;
}

// last few bits
if (bitsToRead > 0)
{
accum = (accum << bitsToRead) | (nextUnsignedByte() >>> (8 - bitsToRead));
bytePosition--; // consumed only few bits
}
}

bitOffset = (bitOffset + numBits) & BYTE_MOD_MASK;

if (bytePosition == buffer.length) // consumed full last byte
{
if (lastByteBits < 8) // check if whole byte is available
{
throw new IOException("ByteArrayBitStreamReader: Unable to read bit on offset position " +
lastByteBits + " in the last byte.");
}
}
else if (bytePosition + 1 >= buffer.length) // consumed last byte only partially or not at all
{
if (bitOffset > lastByteBits) // check if we didn't read more bits than available
{
throw new IOException("ByteArrayBitStreamReader: Unable to read bit on offset position " +
bitOffset + " in the last byte.");
}
}

return accum;
}

private void readFully(final byte[] b) throws IOException
{
readFully(b, 0, b.length);
Expand Down
Loading

0 comments on commit c0f9202

Please sign in to comment.