Here is a better stacktrace (with pygobject dbg symbols installed).

+ Trace 231547

I've also forgot to mention that I'm running Debian sid with the latest updates.

Colud you attach your test.py? Or a small test case which reproduces this problem? It's hard to tell from the stack trace on how search_sync() is being called.

My test.py is exactly what I linked above, here is a raw/downloadable view if you prefer: http://paste.debian.net/plain/236870.

Ooops, sorry for missing the pastebin link :S

This looks like a crash in pygobject. Can reproduce here too. I checked and the value that libsecret is returning from secret_value_get() is kosher. The length is 12 which is correct for "the password".

For reference and debugging, here is the implementation of secret_value_get() and its introspection. I tried removing the (out) from the length parameter, and this didn't fix the crash. Any ideas from the pygobject guys are welcome.

http://git.gnome.org/browse/libsecret/tree/libsecret/secret-value.c#n151

(In reply to comment #4)
> Ooops, sorry for missing the pastebin link :S
> 
> This looks like a crash in pygobject. Can reproduce here too. I checked and the
> value that libsecret is returning from secret_value_get() is kosher. The length
> is 12 which is correct for "the password".

Can you check what's the value that gets passed to PYGLIB_PyUnicode_FromString?

http://git.gnome.org/browse/pygobject/tree/gi/pygi-marshal-to-py.c#n301

The problem here is PyGObject is assuming the array is null terminated (PYGLIB_PyUnicode_FromString) but in this case it is given an explicit length.

Actually the problem is slightly different than this. It seems we are trying to marshal to a unicode string per-item in the array. Besides being wrong, this should be optimized out to just using PyUnicode_FromStringAndSize for the whole array early on, similar to what we do for an array of GI_TYPE_TAG_UINT8.

http://git.gnome.org/browse/pygobject/tree/gi/pygi-marshal-to-py.c#n392

Created attachment 237267 [details]
The test case

Since this was requested on IRC by Tomeu: attached test case downloaded from pastebin.

It turns out PyGObject is attempting the correct behaviour as far as I can tell. GI_TYPE_TAG_UTF8 specifies "a UTF-8 encoded string" and the return annotation from secret_value_get shows the following in the gir:

        <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>

To me the basic type information here means an array of utf8 encoded strings (the c:type obviously shows otherwise). The special case mentioned in comment #8 about GI_TYPE_TAG_UINT8 seems to be for this use case (length terminated string). A brief look at Gjs marshaling and this problem would show up there as well.

Changing the return annotation for secret_get_value to the following fixes the problem:

Returns: (array length=length) (element-type guint8): the secret data

This produces the following gir which PyGObject interprets correctly as a PyString/PyBytes:

        <return-value transfer-ownership="none">
          <doc xml:whitespace="preserve">the secret data</doc>
          <array length="0" zero-terminated="0" c:type="gchar*">
            <type name="guint8"/>
          </array>
        </return-value>

Moving back to libsecret but this could also be considered a GI bug. GI should give an error about this instead of producing a bad gir or it should give a better default for char pointers with length params.

Note this bug is also related to bug 622123 and it seems there has been some historic problems trying to use a utf8 strings with a length parameter.

Created attachment 237353 [details] [review]
Fix introspection for secret_value_get() to return a uint8

Does the attached patch work for you?

This works around a crash in pygobject. It seems to me this should be fixed elsewhere as well. Either with a giscanner warning or not crashing in pygobject. But that's up to the pygobject maintainers.

Stef,

I logged bug 694735 to see if we can get better scanner/binding support. An issue with the uint8 workaround is it will always return a raw PyString/PyBytes without attempting to decode it from utf8. If this is a problem, another workaround would be to have a "binding friendly" function which uses a "Rename to" annotation. This new function would just wrap the old one but not return the length and instead guarantee a zero terminated string is returned.

As for pygobject crashing I don't think there is anything we can do because the only way to interpret the annotations we are given is as an array of utf8 strings, essentially like a GStrv with a specific length.

I think it's appropriate to SecretValue.get() be represented by 'bytes' in python 3 and a non-unicode string in python 2.x. A secret value can be binary data. It's up to the caller to either:

 * Use SecretValue.get_content_type() to determine whether it's text and
   convert.
 * Or use the high level Secret.password_store() functions and friends
   which operate only on text passwords.

Dmitry or Simon, could you review the attached patch before I apply it to libsecret?

The patch fixes the crash for me. However, if SecretValue.get() returns bytes, why doesn't password_store() accept bytes?

>>> Secret.password_store_sync(EXAMPLE_SCHEMA, attributes,
... Secret.COLLECTION_DEFAULT, 'the label', b'the password', None)
Traceback (most recent call last):

+ Trace 231573

(In reply to comment #15)
> The patch fixes the crash for me. However, if SecretValue.get() returns bytes,
> why doesn't password_store() accept bytes?

The password_xxx() functions operate on text passwords. That's the simple API. The lower level API operates on raw binary data.

Attachment 237353 [details] pushed as ddd9bdd - Fix introspection for secret_value_get() to return a uint8

(In reply to comment #16)

Makes sense then, thanks.