gbinding: Canonicalise source and target properties

Rather than interning a property name string which isn’t canonicalised,
canonicalise it first, and enforce stricter validation on inputs.

The previous code was not incorrect (since the property machinery would
have canonicalised the property names itself, internally), but would
have resulted in non-canonical property names getting into the GQuark
table unnecessarily. With the new code, the interned property names from
property installation time should be consistently reused.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Helps: #358
This commit is contained in:
Philip Withnall 2019-11-12 19:29:19 +00:00
parent 30e630c9df
commit ae27f50342
2 changed files with 113 additions and 8 deletions

View File

@ -423,6 +423,53 @@ g_binding_finalize (GObject *gobject)
G_OBJECT_CLASS (g_binding_parent_class)->finalize (gobject);
}
/* @key must have already been validated with is_valid()
* Modifies @key in place. */
static void
canonicalize_key (gchar *key)
{
gchar *p;
for (p = key; *p != 0; p++)
{
gchar c = *p;
if (c == '_')
*p = '-';
}
}
/* @key must have already been validated with is_valid() */
static gboolean
is_canonical (const gchar *key)
{
return (strchr (key, '_') == NULL);
}
static gboolean
is_valid_property_name (const gchar *key)
{
const gchar *p;
/* First character must be a letter. */
if ((key[0] < 'A' || key[0] > 'Z') &&
(key[0] < 'a' || key[0] > 'z'))
return FALSE;
for (p = key; *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;
}
static void
g_binding_set_property (GObject *gobject,
guint prop_id,
@ -437,17 +484,35 @@ g_binding_set_property (GObject *gobject,
binding->source = g_value_get_object (value);
break;
case PROP_SOURCE_PROPERTY:
binding->source_property = g_intern_string (g_value_get_string (value));
break;
case PROP_TARGET:
binding->target = g_value_get_object (value);
break;
case PROP_SOURCE_PROPERTY:
case PROP_TARGET_PROPERTY:
binding->target_property = g_intern_string (g_value_get_string (value));
break;
{
gchar *name_copy = NULL;
const gchar *name = g_value_get_string (value);
const gchar **dest;
/* Ensure the name we intern is canonical. */
if (!is_canonical (name))
{
name_copy = g_value_dup_string (value);
canonicalize_key (name_copy);
name = name_copy;
}
if (prop_id == PROP_SOURCE_PROPERTY)
dest = &binding->source_property;
else
dest = &binding->target_property;
*dest = g_intern_string (name);
g_free (name_copy);
break;
}
case PROP_FLAGS:
binding->flags = g_value_get_flags (value);
@ -606,7 +671,10 @@ g_binding_class_init (GBindingClass *klass)
* GBinding:source-property:
*
* The name of the property of #GBinding:source that should be used
* as the source of the binding
* as the source of the binding.
*
* This should be in [canonical form][canonical-parameter-names] to get the
* best performance.
*
* Since: 2.26
*/
@ -622,7 +690,10 @@ g_binding_class_init (GBindingClass *klass)
* GBinding:target-property:
*
* The name of the property of #GBinding:target that should be used
* as the target of the binding
* as the target of the binding.
*
* This should be in [canonical form][canonical-parameter-names] to get the
* best performance.
*
* Since: 2.26
*/
@ -835,8 +906,10 @@ g_object_bind_property_full (gpointer source,
g_return_val_if_fail (G_IS_OBJECT (source), NULL);
g_return_val_if_fail (source_property != NULL, NULL);
g_return_val_if_fail (is_valid_property_name (source_property), NULL);
g_return_val_if_fail (G_IS_OBJECT (target), NULL);
g_return_val_if_fail (target_property != NULL, NULL);
g_return_val_if_fail (is_valid_property_name (target_property), NULL);
if (source == target && g_strcmp0 (source_property, target_property) == 0)
{

View File

@ -313,6 +313,37 @@ binding_default (void)
g_assert (binding == NULL);
}
static void
binding_canonicalisation (void)
{
BindingSource *source = g_object_new (binding_source_get_type (), NULL);
BindingTarget *target = g_object_new (binding_target_get_type (), NULL);
GBinding *binding;
g_test_summary ("Test that bindings set up with non-canonical property names work");
binding = g_object_bind_property (source, "double_value",
target, "double_value",
G_BINDING_DEFAULT);
g_object_add_weak_pointer (G_OBJECT (binding), (gpointer *) &binding);
g_assert_true ((BindingSource *) g_binding_get_source (binding) == source);
g_assert_true ((BindingTarget *) g_binding_get_target (binding) == target);
g_assert_cmpstr (g_binding_get_source_property (binding), ==, "double-value");
g_assert_cmpstr (g_binding_get_target_property (binding), ==, "double-value");
g_assert_cmpint (g_binding_get_flags (binding), ==, G_BINDING_DEFAULT);
g_object_set (source, "double-value", 24.0, NULL);
g_assert_cmpfloat (target->double_value, ==, source->double_value);
g_object_set (target, "double-value", 69.0, NULL);
g_assert_cmpfloat (source->double_value, !=, target->double_value);
g_object_unref (target);
g_object_unref (source);
g_assert_null (binding);
}
static void
binding_bidirectional (void)
{
@ -734,6 +765,7 @@ main (int argc, char *argv[])
g_test_bug_base ("https://gitlab.gnome.org/GNOME/glib/issues/");
g_test_add_func ("/binding/default", binding_default);
g_test_add_func ("/binding/canonicalisation", binding_canonicalisation);
g_test_add_func ("/binding/bidirectional", binding_bidirectional);
g_test_add_func ("/binding/transform", binding_transform);
g_test_add_func ("/binding/transform-default", binding_transform_default);