Skip to content

Fix crash when parsing karaoke data from a line without an extra field - #676

Open
line0 wants to merge 1 commit into
masterfrom
parse-karaoke-data-no-extradata-crash-fix
Open

Fix crash when parsing karaoke data from a line without an extra field#676
line0 wants to merge 1 commit into
masterfrom
parse-karaoke-data-no-extradata-crash-fix

Conversation

@line0

@line0 line0 commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Passing a line table without an extra field to aegisub.parse_karaoke_data(line), e.g. via the public karaskel.preproc_line_text(meta, styles, line) API, immediately crashes Aegisub (at least on Windows).

The crash is caused by a bug in the LuaAssFile::LuaToAssEntry() function, whose dialog branch leaves a nil on the stack when the extra field is absent or nil. With the now unbalanced stack, LuaAssFile::LuaParseKaraokeData() ends up trying to write syllables into a nil instead of the line table expected at that stack index.
Fixed by popping the value in that case, so the stack remains balanced and the line table at its the expected index.

None of the other LuaAssFile::LuaToAssEntry() call sites are affected either way.

Reproduction

Any automation script, with a subtitle file open:

aegisub.parse_karaoke_data{
  class = "dialogue", comment = false, layer = 0,
  start_time = 0, end_time = 5000, style = "Default", actor = "",
  margin_l = 0, margin_r = 0, margin_t = 0, effect = "", text = "Hello world",
}

The bug is not triggered when the lines passed to karaskel.preproc_line_text(meta, styles, line) are directly taken from the subtitles object, as that is always initialized with an empty extradata table.

However it is easy to hit when a script builds its own line tables, particularly when relying on Aegisub's own documentation:

  • automation/v4-docs/subtitle-data.txt gives the signature as aegisub.parse_karaoke_data(text) with @text (string), but the function takes a dialogue line table and errors on a string.
  • Neither that file nor the karaskel page on aegisub.org lists extra among a dialogue line's fields, so the documented set of fields is exactly the set that crashes.

Side note

The comment above LuaAssFile::LuaToAssEntry() claims to pop the table from the stack, but not only does it not, LuaAssFile::LuaParseKaraokeData() also depends on it not doing so.

@CoffeeFlux CoffeeFlux left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests are pretty specific to libaegisub, so this is a layering violation, no? I'll try to get aegisub cli in soon, which will make this a lot easier.

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.

2 participants