[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: Remove svn_auth_cleanup_walk()?

From: Bert Huijben <rhuijben_at_collab.net>
Date: Wed, 3 Apr 2013 19:43:07 +0000

> -----Original Message-----
> From: C. Michael Pilato [mailto:cmpilato_at_collab.net]
> Sent: woensdag 3 april 2013 21:37
> To: Subversion Development
> Cc: Bert Huijben
> Subject: Remove svn_auth_cleanup_walk()?
>
> While reviewing new-to-1.8 APIs, I found myself rather confused by
> svn_auth_cleanup_walk(). It took me a bit to understand the approach, and
> ultimately I realize that this was because the function and its plumbing
> seemed to conflate the idea of the "simple" auth credential type with our
> on-disk, plaintext-most-of-the-time cache. Anyway, I now feel like I have a
> pretty good idea what the function does and claims to do ... which gives me
> grounds to make some complaints. :-)
>
> IMO, the function should allow cleanup even when no-auth-cache is set in
> the
> auth parameter hash, but doesn't. I think that's because (as far as I can
> tell) Bert (who wrote the function) thought that flag meant "pretend like
> there is no auth cache", when it instead means merely "don't cache new
> auth
> creds". That's trivially fixable. (My justification is that it's probably
> precisely those folks who've since set no-auth-cache that might want to go
> back and purge data Subversion has previously cached.)
>
> Also, if I'm reading it correctly, it will allow orphaned information to
> remain in third-party keyring providers while still blowing away the
> metadata which gives that information its context within Subversion. We use
> our plaintext disk cache to hold most of the information -- everything but
> the most private bits -- even when a third-party provider (Gnome Keyring,
> etc.) is in use. As far as I can tell, this function will consider creds
> stored in this hybrid manner the same as it would those stored entirely in
> the plaintext file, and won't inform the cleanup callback function about
> this detail.

I don't think we really have a hybrid story for keyring, gnome, kde. We store everything in there (and don't create a file with pointers). It is just Windows where we use the two systems in one.

Keyring, gnome and kde have alternate ways to delete cached credentials (as has the Windows crypto store; but we only use the crypto api, not the store)

> Personally, I'd like to remove this API before we ship 1.8 and rethink it in
> light of the work that I plan to revisit post-1.8 with the master passphrase
> feature. I'm certainly willing to do that (or some other more incrementally
> improving) work. But I'm not keen on simply jettisoning someone else's
> code, so I'm seeking feedback here.

I used the current pattern of iterating and allowing to delete from the callback to allow incremental improvements in future versions. This moves some work to the client that uses this. (Common pattern would be iterate to use list and then iterate again to delete items). I would say the current api is easier to extend than the 1.0-1.7 'api' of just blowing away the directory, which would also leave keyring, gnone and kde hanging around.

        Bert
Received on 2013-04-03 21:43:41 CEST

This is an archived mail posted to the Subversion Dev mailing list.