[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: Stefan Sperling <stsp_at_elego.de>
Date: Thu, 7 Feb 2013 16:43:56 +0100

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.

> {
> 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;
> }
>
Received on 2013-02-07 16:44:36 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.