[spice-html5] Check if streams array exists in handle_draw_jpeg_onload

Submitted by Christophe Fergeau on May 16, 2018, 10:17 a.m.

Details

Message ID 20180516101728.360-1-cfergeau@redhat.com
State New
Headers show
Series "Check if streams array exists in handle_draw_jpeg_onload" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Christophe Fergeau May 16, 2018, 10:17 a.m.
From: Joel Purra <mig@joelpurra.se>

- It seems `SpiceDisplayConn` does not always have the array `this.o.sc.streams` set.
- It also seems (stream?) images can be loaded before `streams` is set.
- Without `streams`, or the specific stream matching `this.o.id`, `this.o.sc.streams[this.o.id].frames_loading` cannot be accessed.
- The check for the specific stream woth `this.o.id` is already in place, this patch adds a check for `this.o.sc.streams` in `handle_draw_jpeg_onload`.
- There might be a better place to ensure that `this.o.sc.streams` is initialized; this quick fix saved my bacon today though.

Might be related to, and perhaps fix:

- https://bugs.freedesktop.org/show_bug.cgi?id=94776
- https://bugzilla.redhat.com/show_bug.cgi?id=1323144

Signed-off-by: Joel Purra <mig@joelpurra.se>
---

This is the patch attached in that pull request/added in that bug.

Christophe


 display.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/display.js b/display.js
index 7719b23..dbeffea 100644
--- a/display.js
+++ b/display.js
@@ -883,7 +883,7 @@  function handle_draw_jpeg_onload()
     var temp_canvas = null;
     var context;
 
-    if (this.o.sc.streams[this.o.id])
+    if ("streams" in this.o.sc && this.o.sc.streams[this.o.id])
         this.o.sc.streams[this.o.id].frames_loading--;
 
     /*------------------------------------------------------------

Comments

On 05/16/2018 05:17 AM, Christophe Fergeau wrote:
> From: Joel Purra <mig@joelpurra.se>
> 
> - It seems `SpiceDisplayConn` does not always have the array `this.o.sc.streams` set.
> - It also seems (stream?) images can be loaded before `streams` is set.
> - Without `streams`, or the specific stream matching `this.o.id`, `this.o.sc.streams[this.o.id].frames_loading` cannot be accessed.
> - The check for the specific stream woth `this.o.id` is already in place, this patch adds a check for `this.o.sc.streams` in `handle_draw_jpeg_onload`.
> - There might be a better place to ensure that `this.o.sc.streams` is initialized; this quick fix saved my bacon today though.
> 
> Might be related to, and perhaps fix:
> 
> - https://bugs.freedesktop.org/show_bug.cgi?id=94776
> - https://bugzilla.redhat.com/show_bug.cgi?id=1323144
> 
> Signed-off-by: Joel Purra <mig@joelpurra.se>
> ---
> 
> This is the patch attached in that pull request/added in that bug.
> 
> Christophe
> 
> 
>   display.js | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/display.js b/display.js
> index 7719b23..dbeffea 100644
> --- a/display.js
> +++ b/display.js
> @@ -883,7 +883,7 @@ function handle_draw_jpeg_onload()
>       var temp_canvas = null;
>       var context;
>   
> -    if (this.o.sc.streams[this.o.id])
> +    if ("streams" in this.o.sc && this.o.sc.streams[this.o.id])
>           this.o.sc.streams[this.o.id].frames_loading--;
>   
>       /*------------------------------------------------------------
> 

This fix is obviously correct, and a good change.  However, there is a 
similar check at the bottom of the function that would also be good to 
similarly protect.  (It's not as harmful because if the exception stops 
the async handler, there is nothing else lost as a result).

Cheers,

Jeremy