You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by sv...@apache.org on 2013/03/26 05:00:43 UTC
svn commit: r1460964 - in /subversion/branches/1.7.x: ./
subversion/libsvn_client/ subversion/libsvn_ra_neon/
subversion/libsvn_ra_serf/ subversion/tests/cmdline/
Author: svn-role
Date: Tue Mar 26 04:00:42 2013
New Revision: 1460964
URL: http://svn.apache.org/r1460964
Log:
Reintegrate the 1.7.x-issue4332 branch:
* ^/subversion/branches/1.7.x-issue4332
Fix issue #4332 ("neon OPTIONS request on repository root causes
authz access denied").
Justification:
This is a user-reported bug preventing what should be a
completely valid remote deletion operation when Neon is the HTTP
library in use. (As a bonus side effect, this shaves one or two
network turnarounds per deletion target off of this operation,
too!) It's a regression from 1.6.
Branch:
^/subversion/branches/1.7.x-issue4332
Votes:
+1: cmpilato, philip, breser
Modified:
subversion/branches/1.7.x/ (props changed)
subversion/branches/1.7.x/STATUS
subversion/branches/1.7.x/subversion/libsvn_client/delete.c
subversion/branches/1.7.x/subversion/libsvn_ra_neon/props.c
subversion/branches/1.7.x/subversion/libsvn_ra_serf/commit.c
subversion/branches/1.7.x/subversion/libsvn_ra_serf/options.c
subversion/branches/1.7.x/subversion/libsvn_ra_serf/ra_serf.h
subversion/branches/1.7.x/subversion/tests/cmdline/authz_tests.py
Propchange: subversion/branches/1.7.x/
------------------------------------------------------------------------------
Merged /subversion/branches/1.7.x-issue4332:r1453478-1460963
Merged /subversion/trunk:r1452967,1454088,1454217
Modified: subversion/branches/1.7.x/STATUS
URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/STATUS?rev=1460964&r1=1460963&r2=1460964&view=diff
==============================================================================
--- subversion/branches/1.7.x/STATUS (original)
+++ subversion/branches/1.7.x/STATUS Tue Mar 26 04:00:42 2013
@@ -299,17 +299,3 @@ Veto-blocked changes:
Approved changes:
=================
- * ^/subversion/branches/1.7.x-issue4332
- Fix issue #4332 ("neon OPTIONS request on repository root causes
- authz access denied").
- Justification:
- This is a user-reported bug preventing what should be a
- completely valid remote deletion operation when Neon is the HTTP
- library in use. (As a bonus side effect, this shaves one or two
- network turnarounds per deletion target off of this operation,
- too!) It's a regression from 1.6.
- Branch:
- ^/subversion/branches/1.7.x-issue4332
- Votes:
- +1: cmpilato, philip, breser
-
Modified: subversion/branches/1.7.x/subversion/libsvn_client/delete.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/subversion/libsvn_client/delete.c?rev=1460964&r1=1460963&r2=1460964&view=diff
==============================================================================
--- subversion/branches/1.7.x/subversion/libsvn_client/delete.c (original)
+++ subversion/branches/1.7.x/subversion/libsvn_client/delete.c Tue Mar 26 04:00:42 2013
@@ -210,6 +210,16 @@ single_repos_delete(svn_ra_session_t *ra
return svn_error_trace(editor->close_edit(edit_baton, pool));
}
+
+/* Structure for tracking remote delete targets associated with a
+ specific repository. */
+struct repos_deletables_t
+{
+ svn_ra_session_t *ra_session;
+ apr_array_header_t *target_uris;
+};
+
+
static svn_error_t *
delete_urls_multi_repos(const apr_array_header_t *uris,
const apr_hash_t *revprop_table,
@@ -218,91 +228,132 @@ delete_urls_multi_repos(const apr_array_
svn_client_ctx_t *ctx,
apr_pool_t *pool)
{
- apr_hash_t *sessions = apr_hash_make(pool);
- apr_hash_t *relpaths = apr_hash_make(pool);
+ apr_hash_t *deletables = apr_hash_make(pool);
+ apr_pool_t *iterpool;
apr_hash_index_t *hi;
int i;
- /* Create a hash of repos_root -> ra_session maps and repos_root -> relpaths
- maps, used to group the various targets. */
+ /* Create a hash mapping repository root URLs -> repos_deletables_t *
+ structures. */
for (i = 0; i < uris->nelts; i++)
{
const char *uri = APR_ARRAY_IDX(uris, i, const char *);
- svn_ra_session_t *ra_session = NULL;
- const char *repos_root = NULL;
- const char *repos_relpath = NULL;
- apr_array_header_t *relpaths_list;
+ struct repos_deletables_t *repos_deletables = NULL;
+ const char *repos_relpath;
svn_node_kind_t kind;
- for (hi = apr_hash_first(pool, sessions); hi; hi = apr_hash_next(hi))
+ for (hi = apr_hash_first(pool, deletables); hi; hi = apr_hash_next(hi))
{
- repos_root = svn__apr_hash_index_key(hi);
- repos_relpath = svn_uri__is_child(repos_root, uri, pool);
+ const char *repos_root = svn__apr_hash_index_key(hi);
+ repos_relpath = svn_uri__is_child(repos_root, uri, pool);
if (repos_relpath)
{
- /* Great! We've found another uri underneath this session,
- store it and move on. */
- ra_session = svn__apr_hash_index_val(hi);
- relpaths_list = apr_hash_get(relpaths, repos_root,
- APR_HASH_KEY_STRING);
-
- APR_ARRAY_PUSH(relpaths_list, const char *) = repos_relpath;
+ /* Great! We've found another URI underneath this
+ session. We'll pick out the related RA session for
+ use later, store the new target, and move on. */
+ repos_deletables = svn__apr_hash_index_val(hi);
+ APR_ARRAY_PUSH(repos_deletables->target_uris, const char *) =
+ apr_pstrdup(pool, uri);
break;
}
}
- if (!ra_session)
+ /* If we haven't created a repos_deletable structure for this
+ delete target, we need to do. That means opening up an RA
+ session and initializing its targets list. */
+ if (!repos_deletables)
{
- /* If we haven't found a session yet, we need to open one up.
- Note that we don't have a local directory, nor a place
- to put temp files. */
+ svn_ra_session_t *ra_session = NULL;
+ const char *repos_root;
+ apr_array_header_t *target_uris;
+
+ /* Open an RA session to (ultimately) the root of the
+ repository in which URI is found. */
SVN_ERR(svn_client__open_ra_session_internal(&ra_session, NULL, uri,
NULL, NULL, FALSE,
TRUE, ctx, pool));
SVN_ERR(svn_ra_get_repos_root2(ra_session, &repos_root, pool));
SVN_ERR(svn_ra_reparent(ra_session, repos_root, pool));
-
- apr_hash_set(sessions, repos_root, APR_HASH_KEY_STRING, ra_session);
repos_relpath = svn_uri__is_child(repos_root, uri, pool);
- relpaths_list = apr_array_make(pool, 1, sizeof(const char *));
- apr_hash_set(relpaths, repos_root, APR_HASH_KEY_STRING,
- relpaths_list);
- APR_ARRAY_PUSH(relpaths_list, const char *) = repos_relpath;
+ /* Make a new relpaths list for this repository, and add
+ this URI's relpath to it. */
+ target_uris = apr_array_make(pool, 1, sizeof(const char *));
+ APR_ARRAY_PUSH(target_uris, const char *) = apr_pstrdup(pool, uri);
+
+ /* Build our repos_deletables_t item and stash it in the
+ hash. */
+ repos_deletables = apr_pcalloc(pool, sizeof(*repos_deletables));
+ repos_deletables->ra_session = ra_session;
+ repos_deletables->target_uris = target_uris;
+ apr_hash_set(deletables, repos_root,
+ APR_HASH_KEY_STRING, repos_deletables);
}
- /* Check we identified a non-root relpath. Return an RA error
- code for 1.6 compatibility. */
+ /* If we get here, we should have been able to calculate a
+ repos_relpath for this URI. Let's make sure. (We return an
+ RA error code otherwise for 1.6 compatibility.) */
if (!repos_relpath || !*repos_relpath)
return svn_error_createf(SVN_ERR_RA_ILLEGAL_URL, NULL,
"URL '%s' not within a repository", uri);
- /* Now, test to see if the thing actually exists. */
- SVN_ERR(svn_ra_check_path(ra_session, repos_relpath, SVN_INVALID_REVNUM,
- &kind, pool));
+ /* Now, test to see if the thing actually exists in HEAD. */
+ SVN_ERR(svn_ra_check_path(repos_deletables->ra_session, repos_relpath,
+ SVN_INVALID_REVNUM, &kind, pool));
if (kind == svn_node_none)
return svn_error_createf(SVN_ERR_FS_NOT_FOUND, NULL,
"URL '%s' does not exist", uri);
}
- /* At this point, we should have two hashs:
- SESSIONS maps repos_roots to ra_sessions.
- RELPATHS maps repos_roots to a list of decoded relpaths for that root.
-
- Now we iterate over the collection of sessions and do a commit for each
- one with the collected relpaths. */
- for (hi = apr_hash_first(pool, sessions); hi; hi = apr_hash_next(hi))
+ /* Now we iterate over the DELETABLES hash, issuing a commit for
+ each repository with its associated collected targets. */
+ iterpool = svn_pool_create(pool);
+ for (hi = apr_hash_first(pool, deletables); hi; hi = apr_hash_next(hi))
{
const char *repos_root = svn__apr_hash_index_key(hi);
- svn_ra_session_t *ra_session = svn__apr_hash_index_val(hi);
- const apr_array_header_t *relpaths_list =
- apr_hash_get(relpaths, repos_root, APR_HASH_KEY_STRING);
+ struct repos_deletables_t *repos_deletables = svn__apr_hash_index_val(hi);
+ const char *base_uri;
+ apr_array_header_t *target_relpaths;
+
+ svn_pool_clear(iterpool);
+
+ /* We want to anchor the commit on the longest common path
+ across the targets for this one repository. If, however, one
+ of our targets is that longest common path, we need instead
+ anchor the commit on that path's immediate parent. Because
+ we're asking svn_uri_condense_targets() to remove
+ redundancies, this situation should be detectable by their
+ being returned either a) only a single, empty-path, target
+ relpath, or b) no target relpaths at all. */
+ SVN_ERR(svn_uri_condense_targets(&base_uri, &target_relpaths,
+ repos_deletables->target_uris,
+ TRUE, iterpool, iterpool));
+ SVN_ERR_ASSERT(!svn_path_is_empty(base_uri));
+ if (target_relpaths->nelts == 0)
+ {
+ const char *target_relpath;
- SVN_ERR(single_repos_delete(ra_session, repos_root, relpaths_list,
+ svn_uri_split(&base_uri, &target_relpath, base_uri, iterpool);
+ APR_ARRAY_PUSH(target_relpaths, const char *) = target_relpath;
+ }
+ else if ((target_relpaths->nelts == 1)
+ && (svn_path_is_empty(APR_ARRAY_IDX(target_relpaths, 0,
+ const char *))))
+ {
+ const char *target_relpath;
+
+ svn_uri_split(&base_uri, &target_relpath, base_uri, iterpool);
+ APR_ARRAY_IDX(target_relpaths, 0, const char *) = target_relpath;
+ }
+
+ SVN_ERR(svn_ra_reparent(repos_deletables->ra_session, base_uri, pool));
+ SVN_ERR(single_repos_delete(repos_deletables->ra_session, repos_root,
+ target_relpaths,
revprop_table, commit_callback,
- commit_baton, ctx, pool));
+ commit_baton, ctx, iterpool));
}
+ svn_pool_destroy(iterpool);
return SVN_NO_ERROR;
}
Modified: subversion/branches/1.7.x/subversion/libsvn_ra_neon/props.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/subversion/libsvn_ra_neon/props.c?rev=1460964&r1=1460963&r2=1460964&view=diff
==============================================================================
--- subversion/branches/1.7.x/subversion/libsvn_ra_neon/props.c (original)
+++ subversion/branches/1.7.x/subversion/libsvn_ra_neon/props.c Tue Mar 26 04:00:42 2013
@@ -1313,9 +1313,7 @@ svn_ra_neon__do_check_path(svn_ra_sessio
{
svn_ra_neon__session_t *ras = session->priv;
const char *url = ras->url->data;
- const char *bc_url;
- const char *bc_relative;
- svn_error_t *err;
+ svn_error_t *err = SVN_NO_ERROR;
svn_boolean_t is_dir;
/* ### For now, using svn_ra_neon__get_starting_props() works because
@@ -1350,18 +1348,42 @@ svn_ra_neon__do_check_path(svn_ra_sessio
if (path)
url = svn_path_url_add_component2(url, path, pool);
- err = svn_ra_neon__get_baseline_info(&bc_url, &bc_relative, NULL, ras,
- url, revision, pool);
+ /* If we're querying HEAD, we can do so against the public URL;
+ otherwise, we have to get a revision-specific URL to work with. */
+ if (SVN_IS_VALID_REVNUM(revision))
+ {
+ const char *bc_url;
+ const char *bc_relative;
+
+ err = svn_ra_neon__get_baseline_info(&bc_url, &bc_relative, NULL, ras,
+ url, revision, pool);
+ if (! err)
+ url = svn_path_url_add_component2(bc_url, bc_relative, pool);
+ }
+ else
+ {
+ ne_uri parsed_url;
+
+ /* svn_ra_neon__get_starting_props() wants only the path part of URL. */
+ ne_uri_parse(url, &parsed_url);
+ if (parsed_url.path)
+ {
+ url = apr_pstrdup(pool, parsed_url.path);
+ }
+ else
+ {
+ err = svn_error_createf(SVN_ERR_RA_ILLEGAL_URL, NULL,
+ _("Neon was unable to parse URL '%s'"), url);
+ }
+ ne_uri_free(&parsed_url);
+ }
if (! err)
{
svn_ra_neon__resource_t *rsrc;
- const char *full_bc_url = svn_path_url_add_component2(bc_url,
- bc_relative,
- pool);
-
- /* query the DAV:resourcetype of the full, assembled URL. */
- err = svn_ra_neon__get_starting_props(&rsrc, ras, full_bc_url, pool);
+
+ /* Query the DAV:resourcetype. */
+ err = svn_ra_neon__get_starting_props(&rsrc, ras, url, pool);
if (! err)
is_dir = rsrc->is_collection;
}
Modified: subversion/branches/1.7.x/subversion/libsvn_ra_serf/commit.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/subversion/libsvn_ra_serf/commit.c?rev=1460964&r1=1460963&r2=1460964&view=diff
==============================================================================
--- subversion/branches/1.7.x/subversion/libsvn_ra_serf/commit.c (original)
+++ subversion/branches/1.7.x/subversion/libsvn_ra_serf/commit.c Tue Mar 26 04:00:42 2013
@@ -1370,24 +1370,37 @@ open_root(void *edit_baton,
}
else
{
- svn_ra_serf__options_context_t *opt_ctx;
svn_ra_serf__simple_request_context_t *mkact_ctx;
- const char *activity_str;
+ const char *activity_str = ctx->session->activity_collection_url;
- SVN_ERR(svn_ra_serf__create_options_req(&opt_ctx, ctx->session,
- ctx->session->conns[0],
- ctx->session->session_url.path,
- ctx->pool));
-
- SVN_ERR(svn_ra_serf__context_run_wait(
- svn_ra_serf__get_options_done_ptr(opt_ctx),
- ctx->session, ctx->pool));
-
- activity_str = svn_ra_serf__options_get_activity_collection(opt_ctx);
if (!activity_str)
- return svn_error_create(SVN_ERR_RA_DAV_OPTIONS_REQ_FAILED, NULL,
- _("The OPTIONS response did not include the "
- "requested activity-collection-set value"));
+ {
+ svn_ra_serf__options_context_t *opt_ctx;
+
+ SVN_ERR(svn_ra_serf__create_options_req(&opt_ctx, ctx->session,
+ ctx->session->conns[0],
+ ctx->session->session_url.path,
+ ctx->pool));
+
+ SVN_ERR(svn_ra_serf__context_run_wait(
+ svn_ra_serf__get_options_done_ptr(opt_ctx),
+ ctx->session, ctx->pool));
+
+ activity_str = svn_ra_serf__options_get_activity_collection(opt_ctx);
+ }
+
+ /* Cache the result. */
+ if (activity_str)
+ {
+ ctx->session->activity_collection_url =
+ apr_pstrdup(ctx->session->pool, activity_str);
+ }
+ else
+ {
+ return svn_error_create(SVN_ERR_RA_DAV_OPTIONS_REQ_FAILED, NULL,
+ _("The OPTIONS response did not include the "
+ "requested activity-collection-set value"));
+ }
ctx->activity_url =
svn_path_url_add_component2(activity_str, svn_uuid_generate(ctx->pool),
Modified: subversion/branches/1.7.x/subversion/libsvn_ra_serf/options.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/subversion/libsvn_ra_serf/options.c?rev=1460964&r1=1460963&r2=1460964&view=diff
==============================================================================
--- subversion/branches/1.7.x/subversion/libsvn_ra_serf/options.c (original)
+++ subversion/branches/1.7.x/subversion/libsvn_ra_serf/options.c Tue Mar 26 04:00:42 2013
@@ -516,11 +516,22 @@ svn_ra_serf__exchange_capabilities(svn_r
return SVN_NO_ERROR;
}
- return svn_error_compose_create(
- svn_ra_serf__error_on_status(opt_ctx->status_code,
- serf_sess->session_url.path,
- opt_ctx->parser_ctx->location),
- err);
+ SVN_ERR(svn_error_compose_create(
+ svn_ra_serf__error_on_status(opt_ctx->status_code,
+ serf_sess->session_url.path,
+ opt_ctx->parser_ctx->location),
+ err));
+
+ /* Opportunistically cache any reported activity URL. (We don't
+ want to have to ask for this again later, potentially against an
+ unreadable commit anchor URL.) */
+ if (opt_ctx->activity_collection)
+ {
+ serf_sess->activity_collection_url =
+ apr_pstrdup(serf_sess->pool, opt_ctx->activity_collection);
+ }
+
+ return SVN_NO_ERROR;
}
Modified: subversion/branches/1.7.x/subversion/libsvn_ra_serf/ra_serf.h
URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/subversion/libsvn_ra_serf/ra_serf.h?rev=1460964&r1=1460963&r2=1460964&view=diff
==============================================================================
--- subversion/branches/1.7.x/subversion/libsvn_ra_serf/ra_serf.h (original)
+++ subversion/branches/1.7.x/subversion/libsvn_ra_serf/ra_serf.h Tue Mar 26 04:00:42 2013
@@ -163,6 +163,10 @@ struct svn_ra_serf__session_t {
constants' addresses, therefore). */
apr_hash_t *capabilities;
+ /* Activity collection URL. (Cached from the initial OPTIONS
+ request when run against HTTPv1 servers.) */
+ const char *activity_collection_url;
+
/* Are we using a proxy? */
int using_proxy;
Modified: subversion/branches/1.7.x/subversion/tests/cmdline/authz_tests.py
URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/subversion/tests/cmdline/authz_tests.py?rev=1460964&r1=1460963&r2=1460964&view=diff
==============================================================================
--- subversion/branches/1.7.x/subversion/tests/cmdline/authz_tests.py (original)
+++ subversion/branches/1.7.x/subversion/tests/cmdline/authz_tests.py Tue Mar 26 04:00:42 2013
@@ -1390,6 +1390,21 @@ def upgrade_absent(sbox):
svntest.actions.run_and_verify_update(sbox.wc_dir, expected_output,
None, None)
+@Skip(svntest.main.is_ra_type_file)
+@Issue(4332)
+def authz_del_from_subdir(sbox):
+ "delete file without rights on the root"
+
+ sbox.build(create_wc = False)
+
+ write_authz_file(sbox, {"/": "* = ", "/A": "jrandom = rw"})
+
+ write_restrictive_svnserve_conf(sbox.repo_dir)
+
+ svntest.actions.run_and_verify_svn(None, None, [],
+ 'rm', sbox.repo_url + '/A/mu',
+ '-m', '')
+
########################################################################
# Run the tests
@@ -1419,6 +1434,7 @@ test_list = [ None,
wc_delete,
wc_commit_error_handling,
upgrade_absent,
+ authz_del_from_subdir,
]
serial_only = True