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

RE: svn commit: r1443556 - in /subversion/trunk/subversion: include/svn_auth.h libsvn_subr/simple_providers.c

From: Bert Huijben <bert_at_qqmail.nl>
Date: Thu, 7 Feb 2013 17:16:37 +0100

> -----Original Message-----
> From: Stefan Sperling [mailto:stsp_at_elego.de]
> Sent: donderdag 7 februari 2013 16:44
> To: dev_at_subversion.apache.org
> Subject: Re: svn commit: r1443556 - in /subversion/trunk/subversion:
> include/svn_auth.h libsvn_subr/simple_providers.c
>
> On Thu, Feb 07, 2013 at 03:31:47PM -0000, rhuijben_at_apache.org wrote:
> > Author: rhuijben
> > Date: Thu Feb 7 15:31:46 2013
> > New Revision: 1443556
> >
> > URL: http://svn.apache.org/viewvc?rev=1443556&view=rev
> > Log:
> > Fix a simple to trigger, but never reported segfault in the auth helper
code.
> >
> > * subversion/include/svn_auth.h
> > (SVN_AUTH_PARAM_STORE_PLAINTEXT_PASSWORDS,
> > SVN_AUTH_PARAM_DONT_STORE_SSL_CLIENT_CERT_PP,
> > SVN_AUTH_PARAM_STORE_SSL_CLIENT_CERT_PP_PLAINTEXT): Mark as
> new in 1.6.
> >
> > * subversion/libsvn_subr/simple_providers.c
> > (svn_auth__simple_creds_cache_set): If nobody has set
> > SVN_AUTH_PARAM_STORE_PLAINTEXT_PASSWORDS (introduced in
> 1.6),
> > we shouldn't segfault.
>
> I agree that we shouldn't segfault. But it seems you've changed
> the default behaviour with this commit, from 'ask' to 'yes',
> in case the option isn't specified. See below.
>
> > @@ -372,8 +372,9 @@ svn_auth__simple_creds_cache_set(svn_boo
> > simple_provider_baton_t *b =
> > (simple_provider_baton_t *)provider_baton;
> >
> > - if (svn_cstring_casecmp(store_plaintext_passwords,
> > - SVN_CONFIG_ASK) == 0)
> > + if (store_plaintext_passwords
> > + && svn_cstring_casecmp(store_plaintext_passwords,
> > + SVN_CONFIG_ASK) == 0)
>
> I think this should be:
>
> if (!store_plaintext_passwords
> || svn_cstring_casecmp(store_plaintext_passwords,
> SVN_CONFIG_ASK) == 0)
>
> to make 'ask' the default catch-all case.

Which makes things much harder to verify, and will eventually fall back to
allowing it anyway, because with the pre-1.6 api there is no way to 'ask'.

All our initializers set this value one way or another, so I didn't change a
default. Or every svn invocation would have crashed since your change.
(See r1443562 for a reproduction recipe)

        Bert
Received on 2013-02-07 17:17:15 CET

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.