app, pdb, libimp: prevent core process from raising WARNING on libgimp call.

Whatever a plug-in does, it should not be able to trigger WARNINGs or
CRITICALs on the core process. Here this was possible when requesting an
array of param specs with gimp_drawable_filter_operation_get_pspecs() on
a GEGL operation, while GIMP doesn't support all types for this filter's
arguments. Trying to send unsupported type specs through the wire would
fail.

Unfortunately just saying that we must add support for these types is
not enough because we simply cannot support every possible types. First
because even in current GEGL core filters, there are types we might
never support (e.g. audio fragments?). But even more because third-party
filters could have custom types too (just like our own filters have
custom GIMP types).

So instead, acknowledge that some types cannot be sent through the wire,
verify when it's one such argument and simply output an informational
message on stderr (because the info of a non-supported type is still
interesting in case this is something we should in fact add support for;
it's much better than silently ignoring the argument).
This commit is contained in:
Jehan 2025-07-03 00:16:09 +02:00
parent 19f25bb58e
commit 7189010f1e
7 changed files with 98 additions and 24 deletions

View file

@ -28,10 +28,15 @@
#include <gdk-pixbuf/gdk-pixbuf.h>
#include "libgimpbase/gimpbase.h"
#include "libgimpbase/gimpprotocol.h"
#include "libgimpbase/gimpwire.h"
#include "libgimpbase/gimpbase.h"
#include "pdb-types.h"
#include "libgimp/gimpgpparams.h"
#include "core/gimpcontainer.h"
#include "core/gimpdrawable-filters.h"
#include "core/gimpdrawable.h"
@ -855,15 +860,39 @@ drawable_filter_operation_get_pspecs_invoker (GimpProcedure *procedure,
for (gint i = 0; i < n_specs; i++)
{
GParamSpec *pspec = specs[n_parent_specs + i];
GValue value = G_VALUE_INIT;
GParamSpec *pspec = specs[n_parent_specs + i];
GPParamDef param_def = { 0, };
g_value_init (&value, G_TYPE_PARAM);
/* Make sure we do not try to send param specs over the wire
* if we don't support sending their type.
*/
if (_gimp_param_spec_to_gp_param_def (pspec, &param_def, TRUE))
{
GValue value = G_VALUE_INIT;
g_value_set_param (&value, g_param_spec_ref (pspec));
gimp_value_array_append (pspecs, &value);
g_value_init (&value, G_TYPE_PARAM);
g_value_unset (&value);
g_value_set_param (&value, g_param_spec_ref (pspec));
gimp_value_array_append (pspecs, &value);
g_value_unset (&value);
}
else
{
/* This is not technically a bug if an operation has
* unsupported argument types, because we cannot possibly
* support everything (third-party operations could have any
* kind of arguments; and even official ops have types
* such as audio fragment which we may never support).
* So this should not generate any WARNING. Yet we still
* want to softly notify developers, in case we can
* actually do something about some types.
*/
g_printerr ("%s: ignoring argument '%s' of procedure '%s'. "
"Unsupported argument type '%s'.\n",
G_STRFUNC, pspec->name, operation_name,
g_type_name (G_PARAM_SPEC_VALUE_TYPE (pspec)));
}
}
g_free (specs);

View file

@ -1152,7 +1152,7 @@ plug_in_rc_write_proc_arg (GimpConfigWriter *writer,
{
GPParamDef param_def = { 0, };
_gimp_param_spec_to_gp_param_def (pspec, &param_def);
_gimp_param_spec_to_gp_param_def (pspec, &param_def, FALSE);
gimp_config_writer_open (writer, "proc-arg");
gimp_config_writer_printf (writer, "%d", param_def.param_def_type);

View file

@ -367,12 +367,14 @@ _gimp_gp_param_def_to_param_spec (const GPParamDef *param_def)
return NULL;
}
void
gboolean
_gimp_param_spec_to_gp_param_def (GParamSpec *pspec,
GPParamDef *param_def)
GPParamDef *param_def,
gboolean check_only)
{
GType pspec_type = G_PARAM_SPEC_TYPE (pspec);
GType value_type = G_PARAM_SPEC_VALUE_TYPE (pspec);
gboolean success = TRUE;
param_def->param_def_type = GP_PARAM_DEF_TYPE_DEFAULT;
param_def->type_name = (gchar *) g_type_name (pspec_type);
@ -767,11 +769,19 @@ _gimp_param_spec_to_gp_param_def (GParamSpec *pspec,
}
else
{
g_warning ("%s: GParamSpecObject for unsupported type '%s:%s'",
G_STRFUNC,
param_def->type_name, param_def->value_type_name);
success = FALSE;
if (! check_only)
g_warning ("%s: GParamSpecObject for unsupported type '%s:%s'",
G_STRFUNC,
param_def->type_name, param_def->value_type_name);
}
}
else
{
success = FALSE;
}
return success;
}
static GimpImage *
@ -1709,7 +1719,7 @@ gimp_value_to_gp_param (const GValue *value,
param->param_type = GP_PARAM_TYPE_PARAM_DEF;
_gimp_param_spec_to_gp_param_def (g_value_get_param (value),
&param->data.d_param_def);
&param->data.d_param_def, FALSE);
}
else if (GIMP_VALUE_HOLDS_VALUE_ARRAY (value))
{

View file

@ -26,8 +26,9 @@ G_BEGIN_DECLS
GParamSpec * _gimp_gp_param_def_to_param_spec (const GPParamDef *param_def);
void _gimp_param_spec_to_gp_param_def (GParamSpec *pspec,
GPParamDef *param_def);
gboolean _gimp_param_spec_to_gp_param_def (GParamSpec *pspec,
GPParamDef *param_def,
gboolean check_only);
GimpValueArray * _gimp_gp_params_to_value_array (gpointer gimp,
GParamSpec **pspecs,

View file

@ -411,13 +411,13 @@ gimp_procedure_real_install (GimpProcedure *procedure)
for (i = 0; i < n_args; i++)
{
_gimp_param_spec_to_gp_param_def (args[i],
&proc_install.params[i]);
&proc_install.params[i], FALSE);
}
for (i = 0; i < n_return_vals; i++)
{
_gimp_param_spec_to_gp_param_def (return_vals[i],
&proc_install.return_vals[i]);
&proc_install.return_vals[i], FALSE);
}
plug_in = gimp_procedure_get_plug_in (procedure);

