ensureActiveConnectionProps() erroneously assumes connections have at least one device
While reading a journal generated by Endless' openQA instance, I noticed a backtrace from network.js, which I found in the journal on my own system too:
NetworkManager[1809]: <info> [1560976150.5161] device (wlp2s0): supplicant interface state: inactive -> scanning
NetworkManager[1809]: <info> [1560976151.9586] keyfile: add connection /run/NetworkManager/system-connections/BTHub5-HZ75.nmconnection (f405868c-2e65-4880-8419-aa5d64292abd,"BTHub5-HZ75")
NetworkManager[1809]: <info> [1560976151.9608] device (wlp2s0): Activation: starting connection 'BTHub5-HZ75' (f405868c-2e65-4880-8419-aa5d64292abd)
NetworkManager[1809]: <info> [1560976151.9624] settings-connection[0x562ee1e23470,f405868c-2e65-4880-8419-aa5d64292abd]: write: successfully commited (keyfile: update /etc/NetworkManager/system-connections/BTHub5-HZ75.nmconnection (f405868c-2e65-4
NetworkManager[1809]: <info> [1560976151.9625] audit: op="connection-add-activate" uuid="f405868c-2e65-4880-8419-aa5d64292abd" name="BTHub5-HZ75" pid=22413 uid=1001 result="success"
NetworkManager[1809]: <info> [1560976151.9705] device (wlp2s0): state change: disconnected -> prepare (reason 'none', sys-iface-state: 'managed')
NetworkManager[1809]: <info> [1560976151.9708] manager: NetworkManager state is now CONNECTING
NetworkManager[1809]: <info> [1560976151.9714] device (wlp2s0): state change: prepare -> config (reason 'none', sys-iface-state: 'managed')
NetworkManager[1809]: <info> [1560976151.9718] device (wlp2s0): Activation: (wifi) access point 'BTHub5-HZ75' has security, but secrets are required.
NetworkManager[1809]: <info> [1560976151.9718] device (wlp2s0): state change: config -> need-auth (reason 'none', sys-iface-state: 'managed')
NetworkManager[1809]: <info> [1560976151.9721] sup-iface[0x562ee1e1ccd0,wlp2s0]: wps: type pbc start...
gnome-shell[22413]: JS WARNING: [resource:///org/gnome/shell/ui/status/network.js 73]: reference to undefined property 0
gnome-shell[22413]: JS ERROR: TypeError: active.get_devices(...)[0] is undefined
ensureActiveConnectionProps@resource:///org/gnome/shell/ui/status/network.js:73:22
_getMainConnection@resource:///org/gnome/shell/ui/status/network.js:1791:13
_syncMainConnection@resource:///org/gnome/shell/ui/status/network.js:1809:32
The line numbers are from Endless's fork of gnome-shell but the code in question is the same as in stock gnome-shell's master branch:
function ensureActiveConnectionProps(active, client) {
if (!active._primaryDevice) {
// This list is guaranteed to have only one device in it.
let device = active.get_devices()[0]._delegate; // <-- network.js:73
active._primaryDevice = device;
}
}
// ...
_getMainConnection() {
let connection;
connection = this._client.get_primary_connection();
if (connection) {
ensureActiveConnectionProps(connection, this._client);
return connection;
}
connection = this._client.get_activating_connection();
if (connection) {
ensureActiveConnectionProps(connection, this._client); // <-- network.js:1791
return connection;
}
return null;
}
_syncMainConnection() {
if (this._mainConnectionIconChangedId > 0) {
this._mainConnection._primaryDevice.disconnect(this._mainConnectionIconChangedId);
this._mainConnectionIconChangedId = 0;
}
if (this._mainConnectionStateChangedId > 0) {
this._mainConnection.disconnect(this._mainConnectionStateChangedId);
this._mainConnectionStateChangedId = 0;
}
this._mainConnection = this._getMainConnection(); // <-- network.js:1809
if (this._mainConnection) {
if (this._mainConnection._primaryDevice)
this._mainConnectionIconChangedId = this._mainConnection._primaryDevice.connect('icon-changed', this._updateIcon.bind(this));
this._mainConnectionStateChangedId = this._mainConnection.connect('notify::state', this._mainConnectionStateChanged.bind(this));
this._mainConnectionStateChanged();
}
this._updateIcon();
this._syncConnectivity();
}
The guarantee alleged in ensureActiveConnectionProps
is evidently not true: the list can apparently have 0 devices in it. There is no mention of this guarantee in the nm_active_connection_get_devices() documentation.
Every use of _primaryDevice
is guarded by an (implicit or explicit) truthiness test, so it would work to just check the length of the returned list, but I don't know if that would be strictly correct.