[PATCH] HTTPv2 allow client to control transaction name in protocol

The V2 HTTP protocol replaces MKACTIVITY and the client supplied UUID
with POST and a server supplied name that corresponds to the
underlying FS transaction name. This means that FS transaction names
are now exposed in the HTTP protocol. For ordinary client-server
operation this is not a problem but it does cause difficulties for any
sort of proxy that tries to multiplex or replay HTTP requests.

The root of the problem is that transaction names depend on the
internal state of the repository, via the transaction sequence number.
Consider two nominally identical repository states, say two
repositories where one is a dump/load or svnsync copy of another, or a
single repository before and after a failed commit. These nominally
identical repository states may have different sequence numbers and
so will provide different responses to POST, and that will in turn
require different URLs in subsequent requests that make up the commit.
This is a change from the V1 protocol.

There are ways that proxies could work around this problem but they
involve building more knowledge of the Subversion protocol into the
proxy. An alternative is to change the V2 protocol to disconnect the
internal transaction name from the "visible transaction name". This
patch allows the client to send a UUID with POST, makes the server use
the UUID to construct the "visible transaction name" and makes the
server detect and map client-supplied visible names to the underlying FS
transaction names. The new behaviour is optional, if the client doesn't
send a UUID the server continues to respond with the FS transaction
name. The server uses the existind activities database to store the new
mapping; although these are not strictly DAV activities they are
essentially the same information.

The patch includes changes to make the client send the new header, but
that part is compile-time disabled by default as I don't expect normal
clients to use it. The "client" that sends the new UUID will usually
be the proxy.

[Disclaimer: I work for WANdisco and the WANdisco replicator is a proxy
that will have to handle the V2 protocol.]

Log and patch:

Allow HTTPv2 clients to specify that the FS transaction name should
not be returned by a "create-txn" POST request.

If the client sends an SVN-Txn-Name header it defines part of the
SVN-Txn-Name transaction name to be returned by the server, if no
header is sent the server will use the FS transaction name as before.
The server will identify client supplied "visible transaction names"
in subsequent requests and map them to the underlying FS transaction
name.

* subversion/include/svn_dav.h
  (SVN_DAV_TXN_NAME_HEADER): Document new behaviour.

* notes/http-and-webdav/http-protocol-v2.txt: Update.

* subversion/mod_dav_svn/dav_svn.h
  (DAV_SVN__VISIBLE_TXN_PREFIX): New.
  (struct dav_svn_root): Add visible_txn_name member.

* subversion/mod_dav_svn/posts/create_txn.c
  (dav_svn__post_create_txn): Get visible_txn_name from header if sent,
   otherwise use the FS txn_name, store visible_txn_name:txn_name mapping
   in the activity database if required.

* subversion/mod_dav_svn/version.c
  (merge): Delete visible_txn_name:txn_name mapping after commit.

* subversion/mod_dav_svn/repos.c
  (parse_txnstub_uri, parse_txnroot_uri): Map visible_txn_name to txn_name.
  (remove_resource): Delete mapping when aborting the transaction.

* subversion/libsvn_ra_serf/commit.c
  (setup_post_headers): New.
  (open_root): Set header_delegate.

Index: notes/http-and-webdav/http-protocol-v2.txt
===================================================================
--- notes/http-and-webdav/http-protocol-v2.txt (revision 1075718)
+++ notes/http-and-webdav/http-protocol-v2.txt (working copy)
@@ -226,6 +226,13 @@
          which can then be appended to the transaction stub and
          transaction root stub as necessary.
 
+ - Optionally, the client may send a UUID with the POST request
+ and the the server response will base the "visible name" of the
+ transaction on that UUID rather than on FS transaction nmae.
+ The client still uses the returned transaction name in
+ subsequent requests, and the server maps it to the underlying
+ FS transaction name.
+
      - Once the commit transaction is created, the client is free to
        send write requests against transaction resources it constructs
        itself. This eliminates the CHECKOUT requests, and also
Index: subversion/mod_dav_svn/dav_svn.h
===================================================================
--- subversion/mod_dav_svn/dav_svn.h (revision 1075718)
+++ subversion/mod_dav_svn/dav_svn.h (working copy)
@@ -52,6 +52,9 @@
 /* a pool-key for the shared dav_svn_root used by autoversioning */
 #define DAV_SVN__AUTOVERSIONING_ACTIVITY "svn-autoversioning-activity"
 
