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 crash when seeing 256 '['s #36

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

Conversation

dimbleby
Copy link

@dimbleby dimbleby commented Sep 6, 2021

Addresses, though perhaps doesn't exactly fix, the crash reported in #32.

If a MinimizedInlineDelimiterList contains 256 or more elements, the char-sized size wraps in serialization. This causes the subsequent failure at deserialization.

I've added an assertion to say that this list shouldn't be too long. This improves things because the new assertion now happens during scanning, where the try-except can kick in - and not in deserialization, where it couldn't.

Probably there's some more graceful way of handling this, but that's beyond me for now...

I added similar assertions in the BlockContextStack and BlockDelimiterList: these look as though they have the same error.

@dimbleby
Copy link
Author

dimbleby commented Sep 6, 2021

I tried adding a testcase but apparently TREE_SITTER_MARKDOWN_AVOID_CRASH is not defined in the CI build, so I backed it out.

@dimbleby dimbleby mentioned this pull request Oct 15, 2021
@NullVoxPopuli
Copy link

@dimbleby do you have a branch that I could point my neovim config at in the mean time while your PRs are reviewed?

@dimbleby
Copy link
Author

@dimbleby do you have a branch that I could point my neovim config at in the mean time while your PRs are reviewed?

No. And in case anyone thinks that I know what I'm doing, I want to make clear that this is not so, and I have no intention of becoming an accidental maintainer of this parser.

I cannot emphasise strongly enough how much I don't understand the code in this repository.

If you're feeling lucky, by all means take a fork and apply my fixes and see how that goes: but be clear that you and not I are responsible for whatever happens next!

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.

None yet

2 participants