1961659 - Crash in the NNTP subscription dialog when the group tree is collapsed immediately before a non-existent group

Status: RESOLVED FIXED

(MailNews Core :: Networking: NNTP, defect)

Description

[Alfred Peters [:infofrommozilla]]

Attachment: Screenshot 2025-04-21 202814.png

STR:

1. With a newsgroups account set up in Thunderbird, click the link news:group.does.not.exist.
2. Confirm the dialog for subscribing the newsgroup
3. Ignore the warning and open the subscription dialog of that news server
4. Scroll to the place where this group should appear if it existed
5. Close the subtree just before this place
   => TB crashes

bp-e50c1bd1-3610-4862-8c3d-3a81b0250421

Comment 1

[Wayne Mery (:wsmwk)]

Crash report: https://crash-stats.mozilla.org/report/index/e50c1bd1-3610-4862-8c3d-3a81b0250421

MOZ_CRASH Reason:

ElementAt(aIndex = 393, aLength = 596)

Top 10 frames:

0  mozglue.dll  MOZ_Crash(char const*, int, char const*)  mfbt/Assertions.h:381
0  mozglue.dll  mozilla::detail::InvalidArrayIndex_CRASH(unsigned long long, unsigned long long)  mfbt/Assertions.cpp:50
1  xul.dll  nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_RelocateUsingMemutils>::L...  mailnews/base/src/nsSubscribableServer.cpp:730
1  xul.dll  nsTArray_Impl<SubscribeTreeNode*, nsTArrayInfallibleAllocator>::RemoveElement...  xpcom/ds/nsTArray.h:2524
1  xul.dll  nsSubscribableServer::ToggleOpenState(int)  mailnews/base/src/nsSubscribableServer.cpp:711
2  xul.dll  XPTC__InvokebyIndex()  /builds/worker/workspace/obj-build/toolkit/library/build/Z:/builds/worker/checkouts/gecko/xpcom/reflect/xptcall/md/win32/xptcinvoke_asm_x86_64.asm:97
3  xul.dll  CallMethodHelper::Invoke()  js/xpconnect/src/XPCWrappedNative.cpp:1620
3  xul.dll  CallMethodHelper::Call()  js/xpconnect/src/XPCWrappedNative.cpp:1174
3  xul.dll  XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode)  js/xpconnect/src/XPCWrappedNative.cpp:1120
4  xul.dll  XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*)  js/xpconnect/src/XPCWrappedNativeJSOps.cpp:966

Comment 2

[Wayne Mery (:wsmwk)]

max, can you reproduce this?

Comment 3

[Hartmut Welpmann [:welpy-cw]]

Confirmed. That needs some investigation…

Comment 4

[Hartmut Welpmann [:welpy-cw]]

Attachment: Bug 1961659 - Fix possible crash in NNTP subscription dialog. r=infofrommozilla,mkmelin

Calling setAsSubscribed with a path that does not exist in the tree will add a new leaf. If this happens after stopPopulating has been run, the nsISubscribableServer will be in an inconsistent state since its internal row list will not be updated accordingly.

This seems to be the simplest fix for this, with the side effect that all subscribed groups will also appear in the New Groups list.

Comment 5

[Hartmut Welpmann [:welpy-cw]]

To fix this properly, I think extensive changes to nsSubscribableServer.cpp are necessary, first of all to have an nsSubscribableServer::FindNode(…) method (similar to nsSubscribableServer::FindAndCreateNode(…)) at hand, that does not add a new node. Then several methods like hasChildren, isSubscribed, isSubscribable, or getLeafName could be made read-only to be semantically more correct.

Furthermore, a fail-safe check for mRowMap.IndexOf(node->prevSibling, aIndex) returning -1 here could be considered.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/87b6781e744b
Fix possible crash in NNTP subscription dialog. r=mkmelin