+/* used to distinguish client supplied TXN-NAME from FS supplied, this
+ is a string that is NEVER the start of an FS transaction name */
+#define DAV_SVN__VISIBLE_TXN_PREFIX "$"
 
 /* dav_svn_repos
  *
@@ -202,6 +205,17 @@
   */
   const char *txn_name;
 
+ /* The visible transaction name returned to an HTTPv2 client and
+ used in subsequent requests. This may be equal to the FS
+ txn_name, or it may be derived from a name supplied by the client
+ in which case the activity database is used to map to the FS
+ transaction name.
+
+ PRIVATE resources that directly represent either a txn or
+ txn-root use this field.
+ */
+ const char *visible_txn_name;
+
   /* If the root is part of a transaction, this contains the FS's transaction
      handle. It may be NULL if this root corresponds to a specific revision.
      It may also be NULL if we have not opened the transaction yet.
Index: subversion/mod_dav_svn/version.c
===================================================================
--- subversion/mod_dav_svn/version.c (revision 1075718)
+++ subversion/mod_dav_svn/version.c (working copy)
@@ -1427,6 +1427,16 @@
           svn_error_clear(serr);
           serr = SVN_NO_ERROR;
         }
+
+ /* HTTPv2 doesn't send DELETE after a successful MERGE so if
+ using the optional visible_txn_name mapping then delete it
+ here. */
+ if (source->info->root.visible_txn_name
+ && !strncmp(DAV_SVN__VISIBLE_TXN_PREFIX,
+ source->info->root.visible_txn_name,
+ sizeof(DAV_SVN__VISIBLE_TXN_PREFIX) - 1))
+ dav_svn__delete_activity(source->info->repos,
+ source->info->root.visible_txn_name);
     }
   else
     {
Index: subversion/mod_dav_svn/posts/create_txn.c
===================================================================
--- subversion/mod_dav_svn/posts/create_txn.c (revision 1075718)
+++ subversion/mod_dav_svn/posts/create_txn.c (working copy)
@@ -37,6 +37,7 @@
                          ap_filter_t *output)
 {
   const char *txn_name;
+ const char *visible_txn_name;
   dav_error *derr;
   request_rec *r = resource->info->r;
 
@@ -45,9 +46,24 @@
                                   resource->pool)))
     return derr;
 
+ /* If the client supplied a transaction name then add the special
+ prefix and store a mapping from the prefixed client name to the
+ FS transaction name in the activity database. */
+ visible_txn_name = apr_table_get(r->headers_in, SVN_DAV_TXN_NAME_HEADER);
+ if (!visible_txn_name || !visible_txn_name[0])
+ visible_txn_name = txn_name;
+ else
+ visible_txn_name = apr_pstrcat(resource->pool, DAV_SVN__VISIBLE_TXN_PREFIX,
+ visible_txn_name, NULL);
+ if (!strncmp(DAV_SVN__VISIBLE_TXN_PREFIX, visible_txn_name,
+ sizeof(DAV_SVN__VISIBLE_TXN_PREFIX) - 1))
+ if ((derr = dav_svn__store_activity(resource->info->repos,
+ visible_txn_name, txn_name)))
+ return derr;
+
   /* Build a "201 Created" response with header that tells the
      client our new transaction's name. */
- apr_table_set(r->headers_out, SVN_DAV_TXN_NAME_HEADER, txn_name);
+ apr_table_set(r->headers_out, SVN_DAV_TXN_NAME_HEADER, visible_txn_name);
   r->status = HTTP_CREATED;
 
   return NULL;
Index: subversion/mod_dav_svn/repos.c
===================================================================
--- subversion/mod_dav_svn/repos.c (revision 1075718)
+++ subversion/mod_dav_svn/repos.c (working copy)
@@ -486,7 +486,13 @@
 
   comb->res.type = DAV_RESOURCE_TYPE_PRIVATE;
   comb->priv.restype = DAV_SVN_RESTYPE_TXN_COLLECTION;
- comb->priv.root.txn_name = apr_pstrdup(comb->res.pool, path);
+ comb->priv.root.visible_txn_name = apr_pstrdup(comb->res.pool, path);
+ if (strncmp(DAV_SVN__VISIBLE_TXN_PREFIX, comb->priv.root.visible_txn_name,
+ sizeof(DAV_SVN__VISIBLE_TXN_PREFIX) - 1))
+ comb->priv.root.txn_name = comb->priv.root.visible_txn_name;
+ else
+ comb->priv.root.txn_name
+ = dav_svn__get_txn(comb->priv.repos, comb->priv.root.visible_txn_name);
 
   return FALSE;
 }
