ssg / SimpleBase

.NET library for encoding/decoding Base16, Base32, Base58 and Base85.
Apache License 2.0
147 stars 21 forks source link

Unexpected return value from TryDecode of previously padded encoded value #13

Closed skwasjer closed 4 years ago

skwasjer commented 4 years ago

I may have stumbled onto an issue with (at least) Base32.TryDecode.

I noticed TryDecode seems to use a different input length:

https://github.com/ssg/SimpleBase/blob/master/src/Base32.cs#L267

public unsafe bool TryDecode(ReadOnlySpan<char> input, Span<byte> output, out int numBytesWritten)
{
    int inputLen = input.Length;
    ...
    fixed (char* inputPtr = input)
    fixed (byte* outputPtr = output)
    {
        return internalDecode(inputPtr, inputLen, outputPtr, outputLen, out numBytesWritten);
    }
}

vs Decode

public unsafe Span<byte> Decode(ReadOnlySpan<char> text)
{
    int textLen = text.Length - getPaddingCharCount(text);
    ...
    fixed (byte* outputPtr = outputBuffer)
    fixed (char* inputPtr = text)
    {
        if (!internalDecode(inputPtr, textLen, outputPtr, outputLen, out _))
        {
            throw new ArgumentException("Invalid input or output", nameof(text));
        }
    }
    ...
}

I gobbled the code below together illustrating the issue. In the first test case all is good, Decode and TryDecode work ok. In the second test case Decode of the padded value succeeds, but the TryDecode return value signals otherwise (the output buffer and bytes written however is correct).

[Theory]
[InlineData(1155905301338513408, false, "03G4AM5ZK4510")]
[InlineData(1155905301338513408, true, "03G4AM5ZK4510===")]
public void Test(long longValue, bool pad, string expectedEncodedStr)
{
    Base32 encoder = Base32.Crockford;

    string encodedStr = encoder.Encode(BitConverter.GetBytes(longValue), pad);
    encodedStr.Should().Be(expectedEncodedStr);

        // Decode.
    long decodedLongValue = BitConverter.ToInt64(encoder.Decode(encodedStr));
    decodedLongValue.Should().Be(longValue);

        // TryDecode
    var output = new byte[encoder.GetSafeByteCountForDecoding(encodedStr)];
    bool decodeSuccess = encoder.TryDecode(encodedStr, output, out int numBytesWritten);

    numBytesWritten.Should().Be(8);
    BitConverter.ToInt64(output).Should().Be(longValue);
    decodeSuccess.Should().BeTrue(); // Fails for second test case.
}
ssg commented 4 years ago

Great find! would you be willing to contribute a PR for tests and the fix?

ssg commented 4 years ago

Fix released in 3.0.1 and verified that it didn't exist in 2.x.