GNOME Bugzilla – Bug 694735
giscanner: give error when utf8 strings are annotated with length argument
Last modified: 2018-02-08 12:20:45 UTC
There have been a number of occasions where APIs either take strings with a length arg or return them. The following example came from bug 694448 /** * secret_value_get: * @value: the value * @length: (out): the length of the secret * * Returns: (array length=length): the secret data */ const gchar * secret_value_get (SecretValue *value, gsize *length) This creates the following GIR: <method name="get" c:identifier="secret_value_get"> <return-value transfer-ownership="none"> <doc xml:whitespace="preserve">the secret data</doc> <array length="0" zero-terminated="0" c:type="gchar*"> <type name="utf8" c:type="gchar"/> </array> </return-value> The problem here is language bindings will interpret this as an array of utf8 strings based on the definition of GI_TYPE_TAG_UTF8 being "UTF-8 encoded string". It is unclear if this is a bug in scanner output. If it is not, it would be nice to support a "char" type so we can annotate this as follows: Returns: (array length=length) (element-type char): There is currently a way to work around this by specifying an element-type of "uint8" which bindings will special case. But we cannot assume utf8 encoding when this is done.
the API in question is... weird, to say the least. if it's a user string (i.e. it's UTF-8 encoded) why does the function have a length out argument? the string will be NUL terminated. if the string is meant to have embedded NULs then you should be returning an array of uint8, not a const char*; but embedded NULs mean that the returned value is a binary blob, and not really a UTF-8 string.
For the method in question the right thing ended up being to use a raw uint8 array. However, I think the original annotation should produce a better gir output or at least give a warning. The issue from the binding perspective is we interpret this as an array of utf8 string pointers which crashes during marshaling for obvious reasons. There are other APIs which take input strings with an optional length arg and suffer the same interpretation problem of the annotation (gtk_builder_add_from_string, gtk_text_buffer_insert_text). In PyGObject we can attempt to work around these APIs but it seems like it would be beneficial for other languages to at least give a warning. This will also help minimize future support load.
(In reply to comment #0) > There is currently a way to work around this by specifying an element-type of > "uint8" which bindings will special case. But we cannot assume utf8 encoding > when this is done. Would it work for gtk_text_buffer_insert()? Something like: > @text: (array length=len) (element-type uint8): text in UTF-8 format > @len: length of text in bytes, or -1 The type of @text is "const gchar *". Changing the type of @text to "const guint8 *" is probably not possible, it would be an API break.
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]
Discussion about byte arrays vs (type utf8) strings aside, the element type in the array is obviously wrong. Since the return value is introspected as an array, the element should be the fundamental type "gchar", not "utf8".
(In reply to Sébastien Wilmet from comment #3) > (In reply to comment #0) > > There is currently a way to work around this by specifying an element-type of > > "uint8" which bindings will special case. But we cannot assume utf8 encoding > > when this is done. > > Would it work for gtk_text_buffer_insert()? Something like: > > > @text: (array length=len) (element-type uint8): text in UTF-8 format > > @len: length of text in bytes, or -1 > > The type of @text is "const gchar *". Of course not: the argument is a UTF-8 string, NUL-terminated, like every other string in GTK. The len argument is not an out argument like the one in the description. It's the usual "I'm lazy and I don't want to call strlen(), so do it for me" argument, which can also be used to inject slices of a larger buffer. > Changing the type of @text to "const guint8 *" is probably not possible, it > would be an API break. It would also be wrong; GtkTextBuffer does not work in terms of binary blobs.
(In reply to Emmanuele Bassi (:ebassi) from comment #6) > Of course not: the argument is a UTF-8 string, NUL-terminated, like every > other string in GTK. > > The len argument is not an out argument like the one in the description. > It's the usual "I'm lazy and I don't want to call strlen(), so do it for me" > argument, which can also be used to inject slices of a larger buffer. The issue with the len argument here and in similar APIs is that it has to be annotated for the bindings to never expose it and always pass -1 to it, if the string argument is to be passed as a NUL-terminated string. If the function can't correctly handle strings with inner NULs, there is trouble either way. See here for more discussion, and an example of an API that can safely take byte arrays even if it expects valid UTF-8: https://bugzilla.gnome.org/show_bug.cgi?id=756128#c7
IIRC why I commented on this bug, it's not (only) because of the gtk_text_buffer_insert() function, it's because of the GtkTextBuffer::insert-text signal. GtkSourceView was calling gtk_text_buffer_insert() with a string *not* nul-terminated, taking advantage of the length argument. Then it made the application to crash with a Python plugin in gedit, when listening to the signal in Python it didn't work as expected. The workaround was to nul-terminate the string in GtkSourceView.
Yes, see bug #726689 (pygobject issue).
-- 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/gobject-introspection/issues/81.