On Thu, Feb 07, 2013 at 03:31:47PM -0000, rhuij...@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.
> {
> if (non_interactive)
> /* In non-interactive mode, the default behaviour is
> @@ -438,13 +439,15 @@ svn_auth__simple_creds_cache_set(svn_boo
> may_save_password = TRUE;
> }
> }
> - else if (svn_cstring_casecmp(store_plaintext_passwords,
> - SVN_CONFIG_FALSE) == 0)
> + else if (store_plaintext_passwords
> + && svn_cstring_casecmp(store_plaintext_passwords,
> + SVN_CONFIG_FALSE) == 0)
> {
> may_save_password = FALSE;
> }
> - else if (svn_cstring_casecmp(store_plaintext_passwords,
> - SVN_CONFIG_TRUE) == 0)
> + else if (!store_plaintext_passwords
> + || svn_cstring_casecmp(store_plaintext_passwords,
> + SVN_CONFIG_TRUE) == 0)
And this should be:
else if (store_plaintext_passwords
&& svn_cstring_casecmp(store_plaintext_passwords,
SVN_CONFIG_TRUE) == 0)
So that the password is saved only if the user put 'yes' in the config file.
> {
> may_save_password = TRUE;
> }
>
