bug#36834: 27.0.50; [PATCH] password-cache.el: confuses key absence with nil password

[Óscar Fuentes]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

>> The change uses gethash instead of intern-soft, but those functions act
>> differently when the password (the value associated with the key) was
>> nil.
>
> Is it valid for the password to be nil?  The logic in password-read
> suggests otherwise.

Callers are sending nil. If it is not valid, there is a problem
elsewhere, but my understanding is that a nil password means "no
password" and it is cached in the memoization sense.

>> The effect is that every call to password-cache-add with nil as
>> password creates a new timer,
>
> Where is password-cache-add being passed a nil password?

The caller is auth-source-remember, IIRC, which itself is called from
auth-source-search.

>> and password-in-cache-p returns nil if
>> there exists a (key nil) entry on password-data, when previously it
>> would return non-nil.
>
> I think a nil key is also not expected.

(key nil) means a hash table entry with `key' as key and nil as value,
not that key is nil.

> Note that password-in-cache-p is currently identical to
> password-read-from-cache.  One can probably be written in terms of the
> other.

Yes, right now they are identical, which causes a problem, because
checking for key existence shall not be the same as retrieving the value
when value can be nil.

> Even if these "memhash" checks are TRT, I suggest either reusing or
> copying the hash table method of map-contains-key, rather than comparing
> against an interned symbol.

Is map-contains-key available by default? I'm wary of introducing new
dependencies for saving just a few characters.