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 on multibyte char in page ranges #241 #242

Merged
merged 2 commits into from
Nov 20, 2024
Merged

Conversation

wrenger
Copy link
Contributor

@wrenger wrenger commented Oct 22, 2024

This fixes #241, which is a bug caused by incorrectly splitting the page ranges into groups.
The index calculation fails to account for a multibyte character at the end of a pages group. The subsequent split operation on the incorrect index leads to a crash, as described in the issue.

Examples:

@article{foo,
  ...
  pages     = {321--},
  ...
}
@article{bar,
  ...
  pages     = {321–},
  ...
}

Both page ranges are converted to a 321 followed by , which is an utf8 character with multiple bytes

In addition to the fix, I've added a few tests for this and removed the string window in favor of the chars iterator from the std.

@wrenger
Copy link
Contributor Author

wrenger commented Nov 1, 2024

Hi, seems pretty quiet around this repo. The focus is currently probably on the Typst compiler itself?

As this bug crashes the Typst compiler on valid BibTeX, it's quite critical for me, and I would be happy if it is fixed 😅

@PgBiel
Copy link
Contributor

PgBiel commented Nov 6, 2024

Hi @wrenger, thanks for the PR! Don't worry, we'll be taking a look at your PR soon enough. Unfortunately we can't respond to everyone in a timely manner, but we try our best :)

It's worth noting that, even after the bug fix is merged, it still won't be live until the next release, so, if you need this fix urgently, I'd suggest pointing the compiler's dependency on Hayagriva to your fix and recompiling so you can benefit from it locally. (Unfortunately, there is currently no such solution for the web app.)

@PgBiel
Copy link
Contributor

PgBiel commented Nov 20, 2024

I believe we might want to use an external library such as itertools to handle this for us at some point, but this will do. Thank you!

@PgBiel PgBiel merged commit 21f0185 into typst:main Nov 20, 2024
2 checks passed
@PgBiel
Copy link
Contributor

PgBiel commented Nov 20, 2024

By the way @wrenger, for future PRs (in any repository, not just here), I recommend using a branch other than main in your fork. This ensures there won't be interference between your work in one PR and in other PRs (or even in other branches which you may use, say, for local patches). 😉

@wrenger
Copy link
Contributor Author

wrenger commented Nov 21, 2024

Yes good point, I'll keep this in mind, and thanks again for merging :D

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.

[0.12.0] Compiler Crash at BibTeX Items with Open pages Range
2 participants