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 784441 - core: Debug know keys and not allow empty strings as metadata
core: Debug know keys and not allow empty strings as metadata
Status: RESOLVED OBSOLETE
Product: grilo
Classification: Other
Component: core
unspecified
Other All
: Normal normal
: ---
Assigned To: grilo-maint
grilo-maint
Depends on:
Blocks:
 
 
Reported: 2017-07-02 08:15 UTC by Victor Toso
Modified: 2018-09-24 09:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
source: Debug 'know' keys that are not going to plugins (1.79 KB, patch)
2017-07-02 08:15 UTC, Victor Toso
needs-work Details | Review
data: Do not allow empty strings either (1.28 KB, patch)
2017-07-02 08:15 UTC, Victor Toso
reviewed Details | Review

Description Victor Toso 2017-07-02 08:15:08 UTC
As bugs in plugins or applications can lead to situations where a emptry string
is set.
Comment 1 Victor Toso 2017-07-02 08:15:12 UTC
Created attachment 354787 [details] [review]
source: Debug 'know' keys that are not going to plugins

If some metadata-keys were requested by application and are not going
all the way to the plugins, we should be able to identify where this
is happening and what metadata-keys are ignored.

filter_know_keys() do not consider the metadata-keys value. It is
reasonable that an application is requesting a different or more
reliable value for a given key using different plugins. The core will
never allow it for now.

At least, let's debug that we are dropping the keys in the core.
Comment 2 Victor Toso 2017-07-02 08:15:18 UTC
Created attachment 354788 [details] [review]
data: Do not allow empty strings either

By design, Grilo checks if we have a metadata-key and if it does,
it'll remove those metadata-keys for some source operations like
grl_source_resolve().

Let's not consider empty strings as valid metadata value.
Comment 3 Bastien Nocera 2017-07-02 20:16:55 UTC
Review of attachment 354787 [details] [review]:

Looks fine apart from those comments.

::: src/grl-source.c
@@ +719,3 @@
   GList *unknown_keys = NULL;
   GList *k;
+  GString *know_keys;

"known". Here, and everywhere else in the patch.

@@ +737,3 @@
 
+  if (know_keys->len > 0) {
+    /* Drop trailing ", " */

In which case you really want to check for "len > 2" if you don't want static checkers coming after you.
Comment 4 Bastien Nocera 2017-07-02 20:21:23 UTC
Review of attachment 354788 [details] [review]:

That seems fine though this bug comes to mind:
https://bugzilla.gnome.org/show_bug.cgi?id=741207

And commit e0e30dfa0ea5d9e4814b2cdb1236abfa74289076
Comment 5 GNOME Infrastructure Team 2018-09-24 09:53:59 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/grilo/issues/118.