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

renderer: fix panic on unregistered node kind #299

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

Conversation

emersion
Copy link

@emersion emersion commented Apr 4, 2022

Renderers can ignore nodes they aren't interested in. However when a
renderer doesn't register the highest node kind traversing a Markdown
document results in the following panic:

panic: runtime error: index out of range [1] with length 1

goroutine 1 [running]:
github.com/yuin/goldmark/renderer.(*renderer).Render.func2({0x9d8630, 0xc0001237a0}, 0x40?)
	/home/simon/go/pkg/mod/github.com/yuin/[email protected]/renderer/renderer.go:164 +0xd9
github.com/yuin/goldmark/ast.walkHelper({0x9d8630, 0xc0001237a0}, 0xc000521a48)
	/home/simon/go/pkg/mod/github.com/yuin/[email protected]/ast/ast.go:492 +0x34
github.com/yuin/goldmark/ast.Walk(...)
	/home/simon/go/pkg/mod/github.com/yuin/[email protected]/ast/ast.go:487
github.com/yuin/goldmark/renderer.(*renderer).Render(0xc0002d80a0?, {0x9d3b60?, 0xc0000744a0?}, {0xc0002f4580?, 0x281?, 0x2c0?}, {0x9d8630?, 0xc0001237a0?})
	/home/simon/go/pkg/mod/github.com/yuin/[email protected]/renderer/renderer.go:161 +0x225
github.com/yuin/goldmark.(*markdown).Convert(0xc0001e1d40, {0xc0002f4580, 0x281, 0x2c0}, {0x9d3b60, 0xc0000744a0}, {0x0, 0x0, 0x0})
	/home/simon/go/pkg/mod/github.com/yuin/[email protected]/markdown.go:117 +0x10b
main.renderMarkdown({0xc0002f42c0, 0x281})
	/home/simon/src/hut/markdown.go:16 +0x97
main.newListsListCommand.func1(0xc000270280?, {0xc778b8, 0x0, 0x0?})
	/home/simon/src/hut/lists.go:109 +0x2ee
github.com/spf13/cobra.(*Command).execute(0xc000270280, {0xc778b8, 0x0, 0x0})
	/home/simon/go/pkg/mod/github.com/spf13/[email protected]/command.go:860 +0x663
github.com/spf13/cobra.(*Command).ExecuteC(0xc000242c80)
	/home/simon/go/pkg/mod/github.com/spf13/[email protected]/command.go:974 +0x3b4
github.com/spf13/cobra.(*Command).Execute(...)
	/home/simon/go/pkg/mod/github.com/spf13/[email protected]/command.go:902
github.com/spf13/cobra.(*Command).ExecuteContext(...)
	/home/simon/go/pkg/mod/github.com/spf13/[email protected]/command.go:895
main.main()
	/home/simon/src/hut/main.go:49 +0x30b

Make sure r.nodeRendererFuncs is large enough before trying to access it.

Workaround without this patch:

reg.Register(ast.NodeKind(128), nil)

