Closed Bug 1282419 Opened 4 years ago Closed 3 years ago
Large libnotify alert images trigger a SIGTERM
STR: 1. Open https://tests.peter.sh/notification-generator/. 2. Select "Huge icon (PNG, 3333x5230)" from the "Icon" drop-down menu. 3. Click "Display the notification." The GDB backtrace shows a call to `notify_notification_show`, so we at least make it that far.
I can still reproduce it on GNOME version 3.24.2 with Firefox 55. Test site: https://files.hboeck.de/crashff.html
See some more info in bug 1395940.
Priority: -- → P1
I see that we're always decoding the image to its native size . This seems to be getting stuck in dbus-daemon: when we send a large enough image it rejects it as out of spec (see  and ), then Firefox halts on the broken connection , because glib dbus connections default to exit on close. The notification example is 66.5 MB of RGBA, DBUS_MAXIMUM_ARRAY_LENGTH is 64 MB. We should probably resize the image if it is going to go over the limit. It may be a bug for glib to be sending arrays this large as the dbus documentation is pretty clear about the size limit . (At least this what I found through testing on Lubuntu 17.10, I didn't see notify_notification_show explicitly in the backtrace so I may be looking at a different issue?)  https://searchfox.org/mozilla-central/rev/7a8c667bdd2a4a32746c9862356e199627c0896d/toolkit/components/alerts/AlertNotification.cpp#250-254  https://cgit.freedesktop.org/dbus/dbus/tree/dbus/dbus-marshal-validate.c?id=2f8f4d619b16b134671521c2b4aea3a94fb47848#n447  https://cgit.freedesktop.org/dbus/dbus/tree/dbus/dbus-transport.c?id=2f8f4d619b16b134671521c2b4aea3a94fb47848#n1186  https://github.com/GNOME/glib/blob/glib-2-54/gio/gdbusconnection.c#L802-L807  https://dbus.freedesktop.org/doc/api/html/group__DBusProtocol.html#ga5265afa08a4c8d9f31b287a57e8cb217
Resizing turned out to be more of an ordeal than I expected. In the interest of getting this fixed now, here's a patch that prevents us from sending such large images. The size limit isn't exactly 64MB because the image data array is wrapped in the hints array, which has the same limit. I added a bit of fudge beyond the 60 bytes that are definitely needed by libnotify. Fortunately we don't set any other hints so there aren't known variable sizes to take into consideration, but parts of the message depend on the protocol.
Assignee: nobody → agashlin
Attachment #8932682 - Flags: review?(karlt)
Comment on attachment 8932682 [details] [diff] [review] Don't attempt to notify with a large icon Thank you for the analysis. If not pushing future patches to reviewboard, then please update ~/.hgrc to include more context in patches by default. This might be the relevant portion: [diff] git = true showfunc = 1 unified = 8
Attachment #8932682 - Flags: review?(karlt) → review+
Thanks karlt, and sorry about the ugly patch, I'll make those changes and try to remember reviewboard next time. Try looks ok. https://treeherder.mozilla.org/#/jobs?repo=try&revision=52016dfa7941711286bbc7c9fbdb5a2b90452fa6
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/a2007a896005 Don't send a large notify image. r=karlt
You need to log in before you can comment on or make changes to this bug.