Crash after id clash resolution followed by id deletion in pattern chain

Summary:

Under ludicrously specific conditions, the F&S dialog can be made to perform a strcmp on a null string, which crashes.

Steps to reproduce:

  1. Open nested-chained-pattern.svg.

  2. Ensure that rect1 (the only object) is selected.

  3. In the XML editor, rename patternA to patternB, causing an automatic id clash resolution.

  4. Delete the id attribute of the element you just renamed (i.e. the one now called patternB).

  5. Switch to the F&S dialog.

What happened?

Crash in the F&S dialog, with backtrace

5  Inkscape::Application::crash_handler(int)                                                     inkscape.cpp                       515  0x7f9ee0a9828a
6  ___lldb_unnamed_symbol3219                                                                    (x86_64) /usr/lib/libc.so.6             0x7f9edcf9f8e0
7  ___lldb_unnamed_symbol4340                                                                    (x86_64) /usr/lib/libc.so.6             0x7f9edd0c0f0b
8  Inkscape::UI::Widget::PaintSelector::updatePatternList(SPPattern *)                           paint-selector.cpp                 1183 0x7f9ee099d4bf
9  Inkscape::UI::Widget::FillNStroke::performUpdate()                                            fill-style.cpp                     234  0x7f9ee094a276
10 Inkscape::UI::Widget::FillNStroke::selectionModifiedCB(unsigned int)                          fill-style.cpp                     99   0x7f9ee094a2cc
11 Inkscape::UI::Dialog::FillAndStroke::selectionModified(Inkscape::Selection *, unsigned int)   fill-and-stroke.cpp                101  0x7f9ee07b0d38
12 Inkscape::UI::Dialog::DialogBase::selectionModified_impl(Inkscape::Selection *, unsigned int) dialog-base.cpp                    261  0x7f9ee07679e2
13 sigc::...                                                                                     mem_fun.h                          2143 0x7f9ee0769005
14 sigc::...                                                                                     adaptor_trait.h                    108  0x7f9ee0769017
15 sigc::...                                                                                     slot.h                             451  0x7f9ee076900e
16 sigc::...                                                                                     signal.h                           1296 0x7f9ee0acfce9
17 sigc::...                                                                                     signal.h                           3104 0x7f9ee0acdf99
18 Inkscape::Selection::_emitModified(unsigned int)                                              selection.cpp                      105  0x7f9ee0acdf87
19 Inkscape::Selection::_emit_modified(Inkscape::Selection *)                                    selection.cpp                      96   0x7f9ee0ace019
20 g_main_context_dispatch                                                                       (x86_64) /usr/lib/libglib-2.0.so.0      0x7f9edef3ac6b
21 ___lldb_unnamed_symbol2507                                                                    (x86_64) /usr/lib/libglib-2.0.so.0      0x7f9edef91001
22 g_main_context_iteration                                                                      (x86_64) /usr/lib/libglib-2.0.so.0      0x7f9edef38392
23 g_application_run                                                                             (x86_64) /usr/lib/libgio-2.0.so.0       0x7f9eded4330e
24 main                                                                                          inkscape-main.cpp                  259  0x55fa87868cc3

In stack frame 8, the call to strcmp

while (valid && strcmp(patid, patname) != 0) {

is crashing because patid is null.

Steps to not reproduce:

Strangely, every one of the steps above seems to be necessary:

  1. If you use the simpler file nested-pattern.svg with a shorter pattern href chain, the issue goes away.

  2. The object must be selected, or the F&S dialog won't run the buggy code.

  3. Renaming patternA to patternC (which still causes a clash, but with a different element) doesn't crash.

  4. Not deleting the id attribute, or deleting the whole element instead of just its id attribute, there is no crash.

  5. Obviously this step is necessary for the same reason as 2.

Version info

Reproduced on Inkscape 1.2.1 (9c6d41e410, 2022-07-14) as well as a host of recent builds of 1.3 (e.g. bc63f1e, 9e7ec23).