@@ -528,16 +534,24 @@
       /* There's no slash character in our path. Assume it's just an
          TXN_NAME pointing to the root path. That should be cool.
          We'll just drop through to the normal case handling below. */
- comb->priv.root.txn_name = apr_pstrdup(comb->res.pool, path);
+ comb->priv.root.visible_txn_name = apr_pstrdup(comb->res.pool, path);
       comb->priv.repos_path = "/";
     }
   else
     {
- comb->priv.root.txn_name = apr_pstrndup(comb->res.pool, path,
- slash - path);
+ comb->priv.root.visible_txn_name = apr_pstrndup(comb->res.pool, path,
+ slash - path);
       comb->priv.repos_path = slash;
     }
 
+ if (strncmp(DAV_SVN__VISIBLE_TXN_PREFIX, comb->priv.root.visible_txn_name,
+ sizeof(DAV_SVN__VISIBLE_TXN_PREFIX) - 1))
+ comb->priv.root.txn_name = comb->priv.root.visible_txn_name;
+ else
+ comb->priv.root.txn_name
+ = dav_svn__get_txn(comb->priv.repos,
+ comb->priv.root.visible_txn_name);
+
   return FALSE;
 }
 
@@ -3783,11 +3797,15 @@
   if (resource->type == DAV_RESOURCE_TYPE_PRIVATE
       && resource->info->restype == DAV_SVN_RESTYPE_TXN_COLLECTION)
     {
- /* We'll assume that no activity was created to map to this
- transaction. */
- return dav_svn__abort_txn(resource->info->repos,
- resource->info->root.txn_name,
- resource->pool);
+ if (strncmp(DAV_SVN__VISIBLE_TXN_PREFIX,
+ resource->info->root.visible_txn_name,
+ sizeof(DAV_SVN__VISIBLE_TXN_PREFIX) - 1))
+ return dav_svn__abort_txn(resource->info->repos,
+ resource->info->root.txn_name,
+ resource->pool);
+ else
+ return dav_svn__delete_activity(resource->info->repos,
+ resource->info->root.visible_txn_name);
     }
 
   /* ### note that the parent was checked out at some point, and this
Index: subversion/include/svn_dav.h
===================================================================
--- subversion/include/svn_dav.h (revision 1075718)
+++ subversion/include/svn_dav.h (working copy)
@@ -145,7 +145,9 @@
  * name of the Subversion transaction created by the request. It can
  * then be appended to the transaction stub and transaction root stub
  * for access to the properties and paths, respectively, of the named
- * transaction. (HTTP protocol v2 only) */
+ * transaction. Optionally, the client can send this header in the
+ * POST request to cause the response to be based on the client-supplied
+ * UUID. (HTTP protocol v2 only) */
 #define SVN_DAV_TXN_NAME_HEADER "SVN-Txn-Name"
 
 /**
Index: subversion/libsvn_ra_serf/commit.c
===================================================================
--- subversion/libsvn_ra_serf/commit.c (revision 1075718)
+++ subversion/libsvn_ra_serf/commit.c (working copy)
@@ -1141,6 +1141,19 @@
 }
 
 static svn_error_t *
+setup_post_headers(serf_bucket_t *headers,
+ void *baton,
+ apr_pool_t *pool)
+{
+#ifdef SVN_SERF_SEND_TXN_NAME
+ serf_bucket_headers_set(headers, SVN_DAV_TXN_NAME_HEADER,
+ svn_uuid_generate(pool));
+#endif
+
+ return SVN_NO_ERROR;
+}
+
+static svn_error_t *
 setup_delete_headers(serf_bucket_t *headers,
                      void *baton,
                      apr_pool_t *pool)
@@ -1308,6 +1321,8 @@
       handler->body_type = SVN_SKEL_MIME_TYPE;
       handler->body_delegate = create_txn_post_body;
       handler->body_delegate_baton = NULL;
+ handler->header_delegate = setup_post_headers;
+ handler->header_delegate_baton = NULL;
       handler->path = ctx->session->me_resource;
       handler->conn = ctx->session->conns[0];
       handler->session = ctx->session;

-- 
Philip