[gimp/gimp-2-10] app: in plug-ins, fix invalid parameter names instead of rejecting procedure
- From: Ell <ell src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gimp/gimp-2-10] app: in plug-ins, fix invalid parameter names instead of rejecting procedure
- Date: Sun, 15 Mar 2020 15:17:17 +0000 (UTC)
commit 1af8ad8905352de95bb827beb488aed8997cad71
Author: Ell <ell_se yahoo com>
Date: Sun Mar 15 17:08:38 2020 +0200
app: in plug-ins, fix invalid parameter names instead of rejecting procedure
Instead of rejecting plug-in procedures with invalid parameter- or
return-value names (see issues #4392 and 4641), simply fix the
invalid name and allow the procedure to register. Apparently,
there are plug-ins out there that use invalid parameter names (in
particular, liquid-rescale), so let's keep them working in 2.10.
Show appropriate warning/error for invalid parameter names when not
in PDB compat mode.
app/pdb/gimp-pdb-compat.c | 109 +++++++++++++++++++++++-------
app/pdb/gimp-pdb-compat.h | 3 +-
app/plug-in/gimpplugin-message.c | 139 +++++++++++++++++++++++----------------
app/plug-in/plug-in-rc.c | 2 +-
4 files changed, 169 insertions(+), 84 deletions(-)
---
diff --git a/app/pdb/gimp-pdb-compat.c b/app/pdb/gimp-pdb-compat.c
index 48266bb6d2..b669dfe19c 100644
--- a/app/pdb/gimp-pdb-compat.c
+++ b/app/pdb/gimp-pdb-compat.c
@@ -33,143 +33,154 @@
#include "gimp-pdb-compat.h"
+/* local function prototypes */
+
+static gchar * gimp_pdb_compat_fix_param_name (const gchar *name);
+
+
/* public functions */
GParamSpec *
gimp_pdb_compat_param_spec (Gimp *gimp,
GimpPDBArgType arg_type,
const gchar *name,
- const gchar *desc)
+ const gchar *desc,
+ gboolean *name_valid)
{
GParamSpec *pspec = NULL;
+ gchar *real_name;
g_return_val_if_fail (GIMP_IS_GIMP (gimp), NULL);
g_return_val_if_fail (name != NULL, NULL);
+ real_name = gimp_pdb_compat_fix_param_name (name);
+
+ if (name_valid) *name_valid = ! strcmp (name, real_name);
+
switch (arg_type)
{
case GIMP_PDB_INT32:
- pspec = gimp_param_spec_int32 (name, name, desc,
+ pspec = gimp_param_spec_int32 (real_name, real_name, desc,
G_MININT32, G_MAXINT32, 0,
G_PARAM_READWRITE);
break;
case GIMP_PDB_INT16:
- pspec = gimp_param_spec_int16 (name, name, desc,
+ pspec = gimp_param_spec_int16 (real_name, real_name, desc,
G_MININT16, G_MAXINT16, 0,
G_PARAM_READWRITE);
break;
case GIMP_PDB_INT8:
- pspec = gimp_param_spec_int8 (name, name, desc,
+ pspec = gimp_param_spec_int8 (real_name, real_name, desc,
0, G_MAXUINT8, 0,
G_PARAM_READWRITE);
break;
case GIMP_PDB_FLOAT:
- pspec = g_param_spec_double (name, name, desc,
+ pspec = g_param_spec_double (real_name, real_name, desc,
-G_MAXDOUBLE, G_MAXDOUBLE, 0.0,
G_PARAM_READWRITE);
break;
case GIMP_PDB_STRING:
- pspec = gimp_param_spec_string (name, name, desc,
+ pspec = gimp_param_spec_string (real_name, real_name, desc,
TRUE, TRUE, FALSE,
NULL,
G_PARAM_READWRITE);
break;
case GIMP_PDB_INT32ARRAY:
- pspec = gimp_param_spec_int32_array (name, name, desc,
+ pspec = gimp_param_spec_int32_array (real_name, real_name, desc,
G_PARAM_READWRITE);
break;
case GIMP_PDB_INT16ARRAY:
- pspec = gimp_param_spec_int16_array (name, name, desc,
+ pspec = gimp_param_spec_int16_array (real_name, real_name, desc,
G_PARAM_READWRITE);
break;
case GIMP_PDB_INT8ARRAY:
- pspec = gimp_param_spec_int8_array (name, name, desc,
+ pspec = gimp_param_spec_int8_array (real_name, real_name, desc,
G_PARAM_READWRITE);
break;
case GIMP_PDB_FLOATARRAY:
- pspec = gimp_param_spec_float_array (name, name, desc,
+ pspec = gimp_param_spec_float_array (real_name, real_name, desc,
G_PARAM_READWRITE);
break;
case GIMP_PDB_STRINGARRAY:
- pspec = gimp_param_spec_string_array (name, name, desc,
+ pspec = gimp_param_spec_string_array (real_name, real_name, desc,
G_PARAM_READWRITE);
break;
case GIMP_PDB_COLOR:
- pspec = gimp_param_spec_rgb (name, name, desc,
+ pspec = gimp_param_spec_rgb (real_name, real_name, desc,
TRUE, NULL,
G_PARAM_READWRITE);
break;
case GIMP_PDB_ITEM:
- pspec = gimp_param_spec_item_id (name, name, desc,
+ pspec = gimp_param_spec_item_id (real_name, real_name, desc,
gimp, TRUE,
G_PARAM_READWRITE);
break;
case GIMP_PDB_DISPLAY:
- pspec = gimp_param_spec_display_id (name, name, desc,
+ pspec = gimp_param_spec_display_id (real_name, real_name, desc,
gimp, TRUE,
G_PARAM_READWRITE);
break;
case GIMP_PDB_IMAGE:
- pspec = gimp_param_spec_image_id (name, name, desc,
+ pspec = gimp_param_spec_image_id (real_name, real_name, desc,
gimp, TRUE,
G_PARAM_READWRITE);
break;
case GIMP_PDB_LAYER:
- pspec = gimp_param_spec_layer_id (name, name, desc,
+ pspec = gimp_param_spec_layer_id (real_name, real_name, desc,
gimp, TRUE,
G_PARAM_READWRITE);
break;
case GIMP_PDB_CHANNEL:
- pspec = gimp_param_spec_channel_id (name, name, desc,
+ pspec = gimp_param_spec_channel_id (real_name, real_name, desc,
gimp, TRUE,
G_PARAM_READWRITE);
break;
case GIMP_PDB_DRAWABLE:
- pspec = gimp_param_spec_drawable_id (name, name, desc,
+ pspec = gimp_param_spec_drawable_id (real_name, real_name, desc,
gimp, TRUE,
G_PARAM_READWRITE);
break;
case GIMP_PDB_SELECTION:
- pspec = gimp_param_spec_selection_id (name, name, desc,
+ pspec = gimp_param_spec_selection_id (real_name, real_name, desc,
gimp, TRUE,
G_PARAM_READWRITE);
break;
case GIMP_PDB_COLORARRAY:
- pspec = gimp_param_spec_color_array (name, name, desc,
+ pspec = gimp_param_spec_color_array (real_name, real_name, desc,
G_PARAM_READWRITE);
break;
case GIMP_PDB_VECTORS:
- pspec = gimp_param_spec_vectors_id (name, name, desc,
+ pspec = gimp_param_spec_vectors_id (real_name, real_name, desc,
gimp, TRUE,
G_PARAM_READWRITE);
break;
case GIMP_PDB_PARASITE:
- pspec = gimp_param_spec_parasite (name, name, desc,
+ pspec = gimp_param_spec_parasite (real_name, real_name, desc,
G_PARAM_READWRITE);
break;
case GIMP_PDB_STATUS:
- pspec = g_param_spec_enum (name, name, desc,
+ pspec = g_param_spec_enum (real_name, real_name, desc,
GIMP_TYPE_PDB_STATUS_TYPE,
GIMP_PDB_EXECUTION_ERROR,
G_PARAM_READWRITE);
@@ -180,8 +191,14 @@ gimp_pdb_compat_param_spec (Gimp *gimp,
}
if (! pspec)
- g_warning ("%s: returning NULL for %s (%s)",
- G_STRFUNC, name, gimp_pdb_compat_arg_type_to_string (arg_type));
+ {
+ g_warning ("%s: returning NULL for %s (%s)",
+ G_STRFUNC,
+ real_name,
+ gimp_pdb_compat_arg_type_to_string (arg_type));
+ }
+
+ g_free (real_name);
return pspec;
}
@@ -501,3 +518,45 @@ gimp_pdb_compat_procs_register (GimpPDB *pdb,
compat_procs[i].new_name);
}
}
+
+
+/* private functions */
+
+/* Since GLib 2.63.3, invalid param-spec names are rejected upon creation.
+ * This impacts procedure parameters and return-values, which are stored as
+ * param-specs internally. Since this requirement wasn't previously enforced,
+ * keep supporting arbitrary parameter names in gimp-2.
+ *
+ * See issues #4392 and #4641.
+ */
+static gchar *
+gimp_pdb_compat_fix_param_name (const gchar *name)
+{
+ GString *new_name;
+
+ new_name = g_string_new (NULL);
+
+ /* First character must be a letter. */
+ if ((name[0] < 'A' || name[0] > 'Z') &&
+ (name[0] < 'a' || name[0] > 'z'))
+ {
+ g_string_append (new_name, "param-");
+ }
+
+ for (; *name; name++)
+ {
+ gchar c = *name;
+
+ if ((c < 'A' || c > 'Z') &&
+ (c < 'a' || c > 'z') &&
+ (c < '0' || c > '9') &&
+ c != '-' && c != '_')
+ {
+ c = '-';
+ }
+
+ g_string_append_c (new_name, c);
+ }
+
+ return g_string_free (new_name, FALSE);
+}
diff --git a/app/pdb/gimp-pdb-compat.h b/app/pdb/gimp-pdb-compat.h
index 74c26959e2..7f12fdf66b 100644
--- a/app/pdb/gimp-pdb-compat.h
+++ b/app/pdb/gimp-pdb-compat.h
@@ -22,7 +22,8 @@
GParamSpec * gimp_pdb_compat_param_spec (Gimp *gimp,
GimpPDBArgType arg_type,
const gchar *name,
- const gchar *desc);
+ const gchar *desc,
+ gboolean *name_valid);
GType gimp_pdb_compat_arg_type_to_gtype (GimpPDBArgType type);
GimpPDBArgType gimp_pdb_compat_arg_type_from_gtype (GType type);
diff --git a/app/plug-in/gimpplugin-message.c b/app/plug-in/gimpplugin-message.c
index a397f83adb..b27035eaa0 100644
--- a/app/plug-in/gimpplugin-message.c
+++ b/app/plug-in/gimpplugin-message.c
@@ -76,7 +76,6 @@ static void gimp_plug_in_handle_proc_uninstall (GimpPlugIn *plug_in,
static void gimp_plug_in_handle_extension_ack (GimpPlugIn *plug_in);
static void gimp_plug_in_handle_has_init (GimpPlugIn *plug_in);
-static gboolean gimp_plug_in_is_valid_property_name (const gchar *name);
/* public functions */
@@ -863,49 +862,105 @@ gimp_plug_in_handle_proc_install (GimpPlugIn *plug_in,
for (i = 0; i < proc_install->nparams; i++)
{
GParamSpec *pspec;
+ gboolean name_valid;
- if (! gimp_plug_in_is_valid_property_name (proc_install->params[i].name))
- {
- gimp_message (plug_in->manager->gimp, NULL, GIMP_MESSAGE_ERROR,
- "Plug-in \"%s\"\n(%s)\n"
- "attempted to install procedure \"%s\" with "
- "invalid parameter name \"%s\".",
- gimp_object_get_name (plug_in),
- gimp_file_get_utf8_name (plug_in->file),
- canonical, proc_install->params[i].name);
- g_object_unref (procedure);
- return;
- }
pspec = gimp_pdb_compat_param_spec (plug_in->manager->gimp,
proc_install->params[i].type,
proc_install->params[i].name,
- proc_install->params[i].description);
+ proc_install->params[i].description,
+ &name_valid);
gimp_procedure_add_argument (procedure, pspec);
+
+ if (pspec && ! name_valid)
+ {
+ switch (plug_in->manager->gimp->pdb_compat_mode)
+ {
+ case GIMP_PDB_COMPAT_ON:
+ break;
+
+ case GIMP_PDB_COMPAT_WARN:
+ gimp_message (plug_in->manager->gimp, NULL, GIMP_MESSAGE_WARNING,
+ "Plug-in \"%s\"\n(%s)\n"
+ "attempted to install procedure \"%s\" "
+ "with invalid parameter name \"%s\".\n"
+ "This is deprecated.\n"
+ "The parameter name was changed to \"%s\".",
+ gimp_object_get_name (plug_in),
+ gimp_file_get_utf8_name (plug_in->file),
+ gimp_object_get_name (proc),
+ proc_install->params[i].name,
+ pspec->name);
+ break;
+
+ case GIMP_PDB_COMPAT_OFF:
+ gimp_message (plug_in->manager->gimp, NULL, GIMP_MESSAGE_ERROR,
+ "Plug-in \"%s\"\n(%s)\n"
+ "attempted to install procedure \"%s\" "
+ "with invalid parameter name \"%s\".\n"
+ "This is not allowed.",
+ gimp_object_get_name (plug_in),
+ gimp_file_get_utf8_name (plug_in->file),
+ gimp_object_get_name (proc),
+ proc_install->params[i].name);
+
+ g_object_unref (proc);
+
+ return;
+ }
+ }
}
for (i = 0; i < proc_install->nreturn_vals; i++)
{
GParamSpec *pspec;
+ gboolean name_valid;
- if (! gimp_plug_in_is_valid_property_name (proc_install->return_vals[i].name))
- {
- gimp_message (plug_in->manager->gimp, NULL, GIMP_MESSAGE_ERROR,
- "Plug-in \"%s\"\n(%s)\n"
- "attempted to install procedure \"%s\" with "
- "invalid return value name \"%s\".",
- gimp_object_get_name (plug_in),
- gimp_file_get_utf8_name (plug_in->file),
- canonical, proc_install->return_vals[i].name);
- g_object_unref (procedure);
- return;
- }
pspec = gimp_pdb_compat_param_spec (plug_in->manager->gimp,
proc_install->return_vals[i].type,
proc_install->return_vals[i].name,
- proc_install->return_vals[i].description);
+ proc_install->return_vals[i].description,
+ &name_valid);
gimp_procedure_add_return_value (procedure, pspec);
+
+ if (pspec && ! name_valid)
+ {
+ switch (plug_in->manager->gimp->pdb_compat_mode)
+ {
+ case GIMP_PDB_COMPAT_ON:
+ break;
+
+ case GIMP_PDB_COMPAT_WARN:
+ gimp_message (plug_in->manager->gimp, NULL, GIMP_MESSAGE_WARNING,
+ "Plug-in \"%s\"\n(%s)\n"
+ "attempted to install procedure \"%s\" "
+ "with invalid return-value name \"%s\".\n"
+ "This is deprecated.\n"
+ "The return-value name was changed to \"%s\".",
+ gimp_object_get_name (plug_in),
+ gimp_file_get_utf8_name (plug_in->file),
+ gimp_object_get_name (proc),
+ proc_install->return_vals[i].name,
+ pspec->name);
+ break;
+
+ case GIMP_PDB_COMPAT_OFF:
+ gimp_message (plug_in->manager->gimp, NULL, GIMP_MESSAGE_ERROR,
+ "Plug-in \"%s\"\n(%s)\n"
+ "attempted to install procedure \"%s\" "
+ "with invalid return-value name \"%s\".\n"
+ "This is not allowed.",
+ gimp_object_get_name (plug_in),
+ gimp_file_get_utf8_name (plug_in->file),
+ gimp_object_get_name (proc),
+ proc_install->return_vals[i].name);
+
+ g_object_unref (proc);
+
+ return;
+ }
+ }
}
/* Sanity check menu path */
@@ -1006,33 +1061,3 @@ gimp_plug_in_handle_has_init (GimpPlugIn *plug_in)
gimp_plug_in_close (plug_in, TRUE);
}
}
-
-/*
- * XXX: this function should be removed when/if it becomes public in
- * glib, i.e. when this patch is merged:
- * https://gitlab.gnome.org/GNOME/glib/merge_requests/1302
- * See #4392.
- */
-static gboolean
-gimp_plug_in_is_valid_property_name (const gchar *name)
-{
- const gchar *p;
-
- /* First character must be a letter. */
- if ((name[0] < 'A' || name[0] > 'Z') &&
- (name[0] < 'a' || name[0] > 'z'))
- return FALSE;
-
- for (p = name; *p != 0; p++)
- {
- const gchar c = *p;
-
- if (c != '-' && c != '_' &&
- (c < '0' || c > '9') &&
- (c < 'A' || c > 'Z') &&
- (c < 'a' || c > 'z'))
- return FALSE;
- }
-
- return TRUE;
-}
diff --git a/app/plug-in/plug-in-rc.c b/app/plug-in/plug-in-rc.c
index 6cf7b9f75f..42cfe3d7c5 100644
--- a/app/plug-in/plug-in-rc.c
+++ b/app/plug-in/plug-in-rc.c
@@ -784,7 +784,7 @@ plug_in_proc_arg_deserialize (GScanner *scanner,
token = G_TOKEN_LEFT_PAREN;
- pspec = gimp_pdb_compat_param_spec (gimp, arg_type, name, desc);
+ pspec = gimp_pdb_compat_param_spec (gimp, arg_type, name, desc, NULL);
if (return_value)
gimp_procedure_add_return_value (procedure, pspec);
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
