GNOME Bugzilla – Bug 702121
Gnome-Shell does not handle invalid URIs gracefully
Last modified: 2013-06-18 14:56:36 UTC
Seems like gonme shell (particularly gdm-3.8) does not handle properly invalid URIs passed as parameter for the background image. Something like this seems to break the GDM startup, resulting in a black screen and no option to login: /usr/share/glib-2.0/schemas/mint-artwork-mate.gschema.override:picture-uri='/usr/share/backgrounds/linuxmint/default_background.jpg' Note the URI points to a file without the protocol. JS LOG: IBus version is too old JS ERROR: !!! Exception in callback for signal: sessions-loaded JS ERROR: !!! message = '"Argument 'filename' (type utf8) may not be null"' JS ERROR: !!! fileName = '"/usr/share/gnome-shell/js/ui/background.js"' JS ERROR: !!! lineNumber = '171' JS ERROR: !!! stack = '"BackgroundCache<._loadImageContent@/usr/share/gnome-shell/js/ui/background.js:171 wrapper@/usr/share/gjs-1.0/lang.js:213 BackgroundCache<.getImageContent@/usr/share/gnome-shell/js/ui/background.js:252 wrapper@/usr/share/gjs-1.0/lang.js:213 Background<._loadFile@/usr/share/gnome-shell/js/ui/background.js:548 wrapper@/usr/share/gjs-1.0/lang.js:213 Background<._load@/usr/share/gnome-shell/js/ui/background.js:576 wrapper@/usr/share/gjs-1.0/lang.js:213 Background<._init@/usr/share/gnome-shell/js/ui/background.js:325 wrapper@/usr/share/gjs-1.0/lang.js:213 _Base._construct@/usr/share/gjs-1.0/lang.js:154 Class._construct/newClass@/usr/share/gjs-1.0/lang.js:248 BackgroundManager<._createBackground@/usr/share/gnome-shell/js/ui/background.js:770 wrapper@/usr/share/gjs-1.0/lang.js:213 BackgroundManager<._init@/usr/share/gnome-shell/js/ui/background.js:718 wrapper@/usr/share/gjs-1.0/lang.js:213 _Base._construct@/usr/share/gjs-1.0/lang.js:154 Class._construct/newClass@/usr/share/gjs-1.0/lang.js:248 ScreenShield<._createBackground@/usr/share/gnome-shell/js/ui/screenShield.js:554 wrapper@/usr/share/gjs-1.0/lang.js:213 ScreenShield<._updateBackgrounds@/usr/share/gnome-shell/js/ui/screenShield.js:569 wrapper@/usr/share/gjs-1.0/lang.js:213 ScreenShield<._init@/usr/share/gnome-shell/js/ui/screenShield.js:453 wrapper@/usr/share/gjs-1.0/lang.js:213 _Base._construct@/usr/share/gjs-1.0/lang.js:154 Class._construct/newClass@/usr/share/gjs-1.0/lang.js:248 _initializeUI@/usr/share/gnome-shell/js/ui/main.js:148 _sessionsLoaded@/usr/share/gnome-shell/js/ui/main.js:109 _emit@/usr/share/gjs-1.0/signals.js:124 SessionMode<.init/<@/usr/share/gnome-shell/js/ui/sessionMode.js:171 done@/usr/share/gnome-shell/js/misc/fileUtils.js:33 @/usr/share/gnome-shell/js/misc/fileUtils.js:43 "' Window manager warning: Log level 16: Theme parsing error: gtk-widgets.css:1799:15: Not using units is deprecated. Assuming 'px'. gnome-session[7208]: Gdk-WARNING: gnome-session: Fatal IO error 11 (Resource temporarily unavailable) on X server :0. (gnome-settings-daemon:7231): Gdk-WARNING **: gnome-settings-daemon: Fatal IO error 11 (Resource temporarily unavailable) on X server :0. Window manager warning: Log level 16: gnome-shell: Fatal IO error 11 (Resource temporarily unavailable) on X server :0.
Created attachment 246765 [details] [review] Background: don't require a URI scheme for picture-uri Migration from old settings can result in a path instead of URI there. This is technically invalid, but can easily recognize it and avoid the crash.
Review of attachment 246765 [details] [review]: I think this was just a transient configuration bug in the mate config in mint. I don't think we've ever supported non-uri's since the key name changed from picture_filename to picture_uri a long time ago, right? ::: js/ui/background.js @@ +579,1 @@ The issue with this approach is the crash could still happen with different input (a uri that has something other than a file:// prefix)
Created attachment 246796 [details] [review] background: validate picture file path before trying to load it If a user sets their background picture-uri to and invalid uri, or non-local uri then the shell will crash. This commit changes the above type of errors to cause the background handling code to bail out early instead.
what's your opinion of attachment 246796 [details] [review] as an alternative?
(In reply to comment #4) > what's your opinion of attachment 246796 [details] [review] as an alternative? It's undoubtedly more robust, but I'd rather recover from it than to show no background at all. Also, consider that non local URIs do work, they go through fuse. Arguably, we should fix gdk-pixbuf to use GFile anyway, so even that problem would solve itself. The only case where get_path() returns null is a GFile with an unsupported scheme, which is quite unlikely to happen.
So it seems like the best way forward is a combination of both approaches. + let filename; + if (GLib.uri_parse_scheme(uri) != null) + filename = Gio.File.new_for_uri(uri).get_path(); + else + filename = uri; + + if (!filename) { + this._setLoaded(); + return; + } +
(In reply to comment #6) > So it seems like the best way forward is a combination of both approaches. > [...] Indeed, this seems the best approach.
Attachment 246765 [details] pushed as 798a0ca - Background: don't require a URI scheme for picture-uri