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

Re: [PATCH] Fix issue #2242.

From: Senthil Kumaran S <senthil_at_collab.net>
Date: Wed, 02 Jul 2008 02:38:22 +0530

Daniel Shahaf wrote:
>>>> Tries to checkout onto existing working copy. It happens to work, but
>>>> I'm not sure that we guarantee that 'checkout' will behave as 'update'
>>>> if its target already exists. In which case, we need s/checkout/update/
>>>> here.
>>> Yes checkout works like update here. It says checked out revision 'n'.
>
> I know that it "works". I question whether its working here is
> a feature that we can rely on, or an incident of implementation that may
> be declared a bug and removed. (Briefly: does our API guarantee that
> 'svn co URL dir; svn co URL dir;' will work as it currently does?)

I don't see a solid statement which says 'it will work', but I feel it does so
from the past releases.

>>>>> + exit_code, output, errput = svntest.main.run_command(
>>>>> + svntest.main.svn_binary, None, 1, 'co', sbox.repo_url, wc_dir,
>>>>> + '--username', 'jrandom', '--non-interactive')
>>>>> +
>>>> We need --config-dir here or we would be affected by the user's
>>>> ~/.subversion/, right? (where, possibly, the creds from a previous run of
>>>> the test are cached, etc.)
>>> Yes exactly, that is the intention of using '--non-interactive' where we intend
>>> to use ~/.subversion
>
> I'm sorry, but I don't follow. I think --config-dir should be added to
> all the run_command calls here; I don't understand how --non-interactive
> or its absence come into play.

Sorry for this entire confusion, I tested this patch with my custom
/home/stylesen/.subversion in home directory which had
'store-plaintext-passwords = yes', which gave me false alarms. After careful
examination I found all these commands do need '--config-dir' (also, I messed
up with run_svn, which has a default config_dir and run_command which does
not). Updated patch has this.

>>>> if (svn_cstring_casecmp(store_plaintext_passwords,
>>>> SVN_CONFIG_ASK) == 0)
>>>> {
>>>> if (non_interactive)
>>>> /* In non-interactive mode, the default behaviour is
>>>> * to not store the password, because it is usually
>>>> * passed on the command line. */
>>>> may_save_password = FALSE;
>>> Yes the above code holds true, where I register the new password for this new
>>> user in the auth cache for the following checkouts.
>>>
>
> Assuming I understood you correctly (I think what you just said means
> "Yes" ;)), I would prefer to see --non-interactive passed and
> store-plaintext-passwords=True set in the config directory. (Are you
> running with keyring or kwallet? I'm surprising the above run_command()
> works without --non-interactive.)

I ve made the patch more clear. I think this updated patch solves all your
concerns.

Thank You.

-- 
Senthil Kumaran S
http://www.stylesen.org/

[[[
Test case for issue #2242 - auth cache picking up password from wrong
username entry.

* subversion/tests/cmdline/basic_tests.py
  (basic_auth_test): New test.
  (test_list): Add above test.

Patch by: stylesen
]]]

Index: subversion/tests/cmdline/basic_tests.py
===================================================================
--- subversion/tests/cmdline/basic_tests.py (revision 31957)
+++ subversion/tests/cmdline/basic_tests.py (working copy)
@@ -2355,6 +2355,48 @@
                                 expected_output, [], 'ls', '-r3',
                                 '^//A/@3', iota_url)
 
+
+# Issue 2242, auth cache picking up password from wrong username entry
+def basic_auth_test(sbox):
+ "basic auth test"
+
+ sbox.build(read_only = True)
+ wc_dir = sbox.wc_dir
+
+ # Set up a custom config directory
+ tmp_dir = os.path.abspath(svntest.main.temp_dir)
+ config_dir = os.path.join(tmp_dir, 'auth-test-config')
+ svntest.main.create_config_dir(config_dir, None)
+
+ # Checkout with jrandom
+ exit_code, output, errput = svntest.main.run_command(
+ svntest.main.svn_binary, None, 1, 'co', sbox.repo_url, wc_dir,
+ '--username', 'jrandom', '--password', 'rayjandom',
+ '--config-dir', config_dir)
+
+ exit_code, output, errput = svntest.main.run_command(
+ svntest.main.svn_binary, None, 1, 'co', sbox.repo_url, wc_dir,
+ '--username', 'jrandom', '--non-interactive', '--config-dir', config_dir)
+
+ # Checkout with jconstant
+ exit_code, output, errput = svntest.main.run_command(
+ svntest.main.svn_binary, None, 1, 'co', sbox.repo_url, wc_dir,
+ '--username', 'jconstant', '--password', 'rayjandom',
+ '--config-dir', config_dir)
+
+ exit_code, output, errput = svntest.main.run_command(
+ svntest.main.svn_binary, None, 1, 'co', sbox.repo_url, wc_dir,
+ '--username', 'jconstant', '--non-interactive',
+ '--config-dir', config_dir)
+
+ # Checkout with jrandom which should fail since we do not provide
+ # a password and the above cached password belongs to jconstant
+ expected_err = ["authorization failed: Could not authenticate to server:"]
+ exit_code, output, errput = svntest.main.run_command(
+ svntest.main.svn_binary, expected_err, 1, 'co', sbox.repo_url, wc_dir,
+ '--username', 'jrandom', '--non-interactive', '--config-dir', config_dir)
+
+
 #----------------------------------------------------------------------
 
 ########################################################################
@@ -2407,6 +2449,7 @@
               basic_relative_url_multi_repo,
               basic_relative_url_non_canonical,
               basic_relative_url_with_peg_revisions,
+ basic_auth_test,
              ]
 
 if __name__ == '__main__':

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-07-01 23:08:58 CEST

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.