I was looking at the Kwallet framework (https://invent.kde.org/frameworks/kwallet) and I found something somewhat confusing. In the KWallet C++ API, the Kwallet::readPassword() method is documented as follows (documentation here):

Read the password entry key from the current folder.

Parameters
key The key of the entry to read.
value A password buffer to fill with the value.
Returns
Returns 0 on success, non-zero on error. Will return an error if the key was not originally written as a password.

This makes me think that doing readPassword("non-existing-key", ...) should return an error code, and not success (zero). However, in its implementation I can see that it always returns success, except if a DBus error occurs.

In fact, in the implementation of KWalletD::readPassword() I can see that the return value is always a QString. Even when no password entry with such a name exists, or when another entry with the same name but different type exists, the return value is an empty QString ("").

Code: Select all

QString KWalletD::readPassword(int handle, const QString &folder, const QString &key, const QString &appid)
{
    KWallet::Backend *b;

    if ((b = getWallet(appid, handle))) {
        b->setFolder(folder);
        KWallet::Entry *e = b->readEntry(key);
        if (e && e->type() == KWallet::Wallet::Password) {
            return e->password();
        }
    }

    return QString();
}


Is this the intended implementation? It does not seem like a good way to handle non-existing entries or entries of different type. How should an application recognize that there actually is a password in the wallet? After all, one application could actually store an empty password item in the wallet, and in that case it would get exactly the same result back (i.e. an empty string) when reading it.
An application could theoretically first check with the hasEntry() method and then query the entryType() before finally doing readPassword(), but this seems more complex than needed, and more importantly easily prone to TOCTTOU bugs.

In fact, an application could very well get hasEntry() => true and entryType() => EntryType::Password, but then the password gets removed by the user or some other app before the readPassword() call, and the application reads an empty string, thinking that it's valid, when in reality it was just returned as a default value instead of returning some kind of error.

This ambiguous implementation is the same for all types of entry in KWalletD:

1. KWalletD::readEntry() returns an empty QByteArray by default
2. KWalletD::readPassword() returns an empty QString by default
3. KWalletD::readMap() returns an empty QByteArray by default

In light of the above, shouldn't the current KWalletD runtime and KWallet API implementations be changed?

Nevermind! I found this problem mentioned in this pretty old bug report from 2006 (https://bugs.kde.org/show_bug.cgi?id=138841) so the issue is known. Just a design flaw that stuck around for a long time and will probably never be fixed.