[libmediaart/sam/valid-utf8-2: 1/2] Rationalize NULL/empty string handling in media_art_strip_invalid_entities()

[Sam Thursfield <sthursfield src gnome org>]

commit 517a22dcf6dcb9226fdfc805dedcca3febf06cf5
Author: Sam Thursfield <sam afuera me uk>
Date:   Sun Dec 27 13:47:36 2020 +0100

    Rationalize NULL/empty string handling in media_art_strip_invalid_entities()
    
    We return NULL if input is NULL and a newly allocated empty string if
    input is "". Some comments disagreed with this, and it's possible the
    change causes a memory leak in some app, but the alternative of returning
    NULL when passed "" is dangerous as some code may free the return value
    in this case. (In fact, libmediaart itself does so).
    
    Also, make behaviour occur independently of whether
    `G_ENABLE_CONSISTENCY_CHECKS` was defined at build time.

 libmediaart/cache.c  | 17 ++++++++++++++---
 tests/mediaarttest.c | 18 ++++++++++--------
 2 files changed, 24 insertions(+), 11 deletions(-)
---
diff --git a/libmediaart/cache.c b/libmediaart/cache.c
index ecbc7a1..b5b023b 100644
--- a/libmediaart/cache.c
+++ b/libmediaart/cache.c
@@ -101,7 +101,7 @@ media_art_strip_find_next_block (const gchar    *original,
 
 /**
  * media_art_strip_invalid_entities:
- * @original: original string
+ * @original: (nullable): original string
  *
  * Strip a albumname or artistname string to prepare it for calculating the
  * media art path with it. Certain characters and charactersets will be stripped
@@ -116,7 +116,7 @@ media_art_strip_find_next_block (const gchar    *original,
  * 3. Multiples of space characters are removed.
  *
  * Returns: @original stripped of invalid characters which must be
- * freed. On error or if @original is empty, %NULL is returned.
+ * freed. On error or if @original is NULL, %NULL is returned.
  *
  * Since: 0.2.0
  */
@@ -140,7 +140,8 @@ media_art_strip_invalid_entities (const gchar *original)
                {  0,   0  }
        };
 
-       g_return_val_if_fail (original != NULL, NULL);
+       if (original == NULL)
+               return NULL;
 
        str_no_blocks = g_string_new ("");
 
@@ -284,6 +285,10 @@ media_art_get_file (const gchar  *artist,
 
        /* http://live.gnome.org/MediaArtStorageSpec */
 
+       g_return_val_if_fail (!artist || g_utf8_validate (artist, -1, NULL), FALSE);
+       g_return_val_if_fail (!title || g_utf8_validate (title, -1, NULL), FALSE);
+       g_return_val_if_fail (!prefix || g_utf8_validate (prefix, -1, NULL), FALSE);
+
        if (cache_file) {
                *cache_file = NULL;
        }
@@ -381,6 +386,10 @@ media_art_get_path (const gchar  *artist,
 {
        GFile *cache_file = NULL;
 
+       g_return_val_if_fail (!artist || g_utf8_validate (artist, -1, NULL), FALSE);
+       g_return_val_if_fail (!title || g_utf8_validate (title, -1, NULL), FALSE);
+       g_return_val_if_fail (!prefix || g_utf8_validate (prefix, -1, NULL), FALSE);
+
        /* Rules:
         * 1. artist OR title must be non-NULL.
         * 2. cache_file must be non-NULL
@@ -424,6 +433,8 @@ media_art_remove (const gchar   *artist,
        gboolean success = TRUE;
 
        g_return_val_if_fail (artist != NULL && artist[0] != '\0', FALSE);
+       g_return_val_if_fail (g_utf8_validate (artist, -1, NULL), FALSE);
+       g_return_val_if_fail (!album || g_utf8_validate (album, -1, NULL), FALSE);
 
        dirname = g_build_filename (g_get_user_cache_dir (), "media-art", NULL);
 
diff --git a/tests/mediaarttest.c b/tests/mediaarttest.c
index cef36c2..93ac684 100644
--- a/tests/mediaarttest.c
+++ b/tests/mediaarttest.c
@@ -112,18 +112,20 @@ test_mediaart_stripping_failures_subprocess (void)
 static void
 test_mediaart_stripping_failures (void)
 {
-       gchar *stripped = NULL;
+       gchar *stripped, *input = NULL;
 
        /* a. Return NULL for NULL (subprocess)
-        * b. Return NULL for ""
+        * b. Return a copy for ""
         */
-       stripped = media_art_strip_invalid_entities ("");
-       g_assert (stripped);
-       g_assert_cmpstr (stripped, ==, "");
+       stripped = media_art_strip_invalid_entities (NULL);
+       g_assert (!stripped);
 
-       g_test_trap_subprocess ("/mediaart/stripping_failures/subprocess", 0, 0);
-       g_test_trap_assert_failed ();
-       g_test_trap_assert_stderr ("*assertion 'original != NULL' failed*");
+       input = "";
+       stripped = media_art_strip_invalid_entities (input);
+       g_assert (stripped);
+       g_assert (stripped != input);
+       g_assert (strcmp(stripped, "") == 0);
+       g_free (stripped);
 }