jefffhaynes / BinarySerializer

A declarative serialization framework for controlling formatting of data at the byte and bit level using field bindings, converters, and code.
MIT License
290 stars 62 forks source link

FieldBitLength not respecting bit length if assigned value exceeding bit storage #209

Closed bevanweiss closed 1 year ago

bevanweiss commented 1 year ago

Perhaps this is just me expecting something that isn't quite right. But my thought here is that if I define a Field as being 4 bits long, then it shouldn't be allowed to exceed that length, regardless of what value is attempted to be assigned to it in the deserialized form.

I've got a short little test case below that illustrates this. It uses the existing BitLengthTest class (and the InternalBitLengthValueClass as the simplest example). This class is defined with two members, each with 4 bits to make up a single byte.

    [TestClass]
    public class BitLengthTests : TestBase
    {
        ....
        [TestMethod]
        public void LengthOverrunTest()
        {
            var expectedHighBits = new InternalBitLengthValueClass
            {
                Value = 0,
                Value2 = 0x7FFFFFFF,
            };
            var actualHighBits = Roundtrip(expectedHighBits, new byte[] { 0xF0 });
            Assert.AreEqual(expectedHighBits.Value & 0x0F, actualHighBits.Value);
            Assert.AreEqual(expectedHighBits.Value2 & 0x0F, actualHighBits.Value2);

            var expectedLowBits = new InternalBitLengthValueClass
            {
                Value = 0x7FFFFFFF,
                Value2 = 0,
            };
            var actualLowBits = Roundtrip(expectedLowBits, new byte[] { 0x0F });
            Assert.AreEqual(expectedLowBits.Value & 0x0F, actualLowBits.Value);
            Assert.AreEqual(expectedLowBits.Value2 & 0x0F, actualLowBits.Value2);
        }
        ....
    }

The result from the test case is as below

  Message: 
Assert.IsTrue failed. Value at position 0 does not match expected value.  Expected 0x0F, got 0xFF

  Stack Trace: 
TestBase.AssertEqual(Byte[] expected, Byte[] actual) line 112
TestBase.Roundtrip[T](T o, Byte[] expectedValue) line 82
BitLengthTests.LengthOverrunTest() line 47

  Standard Output: 

Debug Trace:
S-BinarySerialization.Test.BitLength.InternalBitLengthValueClass
    S-Start: Value @ 0
    S-End: Value (0) @ 4 bits
    S-Start: Value2 @ 4 bits
    S-End: Value2 (2147483647) @ 1
D-BinarySerialization.Test.BitLength.InternalBitLengthValueClass
    D-Start: Value @ 0
    D-End: Value (0) @ 4 bits
    D-Start: Value2 @ 4 bits
    D-End: Value2 (15) @ 1
S-BinarySerialization.Test.BitLength.InternalBitLengthValueClass
    S-Start: Value @ 0
    S-End: Value (2147483647) @ 4 bits
    S-Start: Value2 @ 4 bits
    S-End: Value2 (0) @ 1

This appears to show the serialized field being allowed to extend beyond the 4 bits it's defined as.

My expectation is that the result is the bit mask of the bits defined for the field, and the value applied to the member. This would be consistent with things like:

byte val = (byte)0x22FF;
// val will equal 0xFF here
bevanweiss commented 1 year ago

I think the issue here is that the BoundedStream isn't masking the value before OR'ing it into the _bitBuffer.

If I mask these bits then there's a failing test in the LengthTest however. The Internal2 is currently expected to serialize to 0xAA, which I think is incorrect, since the inner field is defined as BitLength(4), so it should only fill 4 bits of the byte.