Hi Prabhu,
My review comment about your patch.
I guess you test the following case sensitiveness,
* section name case sensitivity
* path only case sensitivity
* case sensitivity for read
* case sensitivity for write.
If I represent it as Matrix,
Section Path
read case1 case3
write case2 case4
You have the above covered in your patch partially.
Your test for read is not correct, I mean you are not testing the expectation by tweaking the permissions.
I mean
You have {"/": "jrandom = r", "/A": "jrandom = r", "/a/Mu": "jrandom = rw"} which grants read access to /A/mu.
Already this case works in HEAD and continue to work after the real case sensitivity fix. So make it something like this.
rule = {"/": "jrandom = r", "/A/mu": "jrandom = ", "/a/Mu": "jrandom = rw"} and try 'read' on /A/mu which should fail with case sensitivity fix but succeed in HEAD.
Name your testcase with what it intends to test. i.e case_insensitive_authz makes me get confused.
May be you can name it case_sensitive_authz
Thanks
With regards
Kamesh Jayachandran
-----Original Message-----
From: Prabhu Gnana Sundar [mailto:prabhugs_at_collab.net]
Sent: Fri 2/4/2011 1:04 PM
To: Daniel Shahaf
Cc: dev_at_subversion.apache.org
Subject: Re: [PATCH] New XFail test for the issue 3781
Hi Daniel,
Thank you for the comments... :)
On Friday 04 February 2011 08:12 AM, Daniel Shahaf wrote:
> Prabhu Gnana Sundar wrote on Thu, Feb 03, 2011 at 16:53:54 +0530:
>> Hi all,
>>
>> Currently, as per the issue 3781, "checkout" reads the authz file in
>> *case insensitive* way, whereas "commit" reads the authz file in *case
>> sensitive* way.
>>
>> Here is what is observed:
>> 1. Checkout is *case insensitive* about the repository name section and
>> also the path-inside-repo section.
>> 2. Commit is *case sensitive* about the repository name section *but
>> case insensitive* about the path-inside-repo section.
>>
>> Attached an XFail testcase and the log message with this mail. Please
>> share your views.
>>
>>
> You didn't actually say what the intended behaviour is.
>
The intended behaviour is path based authorization should be case
sensitive for all cases.
But it shows some inconsistencies currently...
It has been discussed in ,
http://mail-archives.apache.org/mod_mbox/subversion-dev/201101.mbox/%3C4D468561.3070807@collab.net%3E
>> Index: subversion/tests/cmdline/authz_tests.py
>> ===================================================================
>> --- subversion/tests/cmdline/authz_tests.py (revision 1066732)
>> +++ subversion/tests/cmdline/authz_tests.py (working copy)
>> @@ -1061,6 +1061,67 @@
>> [], 'ls', '-R',
>> sbox.repo_url)
>>
>> +
>> +def case_insensitive_authz(sbox):
>> + "authz issue #3781, check case insensitiveness"
>> +
>> + sbox.build()
>> +
>> + # test the case-insensitivity of the path inside the repo
>> + write_authz_file(sbox, {"/": "jrandom = r", "/A": "jrandom = r", "/a/Mu": "jrandom = rw"})
>> +
>> + write_restrictive_svnserve_conf(sbox.repo_dir)
>> +
>> + wc_dir = sbox.wc_dir
>> +
>> + mu_path = os.path.join(wc_dir, 'A', 'mu')
>> + mu_url = sbox.repo_url + '/A/mu'
>> + svntest.main.file_append(mu_path, "hi")
>> +
>> + # Create expected output tree.
>> + expected_output = svntest.wc.State(wc_dir, {
>> + 'A/mu' : Item(verb='Sending'),
>> + })
>> +
>> + # Commit the file.
>> + svntest.actions.run_and_verify_commit(wc_dir,
>> + expected_output,
>> + None,
>> + None,
>> + mu_path)
> Hold on. Why does this work? You commit /A/mu and the authz file gives
> you only 'r' access to /A!
>
It works because we have set 'rw' for the /a/Mu file.
And this must fail actually, because of the case-change in the path.
Tweaked the test-case.
>> + mixed_case_repo_dir = mixcases(os.path.basename(sbox.repo_dir))
>> +
>> + # test the case-insensitivity of the repo name
>> + write_authz_file(sbox, {}, sections = {mixed_case_repo_dir + ":/": "jrandom = r",
>> + mixed_case_repo_dir + ":/A": "jrandom = r",
>> + mixed_case_repo_dir + ":/A/mu": "jrandom = rw"})
>> +
> Just for clarity, could you add sections for the correct-case repository
> name to the authz file?
>
Sure :) added for clarity...
>> + svntest.main.file_append(mu_path, "hi")
>> + # Commit the file again.
>> + svntest.actions.run_and_verify_commit(wc_dir,
>> + expected_output,
>> + None,
>> + None,
>> + mu_path)
>> + svntest.actions.run_and_verify_svn2('No error',
>> + svntest.verify.AnyOutput, [],
>> + 0, 'cat', mu_url)
> Could you have here both a commit that fails and a commit that succeeds?
Added :)
I have attached the tweaked correct patch and the log message with this
mail. Please share your views.
Thanks and regards
Prabhu
Received on 2011-02-07 13:23:19 CET