View file

@ -1111,6 +1111,9 @@ GPL
elsif (!/libgimp/) {
s/^/~/;
}
elsif (/libgimp[a-z]/) {
s/^/"/;
}
}
$x cmp $y;
} keys %{$out->{headers}};
@ -1137,7 +1140,7 @@ GPL
$seen = 0 if !/^</;
if (/libgimp/) {
if (/libgimp[a-z]/) {
$lib = 1;
}
else {

View file

@ -825,15 +825,39 @@ HELP
for (gint i = 0; i < n_specs; i++)
{
GParamSpec *pspec = specs[n_parent_specs + i];
GValue value = G_VALUE_INIT;
GParamSpec *pspec = specs[n_parent_specs + i];
GPParamDef param_def = { 0, };
g_value_init (&value, G_TYPE_PARAM);
/* Make sure we do not try to send param specs over the wire
* if we don't support sending their type.
*/
if (_gimp_param_spec_to_gp_param_def (pspec, &param_def, TRUE))
{
GValue value = G_VALUE_INIT;
g_value_set_param (&value, g_param_spec_ref (pspec));
gimp_value_array_append (pspecs, &value);
g_value_init (&value, G_TYPE_PARAM);
g_value_unset (&value);
g_value_set_param (&value, g_param_spec_ref (pspec));
gimp_value_array_append (pspecs, &value);
g_value_unset (&value);
}
else
{
/* This is not technically a bug if an operation has
* unsupported argument types, because we cannot possibly
* support everything (third-party operations could have any
* kind of arguments; and even official ops have types
* such as audio fragment which we may never support).
* So this should not generate any WARNING. Yet we still
* want to softly notify developers, in case we can
* actually do something about some types.
*/
g_printerr ("%s: ignoring argument '%s' of procedure '%s'. "
"Unsupported argument type '%s'.\n",
G_STRFUNC, pspec->name, operation_name,
g_type_name (G_PARAM_SPEC_VALUE_TYPE (pspec)));
}
}
g_free (specs);
@ -903,6 +927,13 @@ CODE
@headers = qw(<gegl.h>
<gegl-plugin.h>
"libgimpbase/gimpbase.h"
"libgimpbase/gimpprotocol.h"
"libgimpbase/gimpwire.h"
"libgimp/gimpgpparams.h"
"core/gimpcontainer.h"
"core/gimpdrawablefilter.h"
"core/gimpdrawable-filters.h"