Skip to content

Tolerate json/v2 invalid UTF-8 munging in YAMLToJSON tests - #141

Open
arthurbdiniz wants to merge 1 commit into
kubernetes-sigs:masterfrom
arthurbdiniz:tolerate-json-v2-invalid-utf8-in-yaml-tests
Open

arthurbdiniz wants to merge 1 commit into
kubernetes-sigs:masterfrom
arthurbdiniz:tolerate-json-v2-invalid-utf8-in-yaml-tests

Conversation

@arthurbdiniz

Copy link
Copy Markdown

What this PR does / why we need it

TestYAMLToJSON's binary data case decodes an invalid UTF-8 payload via a !!binary tag and asserts that the resulting JSON escapes the Unicode replacement character as \ufffd:

"binary data": {
    yaml: "a: !!binary gIGC",
    json: `{"a":"\ufffd\ufffd\ufffd"}`,
    ...
}

Go 1.27 reimplements encoding/json on top of encoding/json/v2. When marshaling a string containing invalid UTF-8 (which decoding !!binary gIGC produces), the standard library now emits the replacement character U+FFFD as raw UTF-8 bytes instead of the \ufffd escape sequence. As a result the test fails on Go 1.27 while passing on Go 1.26 and earlier.

This PR adds a jsonAllow field to the test case struct that lists accepted alternate JSON representations, and tolerates the raw-UTF-8 form for the binary data case. The canonical \ufffd expectation is preserved for every other case, so the byte-exact assertion is unchanged elsewhere. This is a test-only change; there is no change to library behavior.


How to reproduce

Using a Go 1.27 toolchain against the current master:

git clone https://github.com/kubernetes-sigs/yaml.git
cd yaml
go1.27 test -run TestYAMLToJSON ./

Output before this change:

--- FAIL: TestYAMLToJSON (0.00s)
    --- FAIL: TestYAMLToJSON/YAMLToJSON (0.00s)
        --- FAIL: TestYAMLToJSON/YAMLToJSON/binary_data_YAMLToJSON (0.00s)
            yaml_test.go:166: Failed to convert YAML to JSON, yaml: `a: !!binary gIGC`, expected json `{"a":"\ufffd\ufffd\ufffd"}`, got `{"a":"<U+FFFD><U+FFFD><U+FFFD>"}`
    --- FAIL: TestYAMLToJSON/YAMLToJSONStrict (0.00s)
        --- FAIL: TestYAMLToJSON/YAMLToJSONStrict/binary_data_YAMLToJSON (0.00s)
            yaml_test.go:166: ...
FAIL
FAIL	sigs.k8s.io/yaml	0.006s

The same command passes on Go 1.24 through Go 1.26, because their encoding/json still emits the \ufffd escape.


Prior work

Kubernetes made the equivalent change in k8s.io/apimachinery when adapting to json/v2, using the same "tolerate alternate representation" approach:

That PR added an allow []string field to the affected test-case struct and accepted the raw-UTF-8 form for the invalid-UTF-8 cases. This PR mirrors that approach for sigs.k8s.io/yaml.

Signed-off-by: Arthur Diniz <arthurbd@debian.org>
@kubernetes-prow kubernetes-prow Bot added sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 9, 2026
@kubernetes-prow
kubernetes-prow Bot requested review from liggitt and thockin September 9, 2026 22:30
@kubernetes-prow kubernetes-prow Bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 9, 2026
@liggitt

liggitt commented Sep 10, 2026

Copy link
Copy Markdown

/lgtm
/approve

@kubernetes-prow kubernetes-prow Bot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 10, 2026
@kubernetes-prow

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: arthurbdiniz, liggitt

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubernetes-prow kubernetes-prow Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants