Skip to content

json: fix escapeIndex() on big-endian (s390x) - #171

Open
dmkif wants to merge 1 commit into
segmentio:masterfrom
dmkif:fix-escapeindex-bigendian
Open

json: fix escapeIndex() on big-endian (s390x)#171
dmkif wants to merge 1 commit into
segmentio:masterfrom
dmkif:fix-escapeindex-bigendian

Conversation

@dmkif

@dmkif dmkif commented Aug 27, 2026

Copy link
Copy Markdown

Fixes #170 (full story there — chasing a Portainer bug on s390x/zCX, containers with OCI labels not listing).

Putting up a PR since I already had the change sitting locally, but genuinely not sure if this is the "right" fix from a project-conventions standpoint — happy to rework it if you'd rather do it differently. I'm still fairly new to Go so go easy on me if I got a convention wrong somewhere.

What I changed

escapeIndex() was casting 8-byte chunks of the string into a uint64 via unsafe.Pointer (the stringToUint64 helper), which reads native-endian. On big-endian (s390x) that's the wrong byte order for how bits.TrailingZeros64(...)/8 figures out the byte index afterwards — it returns the mirrored position (7-k instead of k) within the chunk, so it can point at the wrong byte and skip a character that should've been escaped.

I swapped that for binary.LittleEndian.Uint64 on each 8-byte block (same as parse.go already does elsewhere in this repo), and added the block offset i to the TrailingZeros64(...)/8 result so the index comes out right no matter the host endianness.

Since stringToUint64 isn't used anywhere else after that, I removed it and the now-unused unsafe import from json/string.go. Left sliceHeader alone since it's still used in json/codec.go.

Testing

Added TestEscapeIndexBigEndian to json/string_test.go:

  • runs escapeIndex against a plain byte-by-byte reference scanner for a handful of strings with the special character at different positions (including right around the 8-byte chunk boundary, since that's where things went wrong for me)
  • also round-trips each string through this package's Marshal and stdlib encoding/json's Unmarshal to check the actual JSON output is valid and comes back as the same string, similar to the reproducer in escapeIndex() breaks string escaping on big-endian (s390x) #170

On amd64:

  • go build ./... — fine
  • go vet ./json — some pre-existing warnings in other test files, unrelated to this change, didn't touch those
  • go test ./json -run TestEscapeIndexBigEndian — passes

I don't have a Go toolchain on the s390x box itself (zCX/Tumbleweed, not really meant for dev work), so I built and ran the test on a big-endian setup I have access to elsewhere — it failed against the old code and passes with this change.

Only touched json/string.go and json/string_test.go, didn't touch go.mod/go.sum.

escapeIndex read 8-byte chunks via an unsafe native-endian cast, so
bits.TrailingZeros64(...)/8 returned the mirrored byte index on
big-endian (s390x). Characters needing escaping were skipped and
written raw, producing invalid JSON. Use binary.LittleEndian.Uint64
like parse.go already does. Adds a regression test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

escapeIndex() breaks string escaping on big-endian (s390x)

1 participant