You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2013/12/27 18:29:46 UTC
svn commit: r1553701 - in /subversion/trunk/subversion: mod_dav_svn/lock.c
tests/cmdline/lock_tests.py
Author: rhuijben
Date: Fri Dec 27 17:29:46 2013
New Revision: 1553701
URL: http://svn.apache.org/r1553701
Log:
Make mod_dav_svn properly report 'svn unlock' errors by producing direct output
instead of waiting for mod_dav to forget handling any error messages and just
reporting a server error on any error.
* subversion/mod_dav_svn/lock.c
(remove_lock): Return NULL instead of 0.
(remove_lock_svn_output): New function, wrapping remove_lock.
(dav_svn__hooks_locks): Use remove_lock_svn_output.
* subversion/tests/cmdline/lock_tests.py
(block_unlock_if_pre_unlock_hook_fails): Require explicit output instead of
accepting a HTTP error.
(lock_hook_messages): New function.
(test_list): Add lock_hook_messages.
Modified:
subversion/trunk/subversion/mod_dav_svn/lock.c
subversion/trunk/subversion/tests/cmdline/lock_tests.py
Modified: subversion/trunk/subversion/mod_dav_svn/lock.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/lock.c?rev=1553701&r1=1553700&r2=1553701&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/lock.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/lock.c Fri Dec 27 17:29:46 2013
@@ -841,14 +841,14 @@ remove_lock(dav_lockdb *lockdb,
/* Sanity check: if the resource has no associated path in the fs,
then there's nothing to do. */
if (! resource->info->repos_path)
- return 0;
+ return NULL;
/* Another easy out: if an svn client sent a 'keep_locks' header
(typically in a DELETE request, as part of 'svn commit
--no-unlock'), then ignore dav_method_delete()'s attempt to
unconditionally remove the lock. */
if (info->keep_locks)
- return 0;
+ return NULL;
/* If the resource's fs path is unreadable, we don't allow a lock to
be removed from it. */
@@ -908,10 +908,40 @@ remove_lock(dav_lockdb *lockdb,
resource->info->r->pool));
}
- return 0;
+ return NULL;
+}
+
+static dav_error *
+remove_lock_svn_output(dav_lockdb *lockdb,
+ const dav_resource *resource,
+ const dav_locktoken *locktoken)
+{
+ dav_error *derr = remove_lock(lockdb, resource, locktoken);
+ int status;
+
+ if (!derr
+ || !resource->info->repos->is_svn_client
+ || (strcmp(lockdb->info->r->method, "UNLOCK") != 0))
+ return derr;
+
+ /* Ok, we have a nice error chain but mod_dav doesn't offer us a way to
+ present it to the client as it will only use the status code for
+ generating a standard error...
+
+ Luckily the unlock processing for the "UNLOCK" method is very simple:
+ call this function and return the result.
+
+ That allows us to just force a response and tell httpd that we are done */
+ status = dav_svn__error_response_tag(lockdb->info->r, derr);
+
+ /* status = DONE */
+
+ /* And push an error that will make mod_dav just report that it is done */
+ return dav_push_error(resource->pool, status, derr->error_id, NULL, derr);
}
+
/*
** Refresh all locks, found on the specified resource, which has a
** locktoken in the provided list.
@@ -1017,7 +1047,7 @@ const dav_hooks_locks dav_svn__hooks_loc
find_lock,
has_locks,
append_locks,
- remove_lock,
+ remove_lock_svn_output,
refresh_locks,
NULL,
NULL /* hook structure context */
Modified: subversion/trunk/subversion/tests/cmdline/lock_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/lock_tests.py?rev=1553701&r1=1553700&r2=1553701&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/lock_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/lock_tests.py Fri Dec 27 17:29:46 2013
@@ -1681,7 +1681,7 @@ def block_unlock_if_pre_unlock_hook_fail
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"
+ expected_unlock_fail_err_re = ".*error text"
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)
@@ -1916,6 +1916,48 @@ def copy_with_lock(sbox):
None,
wc_dir)
+def lock_hook_messages(sbox):
+ "verify (un)lock message is transferred correctly"
+
+ sbox.build(create_wc = False)
+ repo_dir = sbox.repo_dir
+
+ iota_url = sbox.repo_url + "/iota"
+ mu_url = sbox.repo_url + "/A/mu"
+
+ svntest.actions.run_and_verify_svn(None, ".*locked by user", [], 'lock',
+ iota_url)
+
+ error_msg = "Text with <angle brackets> & ampersand"
+ svntest.actions.create_failing_hook(repo_dir, "pre-lock", error_msg)
+ svntest.actions.create_failing_hook(repo_dir, "pre-unlock", error_msg)
+
+ _, _, actual_stderr = svntest.actions.run_and_verify_svn(
+ None, [], svntest.verify.AnyOutput,
+ 'lock', mu_url)
+ if len(actual_stderr) > 2:
+ actual_stderr = actual_stderr[-2:]
+ expected_err = [
+ 'svn: E165001: ' + svntest.actions.hook_failure_message('pre-lock'),
+ error_msg + "\n",
+ ]
+ svntest.verify.compare_and_display_lines(None, 'STDERR',
+ expected_err, actual_stderr)
+
+
+ _, _, actual_stderr = svntest.actions.run_and_verify_svn(
+ None, [], svntest.verify.AnyOutput,
+ 'unlock', iota_url)
+ if len(actual_stderr) > 2:
+ actual_stderr = actual_stderr[-2:]
+ expected_err = [
+ 'svn: E165001: ' + svntest.actions.hook_failure_message('pre-unlock'),
+ error_msg + "\n",
+ ]
+ svntest.verify.compare_and_display_lines(None, 'STDERR',
+ expected_err, actual_stderr)
+
+
########################################################################
# Run the tests
@@ -1970,6 +2012,7 @@ test_list = [ None,
commit_stolen_lock,
drop_locks_on_parent_deletion,
copy_with_lock,
+ lock_hook_messages,
]
if __name__ == '__main__':