Skip to content

fix: prevent panic when merging a notetype with more fields#5070

Open
krMaynard wants to merge 1 commit into
ankitects:mainfrom
krMaynard:fix-notetype-merge-panic
Open

fix: prevent panic when merging a notetype with more fields#5070
krMaynard wants to merge 1 commit into
ankitects:mainfrom
krMaynard:fix-notetype-merge-panic

Conversation

@krMaynard

Copy link
Copy Markdown
Contributor

Linked issue (required)

Fixes #4345.

Summary / motivation (required)

Notetype::merge_fields and merge_templates (rslib/src/notetype/merge.rs) used the incoming notetype's index directly as a position into our own fields/templates. When the incoming notetype has more fields than ours — e.g. a corrupt notetype whose field matches one of ours a second time at an index past the end of our list — Vec::swap/Vec::insert received an out-of-bounds index and panicked:

thread '<unnamed>' panicked at rslib/src/notetype/merge.rs:42:33:
index out of bounds: the len is 22 but the index is 22

This made the affected deck impossible to import.

The fix clamps the target index to the valid range so the merge degrades gracefully instead of panicking. All incoming fields/templates are still merged in; a duplicate match simply collapses onto the existing entry rather than crashing.

Steps to reproduce (required, use N/A if not applicable)

  1. Import a deck whose notetype has more fields than the local notetype it maps to, where one incoming field matches an existing field a second time (reported in the wild on corrupt decks — https://forums.ankiweb.net/t/im-unable-to-import-any-decks/66497).
  2. The import panics with index out of bounds in notetype/merge.rs.

How to test (required)

Checklist (minimum)

  • I ran ./ninja check or an equivalent relevant check locally.
  • I added or updated tests when the change is non-trivial or behavior changed.

Details

Added two regression tests — merge_longer_other_with_duplicate_field and merge_longer_other_with_duplicate_template. Verified they panic on the unfixed code and pass with the fix, and that the whole notetype::merge module passes:

cargo test -p anki --lib notetype::merge::
test result: ok. 8 passed; 0 failed

rustfmt --check is clean on the changed file.

Before / after behavior (optional)

Before: importing such a notetype panics and aborts the import. After: the merge completes, mapping the duplicate onto the existing field/template.

Risk / compatibility / migration (optional)

Low — the clamp only changes behavior in the previously-panicking out-of-bounds case; in-range merges are unaffected.

UI evidence (required for visual changes; otherwise N/A)

N/A

Scope

  • This PR is focused on one change (no unrelated edits).

merge_fields and merge_templates used the incoming notetype's index
directly as a position into our own fields/templates. When the incoming
notetype is longer than ours — e.g. a corrupt notetype whose field
matches one of ours a second time at an out-of-bounds index — the
swap/insert panicked with "index out of bounds", making the deck
impossible to import.

Clamp the target index to the valid range so the merge degrades
gracefully instead of panicking, and add regression tests covering a
longer `other` with a duplicate field and a duplicate template.

Fixes ankitects#4345.
@abdnh abdnh requested a review from iamllama July 1, 2026 12:35
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.

Panic when merging notetypes in import

1 participant