After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 791148 - Crashes if clock is not in GWeather world
Crashes if clock is not in GWeather world
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
3.26.x
Other Linux
: Normal blocker
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2017-12-02 23:25 UTC by Bastien Nocera
Modified: 2017-12-06 01:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
dateMenu: Fix possible crash with unknown locations (1.18 KB, patch)
2017-12-04 13:31 UTC, Bastien Nocera
none Details | Review
dateMenu: Ignore malformed world-clocks settings (898 bytes, patch)
2017-12-05 23:40 UTC, Bastien Nocera
committed Details | Review
dateMenu: Fix possible crash with unknown locations (1.14 KB, patch)
2017-12-05 23:40 UTC, Bastien Nocera
committed Details | Review

Description Bastien Nocera 2017-12-02 23:25:27 UTC
gnome-shell-3.26.2-1.fc27.x86_64

1. Add UTC support to gnome-clocks:
https://bugzilla.gnome.org/show_bug.cgi?id=791066
https://bugzilla.gnome.org/show_bug.cgi?id=692243
2. Add the UTC clock
3. Restart your non-jhbuilt gnome-shell session

gnome-shell won't be able to find the location in question, and crash your session.

JS ERROR: TypeError: b.location is null
WorldClocksSection<._clocksChanged/<@resource:///org/gnome/shell/ui/dateMenu.js:141:1
WorldClocksSection<._clocksChanged@resource:///org/gnome/shell/ui/dateMenu.js:139:9
wrapper@resource:///org/gnome/gjs/modules/_legacy.js:82:22
AppSettingsMonitor<._connectHandler@resource:///org/gnome/shell/misc/util.js:483:9
wrapper@resource:///org/gnome/gjs/modules/_legacy.js:82:22
AppSettingsMonitor<.watchSetting@resource:///org/gnome/shell/misc/util.js:474:9
wrapper@resource:///org/gnome/gjs/modules/_legacy.js:82:22
WorldClocksSection<._init@resource:///org/gnome/shell/ui/dateMenu.js:119:9
wrapper@resource:///org/gnome/gjs/modules/_legacy.js:82:22
_Base.prototype._construct@resource:///org/gnome/gjs/modules/_legacy.js:18:5
Class.prototype._construct/newClass@resource:///org/gnome/gjs/modules/_legacy.js:114:32
DateMenuButton<._init@resource:///org/gnome/shell/ui/dateMenu.js:541:28
wrapper@resource:///org/gnome/gjs/modules/_legacy.js:82:22
_Base.prototype._construct@resource:///org/gnome/gjs/modules/_legacy.js:18:5
Class.prototype._construct/newClass@resource:///org/gnome/gjs/modules/_legacy.js:114:32
Panel<._ensureIndicator@resource:///org/gnome/shell/ui/panel.js:1099:25
wrapper@resource:///org/gnome/gjs/modules/_legacy.js:82:22
Panel<._updateBox@resource:///org/gnome/shell/ui/panel.js:1110:29
wrapper@resource:///org/gnome/gjs/modules/_legacy.js:82:22
Panel<._updatePanel@resource:///org/gnome/shell/ui/panel.js:1020:9
wrapper@resource:///org/gnome/gjs/modules/_legacy.js:82:22
Panel<._init@resource:///org/gnome/shell/ui/panel.js:822:9
wrapper@resource:///org/gnome/gjs/modules/_legacy.js:82:22
_Base.prototype._construct@resource:///org/gnome/gjs/modules/_legacy.js:18:5
Class.prototype._construct/newClass@resource:///org/gnome/gjs/modules/_legacy.js:114:32
_initializeUI@resource:///org/gnome/shell/ui/main.js:172:13
start@resource:///org/gnome/shell/ui/main.js:126:5
@<main>:1:31
Comment 1 Bastien Nocera 2017-12-04 13:31:57 UTC
Created attachment 364910 [details] [review]
dateMenu: Fix possible crash with unknown locations

If there are locations unknown to the libgweather version gnome-shell is
using, don't crash.

JS ERROR: TypeError: b.location is null
WorldClocksSection<._clocksChanged/<@resource:///org/gnome/shell/ui/dateMenu.js:141:1
WorldClocksSection<._clocksChanged@resource:///org/gnome/shell/ui/dateMenu.js:139:9
Comment 2 Bastien Nocera 2017-12-04 13:33:07 UTC
I didn't test the patch, but you should be able to reproduce it by setting your world clocks to this:

$ gsettings get org.gnome.clocks world-clocks
[{'location': <(uint32 2, <('UTC', 'UTC', false, @a(dd) [], @a(dd) [])>)>}]
Comment 3 Florian Müllner 2017-12-05 10:58:05 UTC
Review of attachment 364910 [details] [review]:

::: js/ui/dateMenu.js
@@ +134,3 @@
         for (let i = 0; i < clocks.length; i++) {
+            if (!clocks[i].location)
+                continue;

This looks wrong, or rather: It doesn't fix the issue in this bug.

The check guards against a malformed setting that doesn't contain a 'location' key. GNOME Clocks will never write such a value, so this is defending against something else setting the key to a bogus value (like `gsettings set org.gnome.clocks world-clocks '[{ "foobar": <"hahaha"> }]'`). This looks like a good idea, but then the commit message needs to be adjusted to reflect what's being fixed.


In order to actually address the handling of unknown locations, you need to check the return value of the deserialize call below:

    let l = world.deserialize(clocks[i].location);
    if (l)
        this._locations.push({ location: l });
Comment 4 Bastien Nocera 2017-12-05 23:40:19 UTC
Created attachment 365073 [details] [review]
dateMenu: Ignore malformed world-clocks settings
Comment 5 Bastien Nocera 2017-12-05 23:40:25 UTC
Created attachment 365074 [details] [review]
dateMenu: Fix possible crash with unknown locations

If there are locations unknown to the libgweather version gnome-shell is
using, don't crash.

JS ERROR: TypeError: b.location is null
WorldClocksSection<._clocksChanged/<@resource:///org/gnome/shell/ui/dateMenu.js:141:1
WorldClocksSection<._clocksChanged@resource:///org/gnome/shell/ui/dateMenu.js:139:9
Comment 6 Florian Müllner 2017-12-05 23:44:59 UTC
Review of attachment 365073 [details] [review]:

Missing commit message body, but OK
Comment 7 Florian Müllner 2017-12-05 23:45:14 UTC
Review of attachment 365074 [details] [review]:

LGTM
Comment 8 Bastien Nocera 2017-12-06 01:03:47 UTC
I added a commit message body for the first patch as well.

Attachment 365073 [details] pushed as a29ceb3 - dateMenu: Ignore malformed world-clocks settings
Attachment 365074 [details] pushed as 29987ae - dateMenu: Fix possible crash with unknown locations