Skip to content

fix(wire): handle null/empty in Content and DisplayBlockData UnmarshalJSON#204

Open
Ricardo-M-L wants to merge 1 commit into
MoonshotAI:mainfrom
Ricardo-M-L:fix/wire-content-unmarshal-null
Open

fix(wire): handle null/empty in Content and DisplayBlockData UnmarshalJSON#204
Ricardo-M-L wants to merge 1 commit into
MoonshotAI:mainfrom
Ricardo-M-L:fix/wire-content-unmarshal-null

Conversation

@Ricardo-M-L

Copy link
Copy Markdown

Problem

Content.UnmarshalJSON and DisplayBlockData.UnmarshalJSON switch on data[0] immediately after bytes.TrimSpace, with no handling for the JSON null literal and no guard for empty input:

func (c *Content) UnmarshalJSON(data []byte) error {
	data = bytes.TrimSpace(data)
	switch data[0] {       // panics if data is empty
	case '"':  ...
	case '[':  ...
	default:   return fmt.Errorf("invalid content type, ...")   // hit by "null"
	}
}

Two real consequences:

  1. A null content field fails to parse. Content is used as a bare (non-Optional) field in several wire types (ToolResultReturnValue.Output, PromptParams.UserInput, TurnBegin.UserInput). When the peer sends null for one of them, encoding/json calls Content.UnmarshalJSON([]byte("null")), which falls into default and returns invalid content type — failing the entire message decode. By convention, a json.Unmarshaler should treat null as a no-op.
  2. Empty input panics with runtime error: index out of range [0] with length 0.

Reproduced against main before the fix:

TestContentNullRepro:  unmarshal {output:null} err = invalid content type, expected one of "text" or "content_parts"
TestContentEmptyPanicRepro: panicked: runtime error: index out of range [0] with length 0

Fix

Return early on empty input or the null literal in both unmarshalers:

data = bytes.TrimSpace(data)
if len(data) == 0 || string(data) == "null" {
	return nil
}

DisplayBlockData is currently only reached through Optional[DisplayBlockData] (which already short-circuits null), but the same guard keeps it correct and consistent if it is ever used bare.

Tests

Added to wire/message_test.go:

  • TestContent_UnmarshalJSON_Nullnull standalone and as a struct field is a no-op.
  • TestContent_UnmarshalJSON_EmptyDoesNotPanic — whitespace input no longer panics.
  • TestDisplayBlockData_UnmarshalJSON_Null.

Existing behavior for strings, arrays/objects, and invalid tokens (TestContent_UnmarshalJSON_InvalidToken) is unchanged.

Verification

$ go test ./...
ok  github.com/MoonshotAI/kimi-agent-sdk/go
ok  github.com/MoonshotAI/kimi-agent-sdk/go/wire
ok  github.com/MoonshotAI/kimi-agent-sdk/go/wire/jsonrpc2
ok  github.com/MoonshotAI/kimi-agent-sdk/go/wire/transport
ok  github.com/MoonshotAI/kimi-agent-sdk/go/test/e2e
ok  github.com/MoonshotAI/kimi-agent-sdk/go/test/integration

$ go vet ./...   # clean
$ gofmt -l go/wire/message.go go/wire/message_test.go   # clean

…lJSON

Content.UnmarshalJSON and DisplayBlockData.UnmarshalJSON switched on
data[0] right after bytes.TrimSpace without handling the JSON null literal
or guarding against empty input. Two consequences:

  - A null content field in an otherwise valid message failed to parse:
    json.Unmarshal([]byte(`{"output": null}`), &v) returned
    "invalid content type" instead of leaving the field zero-valued. By
    convention a json.Unmarshaler must treat null as a no-op.
  - Empty/whitespace input panicked with index out of range [0] with
    length 0.

Return early on empty input or the null literal in both unmarshalers, and
add tests covering null (standalone and as a struct field) and the
empty-input case. Existing behavior for valid strings, arrays/objects, and
invalid tokens is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Ricardo-M-L

Copy link
Copy Markdown
Author

Hi! Just a friendly ping on this PR. It's been open for a while — would appreciate a review when you get a chance. Thanks for your time!

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.

1 participant