You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by cm...@apache.org on 2013/01/12 02:55:09 UTC
svn commit: r1432357 - in /subversion/trunk: notes/http-and-webdav/
subversion/include/ subversion/libsvn_ra_serf/ subversion/mod_dav_svn/
subversion/mod_dav_svn/reports/
Author: cmpilato
Date: Sat Jan 12 01:55:08 2013
New Revision: 1432357
URL: http://svn.apache.org/viewvc?rev=1432357&view=rev
Log:
Fix issue #4287 ("svn_ra_replay (and related APIs) HTTP wire protocol
design flaw").
Teach mod_dav_svn to also accept "replay-report" REPORT requests
against a baselined version resource (practically speaking, a revision
resource), which is the correct resource to query about "changes which
occured in this revision". When the REPORT is aimed at a revision
resource, it should *not* include an embedded revision (that's
redundant at a minimum, and could be conflicting), and it *should*
carry an embedded "include path" (since that path is not represented
in the resource URL).
Teach ra_serf to send such REPORT requests against a revision resource
URL, only falling back to the use of the public session URL when the
server doesn't support the new approach.
* subversion/include/svn_dav.h
(SVN_DAV_NS_DAV_SVN_REPLAY_REV_RESOURCE): New header #define.
* subversion/libsvn_ra_serf/ra_serf.h
(svn_ra_serf__session_t): Add 'supports_rev_rsrc_replay' flag.
* subversion/libsvn_ra_serf/options.c
(capabilities_headers_iterator_callback): If we see the
SVN_DAV_NS_DAV_SVN_REPLAY_REV_RESOURCE header, set the session
baton's 'supports_rev_rsrc_replay' flag.
* subversion/libsvn_ra_serf/replay.c
(replay_context_t): Add 'include_path' member.
(create_replay_body): If an include path is provided, drop it into
the REPORT request body, and don't add the revision; otherwise, do
the reverse for compatibility with old servers.
(svn_ra_serf__replay_range): Consult the session baton's new flag to
see if we need to aim the REPORT at a revision resource (which is
the best option) or fall back to hitting the public resource URL
of the include path itself (the session URL).
* subversion/mod_dav_svn/reports/replay.c
(dav_svn__replay_report): Also handle the flavor of this REPORT
which occurs when it is submitted against a revision resource.
* subversion/mod_dav_svn/version.c
(get_option): When HTTP0-v2-enabled, include the new
SVN_DAV_NS_DAV_SVN_REPLAY_REV_RESOURCE header.
* notes/http-and-webdav/webdav-protocol
Document the existence of, and request syntax of, the
"replay-report" REPORT.
Modified:
subversion/trunk/notes/http-and-webdav/webdav-protocol
subversion/trunk/subversion/include/svn_dav.h
subversion/trunk/subversion/libsvn_ra_serf/options.c
subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
subversion/trunk/subversion/libsvn_ra_serf/replay.c
subversion/trunk/subversion/mod_dav_svn/reports/replay.c
subversion/trunk/subversion/mod_dav_svn/version.c
Modified: subversion/trunk/notes/http-and-webdav/webdav-protocol
URL: http://svn.apache.org/viewvc/subversion/trunk/notes/http-and-webdav/webdav-protocol?rev=1432357&r1=1432356&r2=1432357&view=diff
==============================================================================
--- subversion/trunk/notes/http-and-webdav/webdav-protocol (original)
+++ subversion/trunk/notes/http-and-webdav/webdav-protocol Sat Jan 12 01:55:08 2013
@@ -384,7 +384,7 @@ mergeinfo-report
----------------
Purpose: Retrieve the merge history for a portion of the repository
-(e.g. a set of paths) at a particular revision.
+ (e.g. a set of paths) at a particular revision.
Target URL: URL of item we're getting merge info for.
@@ -415,3 +415,41 @@ Response:
<S:mergeinfo-info>/A/B/E:1,3-4</S:mergeinfo-info>
</S:mergeinfo-item>
</S:mergeinfo-report>
+
+replay-report
+-------------
+
+Purpose: Retrieve a record of the changes made in a given revision,
+ possibly limited to only those changes which affect a
+ specific subtree of the repository.
+
+Target URL: Prior to Subversion 1.8, the target URL was the public
+ resource URL of the aforementioned subtree. Per issue #4287
+ (http://subversion.tigris.org/issues/show_bug.cgi?id=4287),
+ it was discovered that this was an incorrect approach, so
+ in Subversion 1.8, mod_dav_svn allowed clients to submit
+ this report (with a slightly different Request syntax)
+ against baselined version resources.
+
+Request:
+
+ Original syntax, used against a regular resource URL:
+
+ <S:replay-report xmlns:S=\"svn:\">
+ <S:revision>REVISION</S:revision>
+ <S:low-water-mark>LOW_WATER_MARK_REV</S:low-water-mark>
+ <S:send-deltas>0</S:send-deltas> (... or non-zero if sending deltas)
+ </S:replay-report>
+
+ New (in Subversion 1.8) syntax, used against a baselined version
+ resource URL:
+
+ <S:replay-report xmlns:S=\"svn:\">
+ <S:include-path>/trunk/subversion/tests</S:include-path>
+ <S:low-water-mark>LOW_WATER_MARK_REV</S:low-water-mark>
+ <S:send-deltas>0</S:send-deltas> (... or non-zero if sending deltas)
+ </S:replay-report>
+
+Response:
+
+ ### TODO ###
\ No newline at end of file
Modified: subversion/trunk/subversion/include/svn_dav.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_dav.h?rev=1432357&r1=1432356&r2=1432357&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_dav.h (original)
+++ subversion/trunk/subversion/include/svn_dav.h Sat Jan 12 01:55:08 2013
@@ -323,6 +323,13 @@ extern "C" {
#define SVN_DAV_NS_DAV_SVN_INLINE_PROPS\
SVN_DAV_PROP_NS_DAV "svn/inline-props"
+/** Presence of this in a DAV header in an OPTIONS response indicates
+ * that the transmitter (in this case, the server) knows how to handle
+ * a replay of a revision resource. Transmitters must be
+ * HTTP-v2-enabled to support this feature. */
+#define SVN_DAV_NS_DAV_SVN_REPLAY_REV_RESOURCE\
+ SVN_DAV_PROP_NS_DAV "svn/replay-rev-resource"
+
/** @} */
Modified: subversion/trunk/subversion/libsvn_ra_serf/options.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/options.c?rev=1432357&r1=1432356&r2=1432357&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/options.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/options.c Sat Jan 12 01:55:08 2013
@@ -211,11 +211,14 @@ capabilities_headers_iterator_callback(v
SVN_RA_CAPABILITY_EPHEMERAL_TXNPROPS, APR_HASH_KEY_STRING,
capability_yes);
}
-
if (svn_cstring_match_list(SVN_DAV_NS_DAV_SVN_INLINE_PROPS, vals))
{
session->supports_inline_props = TRUE;
}
+ if (svn_cstring_match_list(SVN_DAV_NS_DAV_SVN_REPLAY_REV_RESOURCE, vals))
+ {
+ session->supports_rev_rsrc_replay = TRUE;
+ }
}
/* SVN-specific headers -- if present, server supports HTTP protocol v2 */
Modified: subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h?rev=1432357&r1=1432356&r2=1432357&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h Sat Jan 12 01:55:08 2013
@@ -243,6 +243,10 @@ struct svn_ra_serf__session_t {
/* Indicates if the server supports sending inlined props in update editor
* in skelta mode (send-all == 'false'). */
svn_boolean_t supports_inline_props;
+
+ /* Indicates whether the server supports issuing replay REPORTs
+ against rev resources (children of `rev_stub', elsestruct). */
+ svn_boolean_t supports_rev_rsrc_replay;
};
#define SVN_RA_SERF__HAVE_HTTPV2_SUPPORT(sess) ((sess)->me_resource != NULL)
Modified: subversion/trunk/subversion/libsvn_ra_serf/replay.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/replay.c?rev=1432357&r1=1432356&r2=1432357&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/replay.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/replay.c Sat Jan 12 01:55:08 2013
@@ -107,7 +107,11 @@ typedef struct replay_context_t {
const svn_delta_editor_t *editor;
void *editor_baton;
- /* current revision */
+ /* Path and revision used to filter replayed changes. If
+ INCLUDE_PATH is non-NULL, REVISION is unnecessary and will not be
+ included in the replay REPORT. (Because the REPORT is being
+ aimed an HTTP v2 revision resource.) */
+ const char *include_path;
svn_revnum_t revision;
/* Information needed to create the replay report body */
@@ -599,10 +603,22 @@ create_replay_body(serf_bucket_t **bkt,
"xmlns:S", SVN_XML_NAMESPACE,
NULL);
- svn_ra_serf__add_tag_buckets(body_bkt,
- "S:revision",
- apr_ltoa(ctx->src_rev_pool, ctx->revision),
- alloc);
+ /* If we have a non-NULL include path, we add it to the body and
+ omit the revision; otherwise, the reverse. */
+ if (ctx->include_path)
+ {
+ svn_ra_serf__add_tag_buckets(body_bkt,
+ "S:include-path",
+ ctx->include_path,
+ alloc);
+ }
+ else
+ {
+ svn_ra_serf__add_tag_buckets(body_bkt,
+ "S:revision",
+ apr_ltoa(ctx->src_rev_pool, ctx->revision),
+ alloc);
+ }
svn_ra_serf__add_tag_buckets(body_bkt,
"S:low-water-mark",
apr_ltoa(ctx->src_rev_pool, ctx->low_water_mark),
@@ -728,9 +744,39 @@ svn_ra_serf__replay_range(svn_ra_session
svn_revnum_t rev = start_revision;
const char *report_target;
int active_reports = 0;
+ const char *include_path;
SVN_ERR(svn_ra_serf__report_resource(&report_target, session, NULL, pool));
+ /* Prior to 1.8, mod_dav_svn expect to get replay REPORT requests
+ aimed at the session URL. But that's incorrect -- these reports
+ aren't about specific resources -- they are above revisions. The
+ path-based filtering offered by this API is just that: a filter
+ applied to the full set of changes made in the revision. As
+ such, the correct target for these REPORT requests is the "me
+ resource" (or, pre-http-v2, the default VCC).
+
+ Our server should have told us if it supported this protocol
+ correction. If so, we aimed our report at the correct resource
+ and include the filtering path as metadata within the report
+ body. Otherwise, we fall back to the pre-1.8 behavior and just
+ wish for the best.
+
+ See issue #4287:
+ http://subversion.tigris.org/issues/show_bug.cgi?id=4287
+ */
+ if (session->supports_rev_rsrc_replay)
+ {
+ SVN_ERR(svn_ra_serf__get_relative_path(&include_path,
+ session->session_url.path,
+ session, session->conns[0],
+ pool));
+ }
+ else
+ {
+ include_path = NULL;
+ }
+
while (active_reports || rev <= end_revision)
{
svn_ra_serf__list_t *done_list;
@@ -747,6 +793,7 @@ svn_ra_serf__replay_range(svn_ra_session
svn_ra_serf__handler_t *handler;
svn_ra_serf__xml_parser_t *parser_ctx;
apr_pool_t *ctx_pool = svn_pool_create(pool);
+ const char *replay_target;
replay_ctx = apr_pcalloc(ctx_pool, sizeof(*replay_ctx));
replay_ctx->src_rev_pool = ctx_pool;
@@ -754,6 +801,7 @@ svn_ra_serf__replay_range(svn_ra_session
replay_ctx->revfinish_func = revfinish_func;
replay_ctx->replay_baton = replay_baton;
replay_ctx->done = FALSE;
+ replay_ctx->include_path = include_path;
replay_ctx->revision = rev;
replay_ctx->low_water_mark = low_water_mark;
replay_ctx->send_deltas = send_deltas;
@@ -763,10 +811,10 @@ svn_ra_serf__replay_range(svn_ra_session
replay_ctx->revs_props = apr_hash_make(replay_ctx->src_rev_pool);
if (SVN_RA_SERF__HAVE_HTTPV2_SUPPORT(session))
- {
- replay_ctx->revprop_target = apr_psprintf(pool, "%s/%ld",
- session->rev_stub, rev);
- replay_ctx->revprop_rev = SVN_INVALID_REVNUM;
+ {
+ replay_ctx->revprop_target = apr_psprintf(pool, "%s/%ld",
+ session->rev_stub, rev);
+ replay_ctx->revprop_rev = SVN_INVALID_REVNUM;
}
else
{
@@ -786,12 +834,22 @@ svn_ra_serf__replay_range(svn_ra_session
/* Spin up the serf request for the PROPFIND. */
svn_ra_serf__request_create(replay_ctx->propfind_handler);
- /* Send the replay report request. */
+ /* Send the replay REPORT request. */
+ if (session->supports_rev_rsrc_replay)
+ {
+ replay_target = apr_psprintf(pool, "%s/%ld",
+ session->rev_stub, rev);
+ }
+ else
+ {
+ replay_target = session->session_url.path;
+ }
+
handler = apr_pcalloc(replay_ctx->src_rev_pool, sizeof(*handler));
handler->handler_pool = replay_ctx->src_rev_pool;
handler->method = "REPORT";
- handler->path = session->session_url.path;
+ handler->path = replay_target;
handler->body_delegate = create_replay_body;
handler->body_delegate_baton = replay_ctx;
handler->conn = session->conns[0];
Modified: subversion/trunk/subversion/mod_dav_svn/reports/replay.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/reports/replay.c?rev=1432357&r1=1432356&r2=1432357&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/reports/replay.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/reports/replay.c Sat Jan 12 01:55:08 2013
@@ -418,11 +418,11 @@ dav_svn__replay_report(const dav_resourc
{
dav_error *derr = NULL;
svn_revnum_t low_water_mark = SVN_INVALID_REVNUM;
- svn_revnum_t rev = SVN_INVALID_REVNUM;
+ svn_revnum_t rev;
const svn_delta_editor_t *editor;
svn_boolean_t send_deltas = TRUE;
dav_svn__authz_read_baton arb;
- const char *base_dir = resource->info->repos_path;
+ const char *base_dir;
apr_bucket_brigade *bb;
apr_xml_elem *child;
svn_fs_root_t *root;
@@ -430,10 +430,27 @@ dav_svn__replay_report(const dav_resourc
void *edit_baton;
int ns;
- /* The request won't have a repos_path if it's for the root. */
- if (! base_dir)
- base_dir = "";
-
+ /* In Subversion 1.8, we allowed this REPORT to be issue against a
+ revision resource. Doing so means the REV is part of the request
+ URL, and BASE_DIR is embedded in the request body.
+
+ The old-school (and incorrect, see issue #4287 --
+ http://subversion.tigris.org/issues/show_bug.cgi?id=4287) way was
+ to REPORT on the public URL of the BASE_DIR and embed the REV in
+ the report body.
+ */
+ if (resource->baselined
+ && (resource->type == DAV_RESOURCE_TYPE_VERSION))
+ {
+ rev = resource->info->root.rev;
+ base_dir = NULL;
+ }
+ else
+ {
+ rev = SVN_INVALID_REVNUM;
+ base_dir = resource->info->repos_path;
+ }
+
arb.r = resource->info->r;
arb.repos = resource->info->repos;
@@ -455,9 +472,17 @@ dav_svn__replay_report(const dav_resourc
if (strcmp(child->name, "revision") == 0)
{
+ if (SVN_IS_VALID_REVNUM(rev))
+ {
+ /* Uh... we already have a revision to use, either
+ because this tag is non-unique or because the
+ request was submitted against a revision-bearing
+ resource URL. Either way, something's not
+ right. */
+ return malformed_element_error("revision", resource->pool);
+ }
+
cdata = dav_xml_get_cdata(child, resource->pool, 1);
- if (! cdata)
- return malformed_element_error("revision", resource->pool);
rev = SVN_STR_TO_REV(cdata);
}
else if (strcmp(child->name, "low-water-mark") == 0)
@@ -483,6 +508,15 @@ dav_svn__replay_report(const dav_resourc
}
send_deltas = parsed_val != 0;
}
+ else if (strcmp(child->name, "include-path") == 0)
+ {
+ cdata = dav_xml_get_cdata(child, resource->pool, 1);
+ if ((derr = dav_svn__test_canonical(cdata, resource->pool)))
+ return derr;
+
+ /* Force BASE_DIR to be a relative path, not an fspath. */
+ base_dir = svn_relpath_canonicalize(cdata, resource->pool);
+ }
}
}
@@ -498,6 +532,9 @@ dav_svn__replay_report(const dav_resourc
"Request was missing the low-water-mark argument.",
SVN_DAV_ERROR_NAMESPACE, SVN_DAV_ERROR_TAG);
+ if (! base_dir)
+ base_dir = "";
+
bb = apr_brigade_create(resource->pool, output->c->bucket_alloc);
if ((err = svn_fs_revision_root(&root, resource->info->repos->fs, rev,
Modified: subversion/trunk/subversion/mod_dav_svn/version.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/version.c?rev=1432357&r1=1432356&r2=1432357&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/version.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/version.c Sat Jan 12 01:55:08 2013
@@ -266,6 +266,14 @@ get_option(const dav_resource *resource,
{ "create-txn-with-props", { 1, 8, 0, "" } },
};
+ /* Add the header which indicates that this server can handle
+ replay REPORTs submitted against an HTTP v2 revision resource. */
+ apr_table_addn(r->headers_out, "DAV",
+ SVN_DAV_NS_DAV_SVN_REPLAY_REV_RESOURCE);
+
+ /* Add a bunch of HTTP v2 headers which carry resource and
+ resource stub URLs that the client can use to naively build
+ addressable resources. */
apr_table_set(r->headers_out, SVN_DAV_ROOT_URI_HEADER, repos_root_uri);
apr_table_set(r->headers_out, SVN_DAV_ME_RESOURCE_HEADER,
apr_pstrcat(resource->pool, repos_root_uri, "/",