I've added a test in r1594223 for a problem where the svn client fails
to delete a locked path.
The problem can be fixed by merging subversion/trunk:r1553501,1553556,1559197
with --accept=theirs-conflict (+ adding a missing svn_error_t *err; declaration).
Since I wasn't involved in these changes, can someone (ideally, Bert) please
check if the fixes can be backported like I've done below?
This patch includes the new test which will now XPASS over HTTP on 1.8.x.
Thanks.
Patch against 1.8.x:
Index: subversion/libsvn_ra_serf/commit.c
===================================================================
--- subversion/libsvn_ra_serf/commit.c (revision 1594229)
+++ subversion/libsvn_ra_serf/commit.c (working copy)
@@ -99,14 +99,11 @@ typedef struct proppatch_context_t {
} proppatch_context_t;
typedef struct delete_context_t {
- const char *path;
+ const char *relpath;
svn_revnum_t revision;
- const char *lock_token;
- apr_hash_t *lock_token_hash;
- svn_boolean_t keep_locks;
-
+ commit_context_t *commit;
} delete_context_t;
/* Represents a directory. */
@@ -149,7 +146,6 @@ typedef struct dir_context_t {
/* The checked-out working resource for this directory. May be NULL; if so
call checkout_dir() first. */
const char *working_url;
-
} dir_context_t;
/* Represents a file to be committed. */
@@ -1078,6 +1074,96 @@ setup_copy_file_headers(serf_bucket_t *headers,
}
static svn_error_t *
+setup_if_header_recursive(svn_boolean_t *added,
+ serf_bucket_t *headers,
+ commit_context_t *commit_ctx,
+ const char *rq_relpath,
+ apr_pool_t *pool)
+{
+ svn_stringbuf_t *sb = NULL;
+ apr_hash_index_t *hi;
+ apr_pool_t *iterpool = NULL;
+
+ if (!commit_ctx->lock_tokens)
+ {
+ *added = FALSE;
+ return SVN_NO_ERROR;
+ }
+
+ /* We try to create a directory, so within the Subversion world that
+ would imply that there is nothing here, but mod_dav_svn still sees
+ locks on the old nodes here as in DAV it is perfectly legal to lock
+ something that is not there...
+
+ Let's make mod_dav, mod_dav_svn and the DAV RFC happy by providing
+ the locks we know of with the request */
+
+ for (hi = apr_hash_first(pool, commit_ctx->lock_tokens);
+ hi;
+ hi = apr_hash_next(hi))
+ {
+ const char *relpath = svn__apr_hash_index_key(hi);
+ apr_uri_t uri;
+
+ if (!svn_relpath_skip_ancestor(rq_relpath, relpath))
+ continue;
+ else if (svn_hash_gets(commit_ctx->deleted_entries, relpath))
+ {
+ /* When a path is already explicit deleted then its lock
+ will be removed by mod_dav. But mod_dav doesn't remove
+ locks on descendants */
+ continue;
+ }
+
+ if (!iterpool)
+ iterpool = svn_pool_create(pool);
+ else
+ svn_pool_clear(iterpool);
+
+ if (sb == NULL)
+ sb = svn_stringbuf_create("", pool);
+ else
+ svn_stringbuf_appendbyte(sb, ' ');
+
+ uri = commit_ctx->session->session_url;
+ uri.path = (char *)svn_path_url_add_component2(uri.path, relpath,
+ iterpool);
+
+ svn_stringbuf_appendbyte(sb, '<');
+ svn_stringbuf_appendcstr(sb, apr_uri_unparse(iterpool, &uri, 0));
+ svn_stringbuf_appendcstr(sb, "> (<");
+ svn_stringbuf_appendcstr(sb, svn__apr_hash_index_val(hi));
+ svn_stringbuf_appendcstr(sb, ">)");
+ }
+
+ if (iterpool)
+ svn_pool_destroy(iterpool);
+
+ if (sb)
+ {
+ serf_bucket_headers_set(headers, "If", sb->data);
+ *added = TRUE;
+ }
+ else
+ *added = FALSE;
+
+ return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+setup_add_dir_common_headers(serf_bucket_t *headers,
+ void *baton,
+ apr_pool_t *pool)
+{
+ dir_context_t *dir = baton;
+ svn_boolean_t added;
+
+ return svn_error_trace(
+ setup_if_header_recursive(&added, headers, dir->commit, dir->relpath,
+ pool));
+}
+
+static svn_error_t *
setup_copy_dir_headers(serf_bucket_t *headers,
void *baton,
apr_pool_t *pool)
@@ -1109,7 +1195,7 @@ setup_copy_dir_headers(serf_bucket_t *headers,
/* Implicitly checkout this dir now. */
dir->working_url = apr_pstrdup(dir->pool, uri.path);
- return SVN_NO_ERROR;
+ return svn_error_trace(setup_add_dir_common_headers(headers, baton, pool));
}
static svn_error_t *
@@ -1117,54 +1203,22 @@ setup_delete_headers(serf_bucket_t *headers,
void *baton,
apr_pool_t *pool)
{
- delete_context_t *ctx = baton;
+ delete_context_t *del = baton;
+ svn_boolean_t added;
serf_bucket_headers_set(headers, SVN_DAV_VERSION_NAME_HEADER,
- apr_ltoa(pool, ctx->revision));
+ apr_ltoa(pool, del->revision));
- if (ctx->lock_token_hash)
- {
- ctx->lock_token = svn_hash_gets(ctx->lock_token_hash, ctx->path);
+ SVN_ERR(setup_if_header_recursive(&added, headers, del->commit,
+ del->relpath, pool));
- if (ctx->lock_token)
- {
- const char *token_header;
+ if (added && del->commit->keep_locks)
+ serf_bucket_headers_setn(headers, SVN_DAV_OPTIONS_HEADER,
+ SVN_DAV_OPTION_KEEP_LOCKS);
- token_header = apr_pstrcat(pool, "<", ctx->path, "> (<",
- ctx->lock_token, ">)", (char *)NULL);
-
- serf_bucket_headers_set(headers, "If", token_header);
-
- if (ctx->keep_locks)
- serf_bucket_headers_setn(headers, SVN_DAV_OPTIONS_HEADER,
- SVN_DAV_OPTION_KEEP_LOCKS);
- }
- }
-
return SVN_NO_ERROR;
}
-/* Implements svn_ra_serf__request_body_delegate_t */
-static svn_error_t *
-create_delete_body(serf_bucket_t **body_bkt,
- void *baton,
- serf_bucket_alloc_t *alloc,
- apr_pool_t *pool)
-{
- delete_context_t *ctx = baton;
- serf_bucket_t *body;
-
- body = serf_bucket_aggregate_create(alloc);
-
- svn_ra_serf__add_xml_header_buckets(body, alloc);
-
- svn_ra_serf__merge_lock_token_list(ctx->lock_token_hash, ctx->path,
- body, alloc, pool);
-
- *body_bkt = body;
- return SVN_NO_ERROR;
-}
-
/* Helper function to write the svndiff stream to temporary file. */
static svn_error_t *
svndiff_stream_write(void *file_baton,
@@ -1541,7 +1595,6 @@ delete_entry(const char *path,
delete_context_t *delete_ctx;
svn_ra_serf__handler_t *handler;
const char *delete_target;
- svn_error_t *err;
if (USING_HTTPV2_COMMIT_SUPPORT(dir->commit))
{
@@ -1560,10 +1613,9 @@ delete_entry(const char *path,
/* DELETE our entry */
delete_ctx = apr_pcalloc(pool, sizeof(*delete_ctx));
- delete_ctx->path = apr_pstrdup(pool, path);
+ delete_ctx->relpath = apr_pstrdup(pool, path);
delete_ctx->revision = revision;
- delete_ctx->lock_token_hash = dir->commit->lock_tokens;
- delete_ctx->keep_locks = dir->commit->keep_locks;
+ delete_ctx->commit = dir->commit;
handler = apr_pcalloc(pool, sizeof(*handler));
handler->handler_pool = pool;
@@ -1579,31 +1631,8 @@ delete_entry(const char *path,
handler->method = "DELETE";
handler->path = delete_target;
- err = svn_ra_serf__context_run_one(handler, pool);
+ SVN_ERR(svn_ra_serf__context_run_one(handler, pool));
- if (err &&
- (err->apr_err == SVN_ERR_FS_BAD_LOCK_TOKEN ||
- err->apr_err == SVN_ERR_FS_NO_LOCK_TOKEN ||
- err->apr_err == SVN_ERR_FS_LOCK_OWNER_MISMATCH ||
- err->apr_err == SVN_ERR_FS_PATH_ALREADY_LOCKED))
- {
- svn_error_clear(err);
-
- /* An error has been registered on the connection. Reset the thing
- so that we can use it again. */
- serf_connection_reset(handler->conn->conn);
-
- handler->body_delegate = create_delete_body;
- handler->body_delegate_baton = delete_ctx;
- handler->body_type = "text/xml";
-
- SVN_ERR(svn_ra_serf__context_run_one(handler, pool));
- }
- else if (err)
- {
- return err;
- }
-
/* 204 No Content: item successfully deleted */
if (handler->sline.code != 204)
{
@@ -1673,6 +1702,9 @@ add_directory(const char *path,
{
handler->method = "MKCOL";
handler->path = mkcol_target;
+
+ handler->header_delegate = setup_add_dir_common_headers;
+ handler->header_delegate_baton = dir;
}
else
{
@@ -2341,7 +2373,8 @@ svn_ra_serf__get_commit_editor(svn_ra_session_t *r
ctx->callback = callback;
ctx->callback_baton = callback_baton;
- ctx->lock_tokens = lock_tokens;
+ ctx->lock_tokens = (lock_tokens && apr_hash_count(lock_tokens))
+ ? lock_tokens : NULL;
ctx->keep_locks = keep_locks;
ctx->deleted_entries = apr_hash_make(ctx->pool);
Index: subversion/tests/cmdline/lock_tests.py
===================================================================
--- subversion/tests/cmdline/lock_tests.py (revision 1594229)
+++ subversion/tests/cmdline/lock_tests.py (working copy)
@@ -1521,7 +1521,6 @@ def verify_path_escaping(sbox):
#----------------------------------------------------------------------
# Issue #3674: Replace + propset of locked file fails over DAV
-_at_XFail(svntest.main.is_ra_type_dav)
@Issue(3674)
def replace_and_propset_locked_path(sbox):
"test replace + propset of locked file"
@@ -1554,11 +1553,9 @@ def replace_and_propset_locked_path(sbox):
# Replace A/D/G and A/D/G/rho, propset on A/D/G/rho.
svntest.actions.run_and_verify_svn(None, None, [],
'rm', G_path)
- # Recreate path for single-db
- if not os.path.exists(G_path):
- os.mkdir(G_path)
+
svntest.actions.run_and_verify_svn(None, None, [],
- 'add', G_path)
+ 'mkdir', G_path)
svntest.main.file_append(rho_path, "This is the new file 'rho'.\n")
svntest.actions.run_and_verify_svn(None, None, [],
'add', rho_path)
@@ -2031,6 +2028,28 @@ def dav_lock_refresh(sbox):
if r.status != httplib.OK:
raise svntest.Failure('Lock refresh failed: %d %s' % (r.status, r.reason))
+@XFail()
+@SkipUnless(svntest.main.is_ra_type_dav)
+def delete_locked_file_with_percent(sbox):
+ "lock and delete a file called 'a %( ) .txt'"
+
+ sbox.build()
+
+ locked_filename = 'a %( ) .txt'
+ locked_path = sbox.ospath(locked_filename)
+ svntest.main.file_write(locked_path, "content\n")
+ sbox.simple_add(locked_filename)
+ sbox.simple_commit()
+
+ sbox.simple_lock(locked_filename)
+ sbox.simple_rm(locked_filename)
+
+ # XFAIL: With a 1.8.x client, this commit fails with:
+ # svn: E175002: Unexpected HTTP status 400 'Bad Request' on '/svn-test-work/repositories/lock_tests-52/!svn/txr/2-2/a%20%25(%20)%20.txt'
+ # and the following error in the httpd error log:
+ # Invalid percent encoded URI in tagged If-header [400, #104]
+ sbox.simple_commit()
+
########################################################################
# Run the tests
@@ -2087,6 +2106,7 @@ test_list = [ None,
dav_lock_timeout,
non_root_locks,
dav_lock_refresh,
+ delete_locked_file_with_percent,
]
if __name__ == '__main__':
Index: .
===================================================================
--- . (revision 1594229)
+++ . (working copy)
Property changes on: .
___________________________________________________________________
Modified: svn:mergeinfo
Merged /subversion/trunk:r1553501,1553556,1559197,1594223
Merged /subversion/branches/1.8.x-r1594223:r1594224-1594233
Received on 2014-05-13 17:23:13 CEST