On Fri, 2009-10-09 at 16:10 +0100, Julian Foad wrote:
> On Fri, 2009-10-09 at 10:43 -0400, Mark Phippard wrote:
> > On Fri, Oct 9, 2009 at 10:37 AM, Julian Foad <julian.foad_at_wandisco.com> wrote:
> > > I found some strange behaviour in simple_gnome_keyring_first_creds() and
> > > the three similar functions. If we try to unlock the keyring and the
> > > attempt fails, the code says, "give up and try the next provider" and
> > > returns no error but does not set (or save) the credentials. Is that
> > > right? I think it should return an error in that situation.
> > Suppose you have a client built with GNOME keyring support but you
> > have never created a keyring and do not use it. Shouldn't SVN just
> > continue to use the pre-existing password mechanisms, albeit with the
> > new warning about plaintext? Why should it throw an error?
> The desired behaviour is that Subversion should try the next provider,
> yes, but that behaviour is obtained by the ..._first_creds() function
> returning an error or setting its return value (*CREDENTIALS) to NULL.
> This function does neither.
Setting *CREDENTIALS to NULL would achieve the same effect in most
callers, and I can't see from the docs what the difference is supposed
to be between those two ways of indicating failure. However, some
callers handle them differently: e.g. get_username() in
subversion/libsvn_ra_local/ra_plugin.c. In that case, it doesn't look
like either behaviour would work: if creds == NULL it sets username = ""
and tries to continue, whereas if the ..._first_creds() returns an error
it would return an error up its call chain. In neither case, from what I
can see at a quick look, does it try another provider.
Urg, maybe I shouldn't have opened this can. I haven't looked at it
before and so I'm making lots of assumptions and not reading all the
code, nor am I easily able to test it though I'll have to if I think I
still want to make any changes.
Received on 2009-10-09 17:24:36 CEST