Fix out-of-range panic on dangling escape in struct key (#604) - #605
Open
chiliec wants to merge 1 commit into
Open
Fix out-of-range panic on dangling escape in struct key (#604)#605chiliec wants to merge 1 commit into
chiliec wants to merge 1 commit into
Conversation
decodeKeyCharByEscapedChar silently returned no error when an object key
ended with a backslash immediately followed by the buffer's nul terminator
(e.g. `{"\`). The caller then advanced the cursor past the end of the
buffer and read reused (pooled) memory, so a later Unmarshal in the same
sequence panicked with 'index out of range' in skipWhiteSpace.
Mirror the string-value decoder, which already treats nul after a
backslash as ErrUnexpectedEndOfJSON, so a dangling escape in a key now
returns a clean error instead of over-reading the buffer.
Fixes goccy#604
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes #604 —
Unmarshalpanics withindex out of rangewhen a struct is decoded from a sequence of malformed inputs, one of which is an object key ending in a dangling escape (e.g.{"\).Root cause
decodeKeyCharByEscapedChar(internal/decoder/struct.go) handled the known escape characters and\u, but for any other byte — including thenulterminator thatunmarshalappends to the source buffer — it fell through toreturn nil, cursor, nil: no error, no consumed characters. When a key ended with\immediately followed by thatnul(input{"\), the key scanner therefore kept advancing the cursor past the end of the buffer instead of stopping.Because the runtime context / buffer is taken from a
sync.Pool, the bytes past the end are leftovers from a previous decode. That is why the crash only appears when more than one shape of malformed input is decoded in sequence, and why it is attributed to whichever input is current rather than to the sequence — matching the report exactly. The over-read eventually lands inskipWhiteSpaceatcontext.go:49:The value-string decoder (
stringDecoder.decodeByte) already treatsnulafter a backslash asErrUnexpectedEndOfJSON; the key decoder did not. This mirrors that behavior.Fix
Add a
case nultodecodeKeyCharByEscapedCharthat returnsErrUnexpectedEndOfJSON("escaped string", cursor), so a dangling escape in an object key now returns a clean error instead of over-reading the buffer. +2 lines.Tests
Added
TestIssue604indecode_test.go(same style as the existingTestIssue429, which covers the sibling\utruncation cases): decodes the reporter's exact two inputs in a 20-iteration alternating loop and asserts every call returns an error (no panic).Validation (real results, go1.27.0 linux/amd64)
RED→GREEN — revert only the source fix (keep the test), the test panics with the exact reported stack:
Restore the fix → passes:
Full suite green,
gofmtclean:First-time contributor here — happy to adjust naming, test placement, or the error label if you'd prefer a different one.