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

Panic when markdown has dollar signs in it #49

Closed
psanford opened this issue Nov 2, 2023 · 3 comments
Closed

Panic when markdown has dollar signs in it #49

psanford opened this issue Nov 2, 2023 · 3 comments

Comments

@psanford
Copy link

psanford commented Nov 2, 2023

The following markdown doc causes mdtopdf to panic:

Bob had $20 and but lost $10.

The panic is:

$ ./md2pdf -i  /tmp/foo.md  -o /tmp/out.md.pdf
panic: Unknown node type *ast.Math

goroutine 1 [running]:
github.com/mandolyte/mdtopdf.(*PdfRenderer).RenderNode(0x4000030800?, {0x0?, 0x40001a1b08?}, {0x45ac90?, 0x4000020aa0?}, 0xc0?)
        /home/psanford/projects/thirdparty/mdtopdf/mdtopdf.go:454 +0x5e4
github.com/gomarkdown/markdown.Render.func1({0x45ac90?, 0x4000020aa0?}, 0x80?)
        /home/psanford/.cache/gopath/pkg/mod/github.com/gomarkdown/markdown@v0.0.0-20230922105210-14b16010c2ee/markdown.go:63 +0x50
github.com/gomarkdown/markdown/ast.NodeVisitorFunc.Visit(0x4000172840?, {0x45ac90?, 0x4000020aa0?}, 0x68?)
        /home/psanford/.cache/gopath/pkg/mod/github.com/gomarkdown/markdown@v0.0.0-20230922105210-14b16010c2ee/ast/node.go:574 +0x38
github.com/gomarkdown/markdown/ast.Walk({0x45ac90, 0x4000020aa0}, {0x457420, 0x400007b3e0})
        /home/psanford/.cache/gopath/pkg/mod/github.com/gomarkdown/markdown@v0.0.0-20230922105210-14b16010c2ee/ast/node.go:546 +0x58
github.com/gomarkdown/markdown/ast.Walk({0x45a430, 0x4000172840}, {0x457420, 0x400007b3e0})
        /home/psanford/.cache/gopath/pkg/mod/github.com/gomarkdown/markdown@v0.0.0-20230922105210-14b16010c2ee/ast/node.go:557 +0x144
github.com/gomarkdown/markdown/ast.Walk({0x45a3e8, 0x40001727e0}, {0x457420, 0x400007b3e0})
        /home/psanford/.cache/gopath/pkg/mod/github.com/gomarkdown/markdown@v0.0.0-20230922105210-14b16010c2ee/ast/node.go:557 +0x144
github.com/gomarkdown/markdown/ast.WalkFunc(...)
        /home/psanford/.cache/gopath/pkg/mod/github.com/gomarkdown/markdown@v0.0.0-20230922105210-14b16010c2ee/ast/node.go:580
github.com/gomarkdown/markdown.Render({0x45a3e8, 0x40001727e0}, {0x458c20, 0x40001d0000})
        /home/psanford/.cache/gopath/pkg/mod/github.com/gomarkdown/markdown@v0.0.0-20230922105210-14b16010c2ee/markdown.go:62 +0xd0
github.com/mandolyte/mdtopdf.(*PdfRenderer).Run(0x40001d0000, {0x400003e200?, 0x242?, 0x40001a1d30?})
        /home/psanford/projects/thirdparty/mdtopdf/mdtopdf.go:344 +0xb4
github.com/mandolyte/mdtopdf.(*PdfRenderer).Process(0x40001d0000, {0x400003e200?, 0x1e?, 0x200?})
        /home/psanford/projects/thirdparty/mdtopdf/mdtopdf.go:318 +0x1b8
main.main()
        /home/psanford/projects/thirdparty/mdtopdf/cmd/md2pdf/md2pdf.go:157 +0x9fc

Seems like the issue is that you have enabled the MathJax extension via parser.CommonExtensions but you don't actually support those AST types.

I'm not trying to use MathJax so it would be nice if there was an option to override the markdown extensions enabled.

jessp01 added a commit to jessp01/mdtopdf that referenced this issue Nov 2, 2023
For now, I've disabled the `MathAJax` extension, though I created a stub in nodeProcessing.go for future consideration.
Also, replaced the `panic()` call with `fmt.Printf()` when node.(type)
does not match any of the supported types. This way, people could report such
issues and at least get some useful output, rather than a crash.
@jessp01
Copy link
Collaborator

jessp01 commented Nov 2, 2023

Hi @psanford ,

Thanks for reporting. Could you please try: #50 (I've tested it with your sample input, of course but, would be good for you to run on your full file).

If all is well, I'll merge and create a new release.

it would be nice if there was an option to override the markdown extensions enabled

I agree it's a nice feature. I'll implement it at a later date (perhaps this weekend but not making any promises :)).

@psanford
Copy link
Author

psanford commented Nov 2, 2023

I can confirm that fixes the crash I was seeing.

@jessp01
Copy link
Collaborator

jessp01 commented Nov 2, 2023

Thanks for confirming. Release created.

@jessp01 jessp01 closed this as completed Nov 2, 2023
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

No branches or pull requests

2 participants