[go] match fixed length string behavior of java/c++

CHANGELOG.adoc

@@ -6,6 +6,13 @@
 
 * **[Java]** Reject negative choice values in `SetType`. (https://github.com/aeron-io/simple-binary-encoding/pull/1112[#1112])
 * **[C{plus}{plus}]** Expose schema version from `IrDecoder`. (https://github.com/aeron-io/simple-binary-encoding/pull/1117[#1117]).
+* **[Go]** Fix padding on fixed length character arrays so they are null padded in the Go flyweight codec, matching
+the Java and C{plus}{plus} codecs. Setters now also reject values longer than the field rather than overrunning into
+the following field. (https://github.com/aeron-io/simple-binary-encoding/pull/1119[#1119])
+* **[Go]** `RangeCheck` in the Go struct codec no longer rejects null padded fixed length character arrays. `NullValue`
+for `char` is `0`, which sits below `MinValue` of `0x20`, so null padding written by any codec previously failed
+validation, including when decoding a buffer produced by the Java or C{plus}{plus} codec.
+(https://github.com/aeron-io/simple-binary-encoding/pull/1119[#1119])
 
 == 1.40.0 (2026-08-21)
 
@@ -1231,4 +1238,4 @@ $ java --add-opens java.base/jdk.internal.misc=ALL-UNNAMED -Dsbe.generate.ir=tru
 * Project structure updated to be a Gradle modular build.
 * RC4 features are available for testing if appropriate RC4 XSD schema is provided or schema validation is turned off.
 * Generated code is marked with the @Generated annotation.
-* C{plus}{plus} updated to C{plus}{plus} 11.
+* C{plus}{plus} updated to C{plus}{plus} 11.

gocode/flyweight/src/composite/Composite_test.go

@@ -1,6 +1,7 @@
 package composite
 
 import (
+	"bytes"
 	"testing"
 )
 
@@ -50,3 +51,124 @@ func TestEncodeDecode(t *testing.T) {
 		t.Fail()
 	}
 }
+
+// wrapDirtyComposite wraps a buffer pre-filled with a non-zero sentinel, so that a setter which fails to pad
+// cannot pass by accident on a freshly zeroed buffer.
+func wrapDirtyComposite(t *testing.T, data []byte) *Composite {
+	t.Helper()
+
+	for i := range data {
+		data[i] = 0xFF
+	}
+
+	var in Composite
+	in.WrapForEncode(data, 0, uint64(len(data)))
+
+	return &in
+}
+
+// name is a char[5], so every value shorter than 5 bytes must leave the remainder of the field NUL filled.
+func TestSetNamePadsShortStringWithNul(t *testing.T) {
+	tests := []struct {
+		value    string
+		expected []byte
+	}{
+		{"start", []byte{'s', 't', 'a', 'r', 't'}},
+		{"end", []byte{'e', 'n', 'd', 0, 0}},
+		{"a", []byte{'a', 0, 0, 0, 0}},
+		{"", []byte{0, 0, 0, 0, 0}},
+	}
+
+	for _, test := range tests {
+		var data [256]byte
+		point := wrapDirtyComposite(t, data[:]).Start()
+
+		point.SetName(test.value)
+
+		if actual := point.Name(); !bytes.Equal(actual, test.expected) {
+			t.Errorf("SetName(%q) encoded %v, expected %v", test.value, actual, test.expected)
+		}
+		if actual := point.GetNameAsString(); actual != test.value {
+			t.Errorf("SetName(%q) decoded as %q", test.value, actual)
+		}
+	}
+}
+
+// Overwriting a longer value with a shorter one must not leave the tail of the previous value behind.
+func TestSetNameOverwriteDoesNotLeakPreviousValue(t *testing.T) {
+	var data [256]byte
+	point := wrapDirtyComposite(t, data[:]).Start()
+
+	point.SetName("start")
+	point.SetName("end")
+
+	if actual := point.GetNameAsString(); actual != "end" {
+		t.Errorf("expected \"end\" after overwrite, got %q", actual)
+	}
+	if actual := point.Name(); !bytes.Equal(actual, []byte{'e', 'n', 'd', 0, 0}) {
+		t.Errorf("stale bytes left after overwrite: %v", actual)
+	}
+}
+
+// PutName shares the padding contract with SetName so the invariant holds whichever setter is used.
+func TestPutNamePadsShortValueWithNul(t *testing.T) {
+	var data [256]byte
+	point := wrapDirtyComposite(t, data[:]).Start()
+
+	point.PutName([]byte("start"))
+	point.PutName([]byte("end"))
+
+	if actual := point.Name(); !bytes.Equal(actual, []byte{'e', 'n', 'd', 0, 0}) {
+		t.Errorf("PutName left stale bytes: %v", actual)
+	}
+}
+
+// A value longer than the field must be rejected rather than overrun into the following field, matching the
+// Java and C++ codecs which throw.
+func TestSetNameTooLongPanics(t *testing.T) {
+	var data [256]byte
+	point := wrapDirtyComposite(t, data[:]).Start()
+
+	defer func() {
+		if recover() == nil {
+			t.Error("expected SetName to panic for a value longer than the field")
+		}
+	}()
+
+	point.SetName("toolong")
+}
+
+func TestPutNameTooLongPanics(t *testing.T) {
+	var data [256]byte
+	point := wrapDirtyComposite(t, data[:]).Start()
+
+	defer func() {
+		if recover() == nil {
+			t.Error("expected PutName to panic for a value longer than the field")
+		}
+	}()
+
+	point.PutName([]byte("toolong"))
+}
+
+// The padding must stop at the end of the field and not spill into whatever follows it.
+func TestSetNamePaddingDoesNotDisturbAdjacentFields(t *testing.T) {
+	var data [256]byte
+	in := wrapDirtyComposite(t, data[:])
+
+	in.Start().SetName("end").SetD(3.14).SetI(7)
+	in.End().SetName("x")
+
+	if actual := in.Start().GetNameAsString(); actual != "end" {
+		t.Errorf("start.name corrupted: %q", actual)
+	}
+	if actual := in.Start().D(); actual != 3.14 {
+		t.Errorf("start.d corrupted by name setter: %v", actual)
+	}
+	if actual := in.Start().I(); actual != 7 {
+		t.Errorf("start.i corrupted by name setter: %v", actual)
+	}
+	if actual := in.End().GetNameAsString(); actual != "x" {
+		t.Errorf("end.name corrupted: %q", actual)
+	}
+}

gocode/struct/src/baseline/Car_test.go

@@ -137,3 +137,102 @@ func TestDecodeJavaBuffer(t *testing.T) {
 	// fmt.Println(c)
 	return
 }
+
+// newRangeCheckableCar builds a Car that passes RangeCheck, so that tests below can vary a single
+// character array and attribute any failure to that field alone.
+func newRangeCheckableCar(vehicleCode [6]byte, manufacturerCode [3]byte) Car {
+	var optionalExtras [8]bool
+	optionalExtras[OptionalExtrasChoice.CruiseControl] = true
+
+	engine := Engine{2000, 4, 0, manufacturerCode, [6]byte{}, 42, BooleanType.T,
+		EngineBooster{BoostType.NITROUS, 200}}
+
+	fuel := []CarFuelFigures{{30, 35.9, []uint8("Urban Cycle")}}
+	acceleration := []CarPerformanceFiguresAcceleration{{30, 3.8}}
+	performance := []CarPerformanceFigures{{95, acceleration}}
+
+	return Car{1234, 2013, BooleanType.T, Model.A, [4]uint32{0, 1, 2, 3}, vehicleCode,
+		optionalExtras, Model.A, engine, fuel, performance,
+		[]uint8("Honda"), []uint8("Civic VTi"), []uint8("deadbeef")}
+}
+
+func makeVehicleCode(value string) [6]byte {
+	var code [6]byte
+	copy(code[:], value)
+
+	return code
+}
+
+// vehicleCode is a char[6]. NUL is the pad byte written by the Java, C++ and Go flyweight codecs, and is
+// also what the idiomatic `copy` into a zeroed Go array leaves behind, so RangeCheck has to accept it.
+// Space padding must keep working, and genuinely out of range bytes must still be rejected.
+func TestRangeCheckAcceptsPaddedCharArrays(t *testing.T) {
+	var manufacturerCode [3]byte
+	copy(manufacturerCode[:], "123")
+
+	tests := []struct {
+		name      string
+		code      [6]byte
+		wantError bool
+	}{
+		{"exact length", makeVehicleCode("abcdef"), false},
+		{"NUL padded", makeVehicleCode("abc"), false},
+		{"space padded", makeVehicleCode("abc   "), false},
+		{"all NUL", makeVehicleCode(""), false},
+		{"all space", makeVehicleCode("      "), false},
+		{"below range", [6]byte{'a', 'b', 'c', 0x01, 0, 0}, true},
+		{"above range", [6]byte{'a', 'b', 'c', 0x7F, 0, 0}, true},
+	}
+
+	for _, test := range tests {
+		c := newRangeCheckableCar(test.code, manufacturerCode)
+		err := c.RangeCheck(c.SbeSchemaVersion(), c.SbeSchemaVersion())
+
+		if test.wantError && err == nil {
+			t.Errorf("%s (%q): expected RangeCheck to fail, got nil", test.name, test.code)
+		}
+		if !test.wantError && err != nil {
+			t.Errorf("%s (%q): expected RangeCheck to pass, got %v", test.name, test.code, err)
+		}
+	}
+}
+
+// A NUL padded char array must survive a round trip with range checking enabled at both ends.
+func TestEncodeDecodeNulPaddedCharArray(t *testing.T) {
+	var manufacturerCode [3]byte
+	copy(manufacturerCode[:], "12")
+
+	in := newRangeCheckableCar(makeVehicleCode("abc"), manufacturerCode)
+
+	m := NewSbeGoMarshaller()
+	buf := new(bytes.Buffer)
+	if err := in.Encode(m, buf, true); err != nil {
+		t.Fatalf("Encode with range check failed: %v", err)
+	}
+
+	var out Car
+	if err := out.Decode(m, buf, in.SbeSchemaVersion(), in.SbeBlockLength(), true); err != nil {
+		t.Fatalf("Decode with range check failed: %v", err)
+	}
+
+	if in.VehicleCode != out.VehicleCode {
+		t.Errorf("VehicleCode round trip: encoded %q, decoded %q", in.VehicleCode, out.VehicleCode)
+	}
+	if in.Engine.ManufacturerCode != out.Engine.ManufacturerCode {
+		t.Errorf("ManufacturerCode round trip: encoded %q, decoded %q",
+			in.Engine.ManufacturerCode, out.Engine.ManufacturerCode)
+	}
+}
+
+// The NullValue escape must apply only to character arrays; numeric arrays keep strict range checking.
+func TestRangeCheckStillStrictForNumericArrays(t *testing.T) {
+	var manufacturerCode [3]byte
+	copy(manufacturerCode[:], "123")
+
+	c := newRangeCheckableCar(makeVehicleCode("abcdef"), manufacturerCode)
+	c.SomeNumbers[2] = c.SomeNumbersMaxValue() + 1
+
+	if err := c.RangeCheck(c.SbeSchemaVersion(), c.SbeSchemaVersion()); err == nil {
+		t.Error("expected RangeCheck to reject an out of range uint32 array element, got nil")
+	}
+}

sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/golang/flyweight/GolangFlyweightGenerator.java

@@ -1855,14 +1855,22 @@ private void generateArrayProperty(
             offset,
             formatClassName(containingClassName));
 
-        new Formatter(sb).format("\n" +
-            indent + "func (m *%1$s) Put%2$s(src []byte) *%1$s {\n" +
-            indent + "    copy(m.buffer[(m.offset+%3$d):], src)\n" +
-            indent + "    return m\n" +
-            indent + "}\n",
-            formatClassName(containingClassName),
-            formatPropertyName(propertyName),
-            offset);
+        if (primitiveType == PrimitiveType.CHAR)
+        {
+            generateNullPaddedSetter(
+                sb, containingClassName, "Put", propertyName, "[]byte", offset, arrayLength, indent);
+        }
+        else
+        {
+            new Formatter(sb).format("\n" +
+                indent + "func (m *%1$s) Put%2$s(src []byte) *%1$s {\n" +
+                indent + "    copy(m.buffer[(m.offset+%3$d):], src)\n" +
+                indent + "    return m\n" +
+                indent + "}\n",
+                formatClassName(containingClassName),
+                formatPropertyName(propertyName),
+                offset);
+        }
 
         if (arrayLength > 1 && arrayLength <= 4)
         {
@@ -1922,17 +1930,56 @@ private void generateArrayProperty(
 
             generateJsonEscapedStringGetter(sb, encodingToken, indent, propertyName, containingClassName);
 
-            new Formatter(sb).format("\n" +
-                indent + "func (m *%1$s) Set%2$s(src string) *%1$s {\n" +
-                indent + "    copy(m.buffer[(m.offset+%3$d):], []byte(src))\n" +
-                indent + "    return m\n" +
-                indent + "}\n",
-                formatClassName(containingClassName),
-                formatPropertyName(propertyName),
-                offset);
+            generateNullPaddedSetter(
+                sb, containingClassName, "Set", propertyName, "string", offset, arrayLength, indent);
         }
     }
 
+    /**
+     * Generate a setter for a fixed length character array which pads any unused trailing bytes with NUL, matching
+     * the Java and C{plus}{plus} codecs. The field is therefore fully defined after the call regardless of what the
+     * buffer previously held, which is what {@code Get&lt;name&gt;AsString} relies on when it trims at the first NUL.
+     *
+     * @param sb                  to append the generated code to.
+     * @param containingClassName of the flyweight the setter belongs to.
+     * @param methodPrefix        of the generated method, either {@code Set} or {@code Put}.
+     * @param propertyName        of the field being set.
+     * @param srcGoType           Go type of the source parameter, either {@code string} or {@code []byte}.
+     * @param offset              of the field within the containing block.
+     * @param arrayLength         declared length of the character array.
+     * @param indent              for the generated code.
+     */
+    private void generateNullPaddedSetter(
+        final StringBuilder sb,
+        final String containingClassName,
+        final String methodPrefix,
+        final String propertyName,
+        final String srcGoType,
+        final int offset,
+        final int arrayLength,
+        final String indent)
+    {
+        new Formatter(sb).format("\n" +
+            indent + "func (m *%1$s) %2$s%3$s(src %4$s) *%1$s {\n" +
+            indent + "    length := uint64(len(src))\n" +
+            indent + "    if length > %6$d {\n" +
+            indent + "        panic(\"src too large for %2$s%3$s [E106]\")\n" +
+            indent + "    }\n\n" +
+            indent + "    start := m.offset + %5$d\n" +
+            indent + "    copy(m.buffer[start:start+%6$d], src)\n" +
+            indent + "    for i := length; i < %6$d; i++ {\n" +
+            indent + "        m.buffer[start+i] = 0\n" +
+            indent + "    }\n\n" +
+            indent + "    return m\n" +
+            indent + "}\n",
+            formatClassName(containingClassName),
+            methodPrefix,
+            formatPropertyName(propertyName),
+            srcGoType,
+            offset,
+            arrayLength);
+    }
+
     private void generateJsonEscapedStringGetter(
         final StringBuilder sb,
         final Token token,

sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/golang/struct/GolangGenerator.java

@@ -477,10 +477,26 @@ private void generateRangeCheckPrimitive(
         imports.peek().add("fmt");
         if (token.arrayLength() > 1)
         {
+            // A character array holding a value shorter than its declared length is NUL padded by the Java,
+            // C++ and Go flyweight codecs, and NullValue for CHAR (0) sits below MinValue (0x20), so the pad
+            // bytes have to be accepted here too. Space padded values are unaffected either way because the
+            // pad character is itself within Min->Max.
+            // Structured this way for go fmt sanity on both cases.
+            final String elementCheck;
+            if (token.encoding().primitiveType() == PrimitiveType.CHAR)
+            {
+                elementCheck = "\t\t\tif %1$s[idx] != %1$sNullValue() && " +
+                    "(%1$s[idx] < %1$sMinValue() || %1$s[idx] > %1$sMaxValue()) {\n";
+            }
+            else
+            {
+                elementCheck = "\t\t\tif %1$s[idx] < %1$sMinValue() || %1$s[idx] > %1$sMaxValue() {\n";
+            }
+
             sb.append(String.format(
                 "\tif %1$sInActingVersion(actingVersion) {\n" +
                 "\t\tfor idx := 0; idx < %2$s; idx++ {\n" +
-                "\t\t\tif %1$s[idx] < %1$sMinValue() || %1$s[idx] > %1$sMaxValue() {\n" +
+                elementCheck +
                 "\t\t\t\treturn fmt.Errorf(\"Range check failed on %1$s[%%d] " +
                 "(%%v < %%v > %%v)\", idx, %1$sMinValue(), %1$s[idx], %1$sMaxValue())\n" +
                 "\t\t\t}\n" +