bug: UnmarshalUnsafe panics when map values are empty strings

[maheeshap-canopus]

bug: UnmarshalUnsafe panics when map values are empty strings

[nockty]

Interesting edge case, thank you for reporting this bug very clearly! For now I have a couple quick thoughts:

• when intStringLenmapvalue is 0, we could use nil instead of &dAtA[iNdEx]: unsafe.String(nil, 0) should do the job AFAIK.
• in this case, the value is not backed by the original data, which makes sense. I think it's OK to not perform this check for empty strings in the test cases because it doesn't make sense for them anyway. Maybe it would work to compare it against the nil pointer, though. Here is what I have in mind:

func assertStringIsOriginal(t *testing.T, s string, belongs bool, originalData []byte) {
+	start := uintptr(unsafe.Pointer(unsafe.StringData(s)))
+	// empty string has no underlying array, compare pointer to nil
+	if len(s) == 0 {
+		assert.Equal(t, uintptr(unsafe.Pointer(nil)), start)
+		return
+	}
+	end := start + uintptr(len(s)) - 1
+
	originalStart := uintptr(unsafe.Pointer(unsafe.SliceData(originalData)))
	originalEnd := originalStart + uintptr(len(originalData)) - 1

-	start := uintptr(unsafe.Pointer(unsafe.StringData(s)))
-	end := start + uintptr(len(s)) - 1
	assert.Equal(t, belongs, start >= originalStart && start < originalEnd)
	assert.Equal(t, belongs, end > originalStart && end <= originalEnd)
}

What do you think about this?

[maheeshap-canopus]

@nockty Thanks, that fix looks good to me.

I also noticed this panic occurs with empty strings in oneof fields:

--- FAIL: Test_UnmarshalVTUnsafe (0.00s)
panic: runtime error: index out of range [2] with length 2 [recovered]
        panic: runtime error: index out of range [2] with length 2

goroutine 18 [running]:
testing.tRunner.func1.2({0x139fea0, 0xc0000b0318})
        /Users/maheesha/go/go1.20.1/src/testing/testing.go:1526 +0x24e
testing.tRunner.func1()
        /Users/maheesha/go/go1.20.1/src/testing/testing.go:1529 +0x39f
panic({0x139fea0, 0xc0000b0318})
        /Users/maheesha/go/go1.20.1/src/runtime/panic.go:884 +0x213
github.com/planetscale/vtprotobuf/testproto/unsafe.(*UnsafeTest_Sub4).UnmarshalVTUnsafe(0xc0000a2f00, {0xc0000a4842, 0x2, 0x2})
        /Users/maheesha/go/src/vtprotobuf/testproto/unsafe/unsafe_vtproto.pb.go:3015 +0x835
github.com/planetscale/vtprotobuf/testproto/unsafe.(*UnsafeTest).UnmarshalVTUnsafe(0xc0000dbca8, {0xc0000a4840, 0x4, 0x4})
        /Users/maheesha/go/src/vtprotobuf/testproto/unsafe/unsafe_vtproto.pb.go:3444 +0xa34
github.com/planetscale/vtprotobuf/testproto/unsafe.Test_UnmarshalVTUnsafe(0xc000082ea0?)
        /Users/maheesha/go/src/vtprotobuf/testproto/unsafe/unsafe_test.go:153 +0x1811
testing.tRunner(0xc000082ea0, 0x13f15f8)
        /Users/maheesha/go/go1.20.1/src/testing/testing.go:1576 +0x10b
created by testing.(*T).Run
        /Users/maheesha/go/go1.20.1/src/testing/testing.go:1629 +0x3ea

I've used the same approach to fix this, will raise a PR now.