[virt-tools-list] [PATCH virt-viewer v3 3/3] app: Check validity of hotkey

[Pavel Grunt]

On Thu, 2016-06-02 at 09:39 +0200, Fabiano Fidêncio wrote:
> On Tue, May 31, 2016 at 11:01 AM, Pavel Grunt <pgrunt at redhat.com> wrote:
> > The hotkey is valid if it has a valid value. The value is valid if it is
> > not empty and is successfully parsed by gtk_accelerator_parse().
> > 
> > These hotkeys formats are considered invalid:
> >  "key" - missing value
> >  "key=" - missing value
> >  "key=abcd" - value cannot be parsed by gtk_accelerator_parse()
> > 
> > Resolves: rhbz#1339572
> > ---
> >  src/virt-viewer-app.c | 13 +++++++++----
> >  tests/test-hotkeys.c  | 10 +++++++++-
> >  2 files changed, 18 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> > index d449f72..c320d38 100644
> > --- a/src/virt-viewer-app.c
> > +++ b/src/virt-viewer-app.c
> > @@ -2065,18 +2065,23 @@ virt_viewer_app_set_hotkeys(VirtViewerApp *self,
> > const gchar *hotkeys_str)
> > 
> >      for (hotkey = hotkeys; *hotkey != NULL; hotkey++) {
> >          gchar *key = strstr(*hotkey, "=");
> > -        if (key == NULL) {
> > -            g_warn_if_reached();
> > +        const gchar *value = (key == NULL) ? NULL : (*key = '\0', key + 1);
> > +        if (value == NULL || *value == '\0') {
> 
> Hmm. Not a big fan of this part of the code, but I can't come up with
> something better.
> 
> 
> > +            g_warning("missing value for key '%s'", *hotkey);
> >              continue;
> >          }
> > -        *key = '\0';
> > 
> > -        gchar *accel = spice_hotkey_to_gtk_accelerator(key + 1);
> > +        gchar *accel = spice_hotkey_to_gtk_accelerator(value);
> >          guint accel_key;
> >          GdkModifierType accel_mods;
> >          gtk_accelerator_parse(accel, &accel_key, &accel_mods);
> >          g_free(accel);
> > 
> > +        if (accel_key == 0 && accel_mods == 0) {
> > +            g_warning("Invalid value '%s' for key '%s'", value, *hotkey);
> > +            continue;
> > +        }
> > +
> >          if (g_str_equal(*hotkey, "toggle-fullscreen")) {
> >              gtk_accel_map_change_entry("<virt-viewer>/view/toggle-
> > fullscreen", accel_key, accel_mods, TRUE);
> >          } else if (g_str_equal(*hotkey, "release-cursor")) {
> > diff --git a/tests/test-hotkeys.c b/tests/test-hotkeys.c
> > index d3658f5..cd2bd88 100644
> > --- a/tests/test-hotkeys.c
> > +++ b/tests/test-hotkeys.c
> > @@ -91,11 +91,19 @@ test_hotkeys_bad(void)
> >          {
> >              "no_value",
> >              G_LOG_LEVEL_WARNING,
> > -            "*code should not be reached"
> > +            "missing value for key 'no_value'"
> > +        },{
> > +            "smartcard-insert=",
> > +            G_LOG_LEVEL_WARNING,
> > +            "missing value for key 'smartcard-insert'"
> >          },{
> >              "toggle-fullscreen=A,unknown_command=B",
> >              G_LOG_LEVEL_WARNING,
> >              "Unknown hotkey command unknown_command"
> > +        },{
> > +            "secure-attention=value",
> > +            G_LOG_LEVEL_WARNING,
> > +            "Invalid value 'value' for key 'secure-attention'"
> >          },
> >      };
> > 
> > --
> > 2.8.3
> > 
> > _______________________________________________
> > virt-tools-list mailing list
> > virt-tools-list at redhat.com
> > https://www.redhat.com/mailman/listinfo/virt-tools-list
> 
> Acked-by: Fabiano Fidêncio <fidencio at redhat.com>
> 
> As the other two patches of this series were already ACKed, go ahead
> for this series.

Thanks, pushed

Pavel