Renderers can ignore nodes they aren't interested in. However when a
renderer doesn't register the highest node kind traversing a Markdown
document results in the following panic:

    panic: runtime error: index out of range [1] with length 1

    goroutine 1 [running]:
    github.com/yuin/goldmark/renderer.(*renderer).Render.func2({0x9d8630, 0xc0001237a0}, 0x40?)
    	/home/simon/go/pkg/mod/github.com/yuin/[email protected]/renderer/renderer.go:164 +0xd9
    github.com/yuin/goldmark/ast.walkHelper({0x9d8630, 0xc0001237a0}, 0xc000521a48)
    	/home/simon/go/pkg/mod/github.com/yuin/[email protected]/ast/ast.go:492 +0x34
    github.com/yuin/goldmark/ast.Walk(...)
    	/home/simon/go/pkg/mod/github.com/yuin/[email protected]/ast/ast.go:487
    github.com/yuin/goldmark/renderer.(*renderer).Render(0xc0002d80a0?, {0x9d3b60?, 0xc0000744a0?}, {0xc0002f4580?, 0x281?, 0x2c0?}, {0x9d8630?, 0xc0001237a0?})
    	/home/simon/go/pkg/mod/github.com/yuin/[email protected]/renderer/renderer.go:161 +0x225
    github.com/yuin/goldmark.(*markdown).Convert(0xc0001e1d40, {0xc0002f4580, 0x281, 0x2c0}, {0x9d3b60, 0xc0000744a0}, {0x0, 0x0, 0x0})
    	/home/simon/go/pkg/mod/github.com/yuin/[email protected]/markdown.go:117 +0x10b
    main.renderMarkdown({0xc0002f42c0, 0x281})
    	/home/simon/src/hut/markdown.go:16 +0x97
    main.newListsListCommand.func1(0xc000270280?, {0xc778b8, 0x0, 0x0?})
    	/home/simon/src/hut/lists.go:109 +0x2ee
    github.com/spf13/cobra.(*Command).execute(0xc000270280, {0xc778b8, 0x0, 0x0})
    	/home/simon/go/pkg/mod/github.com/spf13/[email protected]/command.go:860 +0x663
    github.com/spf13/cobra.(*Command).ExecuteC(0xc000242c80)
    	/home/simon/go/pkg/mod/github.com/spf13/[email protected]/command.go:974 +0x3b4
    github.com/spf13/cobra.(*Command).Execute(...)
    	/home/simon/go/pkg/mod/github.com/spf13/[email protected]/command.go:902
    github.com/spf13/cobra.(*Command).ExecuteContext(...)
    	/home/simon/go/pkg/mod/github.com/spf13/[email protected]/command.go:895
    main.main()
    	/home/simon/src/hut/main.go:49 +0x30b

Make sure r.nodeRendererFuncs is large enough before trying to access it.
@stale
Copy link

stale bot commented Jun 12, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 12, 2022
@emersion
Copy link
Author

Ping

@stale stale bot removed the stale label Jun 12, 2022
@stale
Copy link

stale bot commented Jul 31, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 31, 2022
@emersion
Copy link
Author

Ping

@stale stale bot removed the stale label Jul 31, 2022
@emersion
Copy link
Author

@yuin any chance to get this reviewed?

@MarceloLimoriGM2dev
Copy link

I was about to write a new bug report for this. So ping! from me too.

@abhinav
Copy link
Contributor

abhinav commented Nov 11, 2022

FWIW, this has come up before.

#107 made a similar change to this PR, removing the panic, but the behavior change was considered undesirable.

Following that, #110 and #166 made a variant of the change that kept the panic but improved the error message.

@yuin, would you be open to merging the message change?

@stale
Copy link

stale bot commented Jan 7, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 7, 2023
@emersion
Copy link
Author

emersion commented Jan 7, 2023

Ping

@stale stale bot removed the stale label Jan 7, 2023
@stale
Copy link

stale bot commented May 21, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label May 21, 2023
@emersion
Copy link
Author

.

@stale stale bot removed the stale label May 21, 2023
Copy link

This PR is stale because it has been open for 180 days with no activity.

@github-actions github-actions bot added the stale label Nov 18, 2023
@emersion
Copy link
Author

.

@github-actions github-actions bot removed the stale label Nov 19, 2023
Copy link

github-actions bot commented Jun 2, 2024

This PR is stale because it has been open for 180 days with no activity.

@github-actions github-actions bot added the stale label Jun 2, 2024
@emersion
Copy link
Author

emersion commented Jun 2, 2024

.

@github-actions github-actions bot removed the stale label Jun 3, 2024
@yuin yuin force-pushed the master branch 5 times, most recently from e7e4266 to a590622 Compare June 14, 2024 13:05
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.

4 participants