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

Key size in dicts seems to be limited to 128 characters #849

Open
shoelzle opened this issue May 9, 2022 · 5 comments · May be fixed by #1031
Open

Key size in dicts seems to be limited to 128 characters #849

shoelzle opened this issue May 9, 2022 · 5 comments · May be fixed by #1031

Comments

@shoelzle
Copy link

shoelzle commented May 9, 2022

Hello,

Originally, I raised an issue againt yq: Strange formatting when the key size is greater than 128 characters #1192

But, since yq is using go-yaml v3, I was pointed here.

Maybe someone here can explain the key size limit or remove it?

Thanks and best regards

@willbeason
Copy link

willbeason commented May 20, 2022

Does this cause any specific issues for you, or is it just a visual thing?

For example: does this cause a bug in some other library, or something you're working on?

@shoelzle
Copy link
Author

shoelzle commented Jun 2, 2022

As far as I know, it's just a visual thing.
The issue came to my attention, because a colleague of mine noticed a difference in a YAML file, that was modified with yq (yq eval -i ...) by a CI/CD pipeline.

After some testing, I stumbled across this key size limit and created the issue in the yq project.

@cjj884
Copy link

cjj884 commented Aug 15, 2022

Can the 128 char limit in emitterc be replaced with 1024 to align with the spec?
"If the “?” indicator is omitted, parsing needs to see past the implicit key to recognize it as such. To limit the amount of lookahead required, the “:” indicator must appear at most 1024 Unicode characters beyond the start of the key. In addition, the key is restricted to a single line."

@schneefisch
Copy link

Hi,

I actually do have an issue with that.
the yaml.v3 encoder is used in libopenapi and in my case I got an Openapi specification from a client that uses patternProperties where the pattern is longer than 128 characters.

So this is not a visual thing but rather produces an invalid yaml document and therefore is definetly a bug.

here is an example:

openapi: 3.1.0
info:
  title: Example API
  version: 1.0.0
paths:
  /test:
    get:
      responses:
        '200':
          description: Success
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/ExampleModel'
components:
  schemas:
    ExampleModel:
      type: object
      patternProperties:
        '^aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa$':
          type: string
          description: A property key started with 'a' and ending in 'z'

if that openapi spec is parsed and in my case I need to encode it as yaml-file, I am getting this broken output:

openapi: 3.1.0
info:
  title: Example API
  version: 1.0.0
paths:
  /test:
    get:
      responses:
        '200':
          description: Success
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/ExampleModel'
components:
  schemas:
    ExampleModel:
      type: object
      patternProperties:
        ?  '^aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa$'
        :   type: string
          description: A property key started with 'a' and ending in 'z'

the result is an invalid yaml document.

here is a sample go-test:

func Test_renderingWithYaml(t *testing.T) {
	//t.Skipf("skipping test")
	type args struct {
		input *yaml.Node
	}
	tests := []struct {
		name string
		args args
		want string
	}{
		{
			name: "failure rendering patternProperties longer than 128 characters",
			args: args{
				input: &yaml.Node{Kind: yaml.MappingNode, Content: []*yaml.Node{
					{Kind: yaml.ScalarNode, Value: "openapi"},
					{Kind: yaml.ScalarNode, Value: "3.1.0"},
					{Kind: yaml.ScalarNode, Value: "info"},
					{Kind: yaml.MappingNode, Content: []*yaml.Node{
						{Kind: yaml.ScalarNode, Value: "title"},
						{Kind: yaml.ScalarNode, Value: "Example API"},
					}},
					{Kind: yaml.ScalarNode, Value: "path"},
					{Kind: yaml.MappingNode, Content: []*yaml.Node{
						{Kind: yaml.ScalarNode, Value: "'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa$'"},
						{Kind: yaml.MappingNode, Content: []*yaml.Node{
							{Kind: yaml.ScalarNode, Value: "type"},
							{Kind: yaml.ScalarNode, Value: "string"},
						}},
					}},
				}},
			},
			want: `openapi: 3.1.0
info:
  title: Example API
path:
  'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa$':
    type: string`,
		},
	}
	for _, test := range tests {
		t.Run(test.name, func(t *testing.T) {
			// yaml.Node:
			yamlDoc := test.args.input

			// yaml-endoding
			var buf bytes.Buffer
			yamlEncoder := yaml.NewEncoder(&buf)
			yamlEncoder.SetIndent(2)
			_ = yamlEncoder.Encode(yamlDoc)
			renderedBytes := string(buf.Bytes())
			if !cmp.Equal(renderedBytes, test.want) {
				t.Errorf("render() error - diff: %v", cmp.Diff(test.want, renderedBytes))
			}
		})
	}
}

What puzzles me is the fact, that the content itself is fine, it's not cutting the key at 128 characters, but rather places the questionmarc in front, but I guess @cjj884 suggestion could explain it.

@DMaxter
Copy link

DMaxter commented Apr 12, 2024

I found a very similar issue as the one @schneefisch. Also a very long path in OpenAPI specification, generating an invalid spec.

@DMaxter DMaxter linked a pull request Apr 14, 2024 that will close this issue
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 a pull request may close this issue.

5 participants