Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix encoding for explicitly marked str nodes #1052

Open
wants to merge 1 commit into
base: v3
Choose a base branch
from
Open

Conversation

torkve
Copy link

@torkve torkve commented Oct 3, 2024

When yaml.v3 encodes string nodes, that are explicitly marked with tag "!!str", it can remove this tag. But with ambiguous values like "y", "yes", "no" etc. it can lead to the result YAML document to be misinterpreted by YAML 1.1 compatible parsers.
This patch enforces old-boolean-like string nodes to be forcedly quoted.

@torkve
Copy link
Author

torkve commented Oct 3, 2024

Real world example: Google Gnostic compiler for OpenAPI encodes any single OpenAPI property name as YAML string, and if one have some Point-like model Point(x int, y int), after compilation it would become:

&yaml.Node{
	Kind: yamlv3.MappingNode,
	Content: []*yamlv3.Node{
		{Kind:  yamlv3.ScalarNode, Tag: "!!str", Value: "x"},
		{Kind:  yamlv3.ScalarNode, Tag: "!!int", Value: "2"},
		{Kind:  yamlv3.ScalarNode, Tag: "!!str", Value: "y"},
		{Kind:  yamlv3.ScalarNode, Tag: "!!int", Value: "3"},
	},
}

And after serialization it becomes

point:
  x: 2
  y: 3

Which would be deserialized incorrectly by YAML 1.1 compatible parser (e.g. one in kubernetes), since y would become true.

To the best of my knowledge I believe that Gnostic is correct here, since it explicitly marks node as string one and expects that it would always be a string in YAML, not boolean. That's why I propose this PR as a fix.

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