On Thursday 02 June 2011 08:36 PM, Daniel Shahaf wrote:
+#----------------------------------------------------------------------
+def block_unlock_if_pre_unlock_hook_fails(sbox):
+ "block unlock operation if pre-unlock hook fails"
+
+ sbox.build()
+ wc_dir = sbox.wc_dir
+ repo_dir = sbox.repo_dir
+
+ svntest.actions.create_failing_hook(repo_dir, "pre-unlock", "")
+
Could you pass a non-"" here and then test that it appears in the error message?
If pre-unlock hook fails, the error message is not marshalled back to
the client, instead "500 Internal Server Error" is displayed to the
user. This is the issue I mentioned here
http://svn.haxx.se/dev/archive-2011-06/0025.shtml
Original issue I filed for this one in Apache's mod_dav:
https://issues.apache.org/bugzilla/show_bug.cgi?id=51297
I have attached a patch which tests whether the error message contains
"500 Internal Server Error"(In case of ra_serf/ra_neon) or the "error
text"(in case of ra_local).
+ # lock a file.
+ pi_path = os.path.join(wc_dir, 'A', 'D', 'G', 'pi')
+ expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
+ expected_status.tweak('A/D/G/pi', writelocked='K')
+
+ svntest.actions.run_and_verify_svn(None, ".*locked by user", [], 'lock',
+ '-m', '', pi_path)
+
+ svntest.actions.run_and_verify_status(wc_dir, expected_status)
+
+ # Now, unlock the file via ra_dav. Make sure the unlock operation fails as
The comment is wrong in mentioning ra_dav.
Updated the comment.
Thanks & Regards,
Vijayaguru
+ # pre-unlock hook blocks it.
+ svntest.actions.run_and_verify_svn2(None, None, "", 1, 'unlock', pi_path)
+ svntest.actions.run_and_verify_status(wc_dir, expected_status)
+
+
########################################################################
# Run the tests
@@ -1739,6 +1765,7 @@
replace_and_propset_locked_path,
cp_isnt_ro,
update_locked_deleted,
+ block_unlock_if_pre_unlock_hook_fails,
]
if __name__ == '__main__':
Index: subversion/libsvn_ra_neon/lock.c
===================================================================
--- subversion/libsvn_ra_neon/lock.c (revision 1130445)
+++ subversion/libsvn_ra_neon/lock.c (working copy)
@@ -399,6 +399,7 @@
const char *url;
const char *url_path;
ne_uri uri;
+ svn_error_t *err = SVN_NO_ERROR;
apr_hash_t *extra_headers = apr_hash_make(pool);
@@ -441,7 +442,6 @@
{
int code = 0;
- svn_error_t *err;
err = svn_ra_neon__simple_request(&code, ras, "UNLOCK", url_path,
extra_headers, NULL, 204, 0, pool);
@@ -460,13 +460,11 @@
_("No lock on path '%s'"
" (%d Bad Request)"), path, code);
default:
- break; /* Handle as error */
+ break;
}
}
- else
- SVN_ERR(err);
}
- return SVN_NO_ERROR;
+ return svn_error_return(err);
}
Index: subversion/tests/cmdline/lock_tests.py
===================================================================
--- subversion/tests/cmdline/lock_tests.py (revision 1130445)
+++ subversion/tests/cmdline/lock_tests.py (working copy)
@@ -1693,6 +1693,33 @@
None, expected_status)
+#----------------------------------------------------------------------
+def block_unlock_if_pre_unlock_hook_fails(sbox):
+ "block unlock operation if pre-unlock hook fails"
+
+ sbox.build()
+ wc_dir = sbox.wc_dir
+ repo_dir = sbox.repo_dir
+
+ svntest.actions.create_failing_hook(repo_dir, "pre-unlock", "error text")
+
+ # lock a file.
+ pi_path = os.path.join(wc_dir, 'A', 'D', 'G', 'pi')
+ expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
+ expected_status.tweak('A/D/G/pi', writelocked='K')
+
+ svntest.actions.run_and_verify_svn(None, ".*locked by user", [], 'lock',
+ '-m', '', pi_path)
+
+ svntest.actions.run_and_verify_status(wc_dir, expected_status)
+
+ # Make sure the unlock operation fails as pre-unlock hook blocks it.
+ expected_unlock_fail_err_re = ".*error text|.*500 Internal Server Error"
+ svntest.actions.run_and_verify_svn2(None, None, expected_unlock_fail_err_re,
+ 1, 'unlock', pi_path)
+ svntest.actions.run_and_verify_status(wc_dir, expected_status)
+
+
########################################################################
# Run the tests
@@ -1739,6 +1766,7 @@
replace_and_propset_locked_path,
cp_isnt_ro,
update_locked_deleted,
+ block_unlock_if_pre_unlock_hook_fails,
]
if __name__ == '__main__':
