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

String method call panics on empty document node #629

Closed
bcho opened this issue Jan 28, 2025 · 0 comments · Fixed by #630
Closed

String method call panics on empty document node #629

bcho opened this issue Jan 28, 2025 · 0 comments · Fixed by #630
Labels
bug Something isn't working

Comments

@bcho
Copy link
Contributor

bcho commented Jan 28, 2025

Describe the bug

The ast.DocumentNode panics on empty document, example program:

https://go.dev/play/p/YmrzDKhtFNp

package main

import (
	"fmt"

	"github.com/goccy/go-yaml/lexer"
	"github.com/goccy/go-yaml/parser"
)

func main() {
	f, err := parser.Parse(lexer.Tokenize(""), 0)
	if err != nil {
		panic(err)
	}
	fmt.Println(f.String())
}

The above program fails with panic:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x58 pc=0x4ada54]

goroutine 1 [running]:
github.com/goccy/go-yaml/ast.(*DocumentNode).String(0xc0000162a0)
	/tmp/gopath1302303734/pkg/mod/github.com/goccy/go-yaml@v1.15.5/ast/ast.go:578 +0xb4
github.com/goccy/go-yaml/ast.(*File).String(0x0?)
	/tmp/gopath1302303734/pkg/mod/github.com/goccy/go-yaml@v1.15.5/ast/ast.go:535 +0x78
main.main()
	/tmp/sandbox432899830/prog.go:15 +0x34

This seems to be an regression introduced in v1.15.0 as the above same program works in v1.14.3 :

https://go.dev/play/p/VubIuMgDfYQ

I believe this is because now the body of the document node can be nil due to the new implementation in here:

go-yaml/parser/parser.go

Lines 107 to 109 in 3399084

if len(docGroup.Tokens) == 0 {
return ast.Document(docGroup.RawToken(), nil), nil
}

while the DocumentNode.String method skips the nil check on body:

doc = append(doc, d.Body.String())

To Reproduce

Please provide a minimum yaml content that can be reproduced.
We are more than happy to use Go Playground

Expected behavior

.String() should not panic for an empty document node

Screenshots
If applicable, add screenshots to help explain your problem.

Version Variables

  • Go version: [e.g. 1.21 ]
  • go-yaml's Version: [e.g. v1.11.1 ]

Additional context
Add any other context about the problem here.

@bcho bcho added the bug Something isn't working label Jan 28, 2025
bcho added a commit to bcho/go-yaml that referenced this issue Jan 28, 2025
@goccy goccy closed this as completed in 6f80c57 Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant