diff --git a/src/Adaptive.Agrona.Tests/AsciiStringBufferCharWidthTests.cs b/src/Adaptive.Agrona.Tests/AsciiStringBufferCharWidthTests.cs new file mode 100644 index 00000000..4d6985fe --- /dev/null +++ b/src/Adaptive.Agrona.Tests/AsciiStringBufferCharWidthTests.cs @@ -0,0 +1,142 @@ +/* + * Copyright 2014 - 2026 Adaptive Financial Consulting Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using System.Text; +using Adaptive.Agrona.Concurrent; +using NUnit.Framework; + +namespace Adaptive.Agrona.Tests +{ + /// + /// Regression tests for a Java-to-.NET porting bug in and + /// where ASCII string helpers used + /// *(char*)(buf + i) = c in a byte-indexed loop. Because a .NET char + /// is 2 bytes, that: + /// (1) overran the bounds-checked region by one byte in + /// PutStringWithoutLengthAscii, and + /// (2) read the same 2 bytes every iteration in + /// GetStringAscii(int, StringBuilder) because the offset + /// expression mistakenly used index instead of the loop cursor. + /// + [TestFixture] + public class AsciiStringBufferCharWidthTests + { + // ---- UnsafeBuffer ---- + + [Test] + public void UnsafeBuffer_PutStringWithoutLengthAscii_DoesNotOverrunBoundsByOneByte() + { + byte[] backing = { 0xAA, 0xAA, 0xAA, 0xAA }; + var buffer = new UnsafeBuffer(backing); + + int written = buffer.PutStringWithoutLengthAscii(0, "abc"); + + Assert.AreEqual(3, written); + Assert.AreEqual((byte)'a', backing[0]); + Assert.AreEqual((byte)'b', backing[1]); + Assert.AreEqual((byte)'c', backing[2]); + // With the pre-fix code this byte was clobbered to 0x00 by the + // trailing high-byte of the final 2-byte char write. + Assert.AreEqual(0xAA, backing[3], "sentinel byte at offset==length must not be overwritten"); + } + + [Test] + public void UnsafeBuffer_PutStringWithoutLengthAscii_WithValueOffsetAndLength_DoesNotOverrun() + { + byte[] backing = { 0xAA, 0xAA, 0xAA, 0xAA, 0xAA }; + var buffer = new UnsafeBuffer(backing); + + int written = buffer.PutStringWithoutLengthAscii(0, "xabcdx", 1, 4); + + Assert.AreEqual(4, written); + Assert.AreEqual((byte)'a', backing[0]); + Assert.AreEqual((byte)'b', backing[1]); + Assert.AreEqual((byte)'c', backing[2]); + Assert.AreEqual((byte)'d', backing[3]); + Assert.AreEqual(0xAA, backing[4], "sentinel byte at offset==length must not be overwritten"); + } + + [Test] + public void UnsafeBuffer_GetStringAscii_StringBuilder_AdvancesReadOffset() + { + byte[] backing = new byte[16]; + var buffer = new UnsafeBuffer(backing); + + buffer.PutStringAscii(0, "abcd"); + + var sb = new StringBuilder(); + buffer.GetStringAscii(0, sb); + + // With the pre-fix code, every loop iteration read from `index` + // (the length prefix), so the result was 4 copies of ''. + Assert.AreEqual("abcd", sb.ToString()); + } + + // ---- ExpandableArrayBuffer ---- + + [Test] + public void ExpandableArrayBuffer_PutStringWithoutLengthAscii_DoesNotOverrunBoundsByOneByte() + { + var buffer = new ExpandableArrayBuffer(16); + byte[] backing = buffer.ByteArray; + for (int i = 0; i < backing.Length; i++) + { + backing[i] = 0xAA; + } + + int written = buffer.PutStringWithoutLengthAscii(0, "abc"); + + Assert.AreEqual(3, written); + Assert.AreEqual((byte)'a', backing[0]); + Assert.AreEqual((byte)'b', backing[1]); + Assert.AreEqual((byte)'c', backing[2]); + Assert.AreEqual(0xAA, backing[3], "sentinel byte at offset==length must not be overwritten"); + } + + [Test] + public void ExpandableArrayBuffer_PutStringWithoutLengthAscii_WithValueOffsetAndLength_DoesNotOverrun() + { + var buffer = new ExpandableArrayBuffer(16); + byte[] backing = buffer.ByteArray; + for (int i = 0; i < backing.Length; i++) + { + backing[i] = 0xAA; + } + + int written = buffer.PutStringWithoutLengthAscii(0, "xabcdx", 1, 4); + + Assert.AreEqual(4, written); + Assert.AreEqual((byte)'a', backing[0]); + Assert.AreEqual((byte)'b', backing[1]); + Assert.AreEqual((byte)'c', backing[2]); + Assert.AreEqual((byte)'d', backing[3]); + Assert.AreEqual(0xAA, backing[4], "sentinel byte at offset==length must not be overwritten"); + } + + [Test] + public void ExpandableArrayBuffer_GetStringAscii_StringBuilder_AdvancesReadOffset() + { + var buffer = new ExpandableArrayBuffer(16); + + buffer.PutStringAscii(0, "abcd"); + + var sb = new StringBuilder(); + buffer.GetStringAscii(0, sb); + + Assert.AreEqual("abcd", sb.ToString()); + } + } +} diff --git a/src/Adaptive.Agrona/Concurrent/UnsafeBuffer.cs b/src/Adaptive.Agrona/Concurrent/UnsafeBuffer.cs index c1581ab7..8894874c 100644 --- a/src/Adaptive.Agrona/Concurrent/UnsafeBuffer.cs +++ b/src/Adaptive.Agrona/Concurrent/UnsafeBuffer.cs @@ -828,7 +828,7 @@ public int GetStringAscii(int index, int length, StringBuilder appendable) { for (int i = index + BitUtil.SIZE_OF_INT, limit = index + BitUtil.SIZE_OF_INT + length; i < limit; i++) { - char c = *(char*)(_pBuffer + index); + char c = (char)*(_pBuffer + i); appendable.Append(c > (char)127 ? '?' : c); } @@ -879,7 +879,7 @@ public int PutStringWithoutLengthAscii(int index, string value) c = '?'; } - *(char*)(_pBuffer + index + i) = c; + *(_pBuffer + index + i) = (byte)c; } return length; @@ -899,7 +899,7 @@ public int PutStringWithoutLengthAscii(int index, string value, int valueOffset, c = '?'; } - *(char*)(_pBuffer + index + i) = c; + *(_pBuffer + index + i) = (byte)c; } return len; diff --git a/src/Adaptive.Agrona/ExpandableArrayBuffer.cs b/src/Adaptive.Agrona/ExpandableArrayBuffer.cs index 8ff6a115..4aeb153b 100644 --- a/src/Adaptive.Agrona/ExpandableArrayBuffer.cs +++ b/src/Adaptive.Agrona/ExpandableArrayBuffer.cs @@ -258,7 +258,7 @@ public int GetStringAscii(int index, int length, StringBuilder appendable) { for (int i = index + BitUtil.SIZE_OF_INT, limit = index + BitUtil.SIZE_OF_INT + length; i < limit; i++) { - char c = *(char*)(_pBuffer + index); + char c = (char)*(_pBuffer + i); appendable.Append(c > (char)127 ? '?' : c); } @@ -472,7 +472,7 @@ public int PutStringWithoutLengthAscii(int index, string value) c = '?'; } - *(char*)(_pBuffer + index + i) = c; + *(_pBuffer + index + i) = (byte)c; } return length; @@ -493,7 +493,7 @@ public int PutStringWithoutLengthAscii(int index, string value, int valueOffset, c = '?'; } - *(char*)(_pBuffer + index + i) = c; + *(_pBuffer + index + i) = (byte)c; } return len;