[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[PATCH] Add explicit protocol version number to HTTP OPTIONS response.

From: C. Michael Pilato <cmpilato_at_collab.net>
Date: Thu, 24 Jun 2010 12:42:00 -0400

Since Subversion 1.5, our HTTP clients always make an OPTIONS exchange with
the server. The OPTIONS response originally carried information about
supported server features by way of naming those features individually. In
Subversion 1.7, we're expanding our horizons a bit by implementing a whole
new customized WebDAV implementation, complete with magical and mystical
private DAV resources which can be queried more directly than the former
negotiate-a-resource-target system allowed. For now we're letting the
presence of a header which names a particular such resource (the "me
resource") carry the information that the server can speak this new
protocol. But I'm wondering if at this point -- now that we've embraced the
idea of *not* embracing the WebDAV standard quite so firmly -- we wouldn't
be best served by adding an explicit protocol version number to the OPTIONS
response? A single number can carry a multitude of information far more
efficiently than a potentially boundless list of individual support strings.

(I'm thinking about these things because our current HTTPv2 is not the end
of our development of this protocol. We've talked quite a bit lately about
extending our POST handler in various ways, for example.)

There are, to my knowledge, two downsides to this:

1. third-party server implementations of our protocol would need to support
the whole of our latest protocol before claiming they support any of it. To
my knowledge there are a couple of such server implementations (WANdisco's
SVN-J and another company's IIS plugin).

2. to the degree that we wish to support not-yet-released server versions
(for our dev work, for example), we'd be bumping the protocol version number
just as often as if we'd added individual feature negotiation strings. That
means more code churn with less protocol-level transparency about what's
changed.

I think I still like the idea, if only because it disentangles "server
features" from "server vocabulary". But I'm not convinced enough to commit
the attached patch. So, as I'm wont to do, I'll use this last as my patch
backup system. :-)

Thoughts? Concerns? Greg Steins?

-- 
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

Add an explicit protocol format number to the HTTP OPTIONS response.

* subversion/include/svn_dav.h
  (SVN_DAV_PROTOCOL_VERSION_HEADER, SVN_DAV_PROTOCOL_VERSION_LATEST,
   SVN_DAV_MIN_HTTPV2_VERSION): New #defines.

* subversion/libsvn_ra_neon/ra_neon.h
  (svn_ra_neon__session_t): Add 'protocol_version' member, and
    rejigger the formatting of nearby comments and such.
  (SVN_RA_NEON__HAVE_HTTPV2_SUPPORT): Removed. All consumers updated
    to instead use...
  (SVN_RA_NEON__IS_VERSION_SUPPORTED): ... this new macro.

* subversion/libsvn_ra_neon/session.c
  (svn_ra_neon__open): Set the default value of the session baton's
    new protocol_version member.

* subversion/libsvn_ra_neon/options.c
  (parse_capabilities): Look for the new SVN-Protocol-Version header,
    and store its value in the session baton. If we see a "me
    resource" but not a protocol version, store the minimal "HTTP-v2-ish"
    version in the session baton.

* subversion/libsvn_ra_neon/fetch.c,
* subversion/libsvn_ra_neon/get_dated_rev.c
  Update consumer of SVN_RA_NEON__HAVE_HTTPV2_SUPPORT() to use
  SVN_RA_NEON__IS_VERSION_SUPPORTED() instead.

* subversion/libsvn_ra_serf/ra_serf.h
  (svn_ra_serf__session_t): Add 'protocol_version' member, and
    rejigger the formatting of nearby comments and such.
  (SVN_RA_SERF__HAVE_HTTPV2_SUPPORT): Removed. All consumers updated
    to instead use...
  (SVN_RA_SERF__IS_VERSION_SUPPORTED): ... this new macro.

* subversion/libsvn_ra_serf/serf.c
  (svn_ra_serf__open): Set the default value of the session baton's
    new protocol_version member.

* subversion/libsvn_ra_serf/options.c
  (capabilities_headers_iterator_callback): Look for the new
    SVN-Protocol-Version header, and store its value in the session baton.
  (options_response_handler): If we got a "me resource" in the
    headers, but not a protocol version, store the minimal "HTTP-v2-ish"
    version in the session baton.

* subversion/libsvn_ra_serf/commit.c,
* subversion/libsvn_ra_serf/merge.c,
* subversion/libsvn_ra_serf/property.c,
* subversion/libsvn_ra_serf/update.c,
* subversion/libsvn_ra_serf/util.c
  Update consumers of SVN_RA_SERF__HAVE_HTTPV2_SUPPORT() to use
  SVN_RA_SERF__IS_VERSION_SUPPORTED() instead.

--This line, and those below, will be ignored--
Index: subversion/include/svn_dav.h
===================================================================
--- subversion/include/svn_dav.h (revision 957402)
+++ subversion/include/svn_dav.h (working copy)
@@ -96,10 +96,16 @@
  * repository. */
 #define SVN_DAV_REPOS_UUID_HEADER "SVN-Repository-UUID"
 
-/** Presence of this in a DAV header in an OPTIONS response indicates
- * that the server speaks HTTP protocol v2. This header provides an
- * opaque URI that the client should send all custom REPORT requests
- * against. */
+/** This header indicates the protocol version understood by the
+ * server (an integer value). If this header is not present in an
+ * OPTIONS response, clients should assume that the server understands
+ * only Subversion's original (pre-v2) WebDAV protocol. */
+#define SVN_DAV_PROTOCOL_VERSION_HEADER "SVN-Protocol-Version"
+
+/** This header provides an opaque URI that the client should send all
+ * custom REPORT requests against. (Presence of this in a DAV header
+ * in an OPTIONS response indicates that the server speaks at least
+ * HTTP protocol v2.) */
 #define SVN_DAV_ME_RESOURCE_HEADER "SVN-Me-Resource"
 
 /** This header provides the repository root URI, suitable for use in
@@ -167,6 +173,20 @@
 #define SVN_DAV_RESULT_FULLTEXT_MD5_HEADER "X-SVN-Result-Fulltext-MD5"
 /** @} */
 
+
+/**
+ * @defgroup svn_dav_protocol HTTP protocol formats.
+ * @{ */
+
+/** The latest version of the HTTP protocol. */
+#define SVN_DAV_PROTOCOL_VERSION_LATEST 2
+
+/** Minimum protocol version that supports "HTTP v2" semantics. */
+#define SVN_DAV_MIN_HTTPV2_VERSION 2
+
+/** @} */
+
+
 /* ### should add strings for the various XML elements in the reports
    ### and things. also the custom prop names. etc.
 */
Index: subversion/libsvn_ra_serf/serf.c
===================================================================
--- subversion/libsvn_ra_serf/serf.c (revision 957402)
+++ subversion/libsvn_ra_serf/serf.c (working copy)
@@ -344,6 +344,7 @@
 
   serf_sess = apr_pcalloc(pool, sizeof(*serf_sess));
   serf_sess->pool = svn_pool_create(pool);
+ serf_sess->protocol_version = 1;
   serf_sess->bkt_alloc = serf_bucket_allocator_create(serf_sess->pool, NULL,
                                                       NULL);
   serf_sess->cached_props = apr_hash_make(serf_sess->pool);
@@ -362,8 +363,9 @@
                                _("Illegal repository URL '%s'"),
                                repos_URL);
     }
- /* Contrary to what the comment for apr_uri_t.path says in apr-util 1.2.12 and
- older, for root paths url.path will be "", where serf requires "/". */
+ /* Contrary to what the comment for apr_uri_t.path says in apr-util
+ 1.2.12 and older, for root paths url.path will be "", where serf
+ requires "/". */
   if (url.path == NULL || url.path[0] == '\0')
     url.path = apr_pstrdup(serf_sess->pool, "/");
   if (!url.port)
@@ -521,7 +523,7 @@
   props = apr_hash_make(pool);
   *ret_props = apr_hash_make(pool);
 
- if (SVN_RA_SERF__HAVE_HTTPV2_SUPPORT(session))
+ if (SVN_RA_SERF__IS_VERSION_SUPPORTED(session, SVN_DAV_MIN_HTTPV2_VERSION))
     {
       propfind_path = apr_psprintf(pool, "%s/%ld", session->rev_stub, rev);
 
@@ -968,7 +970,8 @@
       /* We should never get here if we have HTTP v2 support, because
          any server with that support should be transmitting the
          UUID in the initial OPTIONS response. */
- SVN_ERR_ASSERT(! SVN_RA_SERF__HAVE_HTTPV2_SUPPORT(session));
+ SVN_ERR_ASSERT(! SVN_RA_SERF__IS_VERSION_SUPPORTED(session,
+ SVN_DAV_MIN_HTTPV2_VERSION));
 
       /* We're not interested in vcc_url and relative_url, but this call also
          stores the repository's uuid in the session. */
Index: subversion/libsvn_ra_serf/merge.c
===================================================================
--- subversion/libsvn_ra_serf/merge.c (revision 957402)
+++ subversion/libsvn_ra_serf/merge.c (working copy)
@@ -309,7 +309,8 @@
           /* We now need to dive all the way into the WC to update the
            * base VCC url.
            */
- if ((! SVN_RA_SERF__HAVE_HTTPV2_SUPPORT(ctx->session))
+ if ((! SVN_RA_SERF__IS_VERSION_SUPPORTED(ctx->session,
+ SVN_DAV_MIN_HTTPV2_VERSION))
               && ctx->session->wc_callbacks->push_wc_prop)
             {
               svn_string_t checked_in_str;
Index: subversion/libsvn_ra_serf/util.c
===================================================================
--- subversion/libsvn_ra_serf/util.c (revision 957402)
+++ subversion/libsvn_ra_serf/util.c (working copy)
@@ -1659,7 +1659,8 @@
 
       /* This should only happen if we haven't detected HTTP v2
          support from the server. */
- assert(! SVN_RA_SERF__HAVE_HTTPV2_SUPPORT(session));
+ assert(! SVN_RA_SERF__IS_VERSION_SUPPORTED(session,
+ SVN_DAV_MIN_HTTPV2_VERSION));
 
       /* We don't actually care about the VCC_URL, but this API
          promises to populate the session's root-url cache, and that's
@@ -1691,7 +1692,7 @@
 {
   /* If we have HTTP v2 support, we want to report against the 'me'
      resource. */
- if (SVN_RA_SERF__HAVE_HTTPV2_SUPPORT(session))
+ if (SVN_RA_SERF__IS_VERSION_SUPPORTED(session, SVN_DAV_MIN_HTTPV2_VERSION))
     *report_target = apr_pstrdup(pool, session->me_resource);
 
   /* Otherwise, we'll use the default VCC. */
Index: subversion/libsvn_ra_serf/update.c
===================================================================
--- subversion/libsvn_ra_serf/update.c (revision 957402)
+++ subversion/libsvn_ra_serf/update.c (working copy)
@@ -1754,7 +1754,8 @@
        * In HTTP v2, we can simply construct the URL we need given the
        * path and base revision number.
        */
- if (SVN_RA_SERF__HAVE_HTTPV2_SUPPORT(ctx->sess))
+ if (SVN_RA_SERF__IS_VERSION_SUPPORTED(ctx->sess,
+ SVN_DAV_MIN_HTTPV2_VERSION))
         {
           const char *fs_path;
           const char *full_path = svn_uri_join(ctx->sess->repos_url.path,
Index: subversion/libsvn_ra_serf/property.c
===================================================================
--- subversion/libsvn_ra_serf/property.c (revision 957402)
+++ subversion/libsvn_ra_serf/property.c (working copy)
@@ -963,7 +963,7 @@
   /* If we detected HTTP v2 support on the server, we can construct
      the baseline collection URL ourselves, and fetch the latest
      revision (if needed) with an OPTIONS request. */
- if (SVN_RA_SERF__HAVE_HTTPV2_SUPPORT(session))
+ if (SVN_RA_SERF__IS_VERSION_SUPPORTED(session, SVN_DAV_MIN_HTTPV2_VERSION))
     {
       basecoll_url = apr_psprintf(pool, "%s/%ld",
                                   session->rev_root_stub, revision);
Index: subversion/libsvn_ra_serf/ra_serf.h
===================================================================
--- subversion/libsvn_ra_serf/ra_serf.h (revision 957402)
+++ subversion/libsvn_ra_serf/ra_serf.h (working copy)
@@ -225,31 +225,23 @@
   /* Connection timeout value */
   long timeout;
 
- /*** HTTP v2 protocol stuff. ***
- *
- * We assume that if mod_dav_svn sends one of the special v2 OPTIONs
- * response headers, it has sent all of them. Specifically, we'll
- * be looking at the presence of the "me resource" as a flag that
- * the server supports v2 of our HTTP protocol.
- */
+ /*** Enhanced HTTP protocol stuff ***/
 
- /* The "me resource". Typically used as a target for REPORTs that
- are path-agnostic. If we have this, we can speak HTTP v2 to the
- server. */
- const char *me_resource;
+ int protocol_version; /* protocol version supported by the server */
+ const char *me_resource; /* resource decribing the repository as a whole
+ (generally used for path-agnostic ops) */
+ const char *rev_stub; /* for accessing revisions (i.e. revprops) */
+ const char *rev_root_stub; /* for accessing REV/PATH pairs */
+ const char *txn_stub; /* for accessing transactions (i.e. txnprops) */
+ const char *txn_root_stub; /* for accessing TXN/PATH pairs */
 
- /* Opaque URL "stubs". If the OPTIONS response returns these, then
- we know we're using HTTP protocol v2. */
- const char *rev_stub; /* for accessing revisions (i.e. revprops) */
- const char *rev_root_stub; /* for accessing REV/PATH pairs */
- const char *txn_stub; /* for accessing transactions (i.e. txnprops) */
- const char *txn_root_stub; /* for accessing TXN/PATH pairs */
+ /*** End Enhanced HTTP stuff ***/
 
- /*** End HTTP v2 stuff ***/
-
 };
 
-#define SVN_RA_SERF__HAVE_HTTPV2_SUPPORT(sess) ((sess)->me_resource != NULL)
+/* Test svn_ra_serf__session_t *S to see if the server to which it is
+ connected supports at least version V of our HTTP protocol. */
+#define SVN_RA_SERF__IS_VERSION_SUPPORTED(s,v) (v >= s->protocol_version)
 
 /*
  * Structure which represents a DAV element with a NAMESPACE and NAME.
Index: subversion/libsvn_ra_serf/commit.c
===================================================================
--- subversion/libsvn_ra_serf/commit.c (revision 957402)
+++ subversion/libsvn_ra_serf/commit.c (working copy)
@@ -1084,7 +1084,8 @@
   apr_hash_index_t *hi;
   const char *proppatch_target;
 
- if (SVN_RA_SERF__HAVE_HTTPV2_SUPPORT(ctx->session))
+ if (SVN_RA_SERF__IS_VERSION_SUPPORTED(ctx->session,
+ SVN_DAV_MIN_HTTPV2_VERSION))
     {
       svn_ra_serf__simple_request_context_t *post_ctx;
       post_response_ctx_t *prc;
@@ -2163,7 +2164,7 @@
   commit->session = session;
   commit->conn = session->conns[0];
 
- if (SVN_RA_SERF__HAVE_HTTPV2_SUPPORT(session))
+ if (SVN_RA_SERF__IS_VERSION_SUPPORTED(session, SVN_DAV_MIN_HTTPV2_VERSION))
     {
       proppatch_target = apr_psprintf(pool, "%s/%ld", session->rev_stub, rev);
     }
Index: subversion/libsvn_ra_serf/options.c
===================================================================
--- subversion/libsvn_ra_serf/options.c (revision 957402)
+++ subversion/libsvn_ra_serf/options.c (working copy)
@@ -332,6 +332,10 @@
                                                  0),
                                   orc->session->pool);
         }
+ else if (svn_cstring_casecmp(key, SVN_DAV_PROTOCOL_VERSION_HEADER) == 0)
+ {
+ orc->session->protocol_version = atoi(val);
+ }
       else if (svn_cstring_casecmp(key, SVN_DAV_ME_RESOURCE_HEADER) == 0)
         {
 #ifdef SVN_DEBUG
@@ -404,7 +408,14 @@
   /* Then see which ones we can discover. */
   serf_bucket_headers_do(hdrs, capabilities_headers_iterator_callback, orc);
 
- /* Execute the 'real' response handler to XML-parse the repsonse body. */
+ /* As a kindness to folks using some 1.7-dev Subversion servers, if
+ we get the "me resource" header without a protocol_version
+ header, we'll pretend we got protocol_version=2. */
+ if ((orc->session->protocol_version < SVN_DAV_MIN_HTTPV2_VERSION)
+ && (orc->session->me_resource != NULL))
+ orc->session->protocol_version = SVN_DAV_MIN_HTTPV2_VERSION;
+
+ /* Execute the 'real' response handler to XML-parse the response body. */
   return svn_ra_serf__handle_xml_parser(request, response,
                                         orc->parser_ctx, pool);
 }
Index: subversion/libsvn_ra_neon/session.c
===================================================================
--- subversion/libsvn_ra_neon/session.c (revision 957402)
+++ subversion/libsvn_ra_neon/session.c (working copy)
@@ -935,6 +935,7 @@
 
   /* Create and fill a session_baton. */
   ras = apr_pcalloc(pool, sizeof(*ras));
+ ras->protocol_version = 1;
   ras->pool = pool;
   ras->url = svn_stringbuf_create(repos_URL, pool);
   /* copies uri pointer members, they get free'd in __close. */
Index: subversion/libsvn_ra_neon/ra_neon.h
===================================================================
--- subversion/libsvn_ra_neon/ra_neon.h (revision 957402)
+++ subversion/libsvn_ra_neon/ra_neon.h (working copy)
@@ -125,33 +125,26 @@
      constants' addresses, therefore). */
   apr_hash_t *capabilities;
 
- /*** HTTP v2 protocol stuff. ***
- *
- * We assume that if mod_dav_svn sends one of the special v2 OPTIONs
- * response headers, it has sent all of them. Specifically, we'll
- * be looking at the presence of the "me resource" as a flag that
- * the server supports v2 of our HTTP protocol.
- */
+ /*** Enhanced HTTP protocol stuff ***/
 
- /* The "me resource". Typically used as a target for REPORTs that
- are path-agnostic. If we have this, we can speak HTTP v2 to the
- server. */
- const char *me_resource;
+ int protocol_version; /* protocol version supported by the server */
+ const char *me_resource; /* resource decribing the repository as a whole
+ (generally used for path-agnostic ops) */
+ const char *rev_stub; /* for accessing revisions (i.e. revprops) */
+ const char *rev_root_stub; /* for accessing REV/PATH pairs */
+ const char *txn_stub; /* for accessing transactions (i.e. txnprops) */
+ const char *txn_root_stub; /* for accessing TXN/PATH pairs */
 
- /* Opaque URL "stubs". If the OPTIONS response returns these, then
- we know we're using HTTP protocol v2. */
- const char *rev_stub; /* for accessing revisions (i.e. revprops) */
- const char *rev_root_stub; /* for accessing REV/PATH pairs */
- const char *txn_stub; /* for accessing transactions (i.e. txnprops) */
- const char *txn_root_stub; /* for accessing TXN/PATH pairs */
+ /*** End Enhanced HTTP stuff ***/
 
- /*** End HTTP v2 stuff ***/
-
 } svn_ra_neon__session_t;
 
-#define SVN_RA_NEON__HAVE_HTTPV2_SUPPORT(ras) ((ras)->me_resource != NULL)
 
+/* Test svn_ra_neon__session_t *R to see if the server to which it is
+ connected supports at least version V of our HTTP protocol. */
+#define SVN_RA_NEON__IS_VERSION_SUPPORTED(r,v) (v >= r->protocol_version)
 
+
 typedef struct {
   ne_request *ne_req; /* neon request structure */
   ne_session *ne_sess; /* neon session structure */
Index: subversion/libsvn_ra_neon/fetch.c
===================================================================
--- subversion/libsvn_ra_neon/fetch.c (revision 957402)
+++ subversion/libsvn_ra_neon/fetch.c (working copy)
@@ -1095,7 +1095,7 @@
   /* If we detected HTTPv2 support, we can fetch the youngest revision
      from a quick OPTIONS request instead of via a batch of
      PROPFINDs. */
- if (SVN_RA_NEON__HAVE_HTTPV2_SUPPORT(ras))
+ if (SVN_RA_NEON__IS_VERSION_SUPPORTED(ras, SVN_DAV_MIN_HTTPV2_VERSION))
     {
       SVN_ERR(svn_ra_neon__exchange_capabilities(ras, latest_revnum, pool));
       if (! SVN_IS_VALID_REVNUM(*latest_revnum))
@@ -1203,7 +1203,7 @@
      have HTTP v2 support available, we can build the URI of that object.
      Otherwise, we have to hunt for a bit. (We pass NULL for 'which_props'
      in these functions because we want 'em all.) */
- if (SVN_RA_NEON__HAVE_HTTPV2_SUPPORT(ras))
+ if (SVN_RA_NEON__IS_VERSION_SUPPORTED(ras, SVN_DAV_MIN_HTTPV2_VERSION))
     {
       const char *url = apr_psprintf(pool, "%s/%ld", ras->rev_stub, rev);
       SVN_ERR(svn_ra_neon__get_props_resource(&bln, ras, url,
@@ -2398,7 +2398,7 @@
   rb->href = MAKE_BUFFER(rb->pool);
 
   /* Got HTTP v2 support? We'll report against the "me resource". */
- if (SVN_RA_NEON__HAVE_HTTPV2_SUPPORT(rb->ras))
+ if (SVN_RA_NEON__IS_VERSION_SUPPORTED(rb->ras, SVN_DAV_MIN_HTTPV2_VERSION))
     {
       report_target = rb->ras->me_resource;
     }
Index: subversion/libsvn_ra_neon/options.c
===================================================================
--- subversion/libsvn_ra_neon/options.c (revision 957402)
+++ subversion/libsvn_ra_neon/options.c (working copy)
@@ -223,10 +223,20 @@
       free(root_uri);
     }
 
- /* HTTP v2 stuff */
+ /* Enhanced HTTP stuff */
+ if ((val = ne_get_response_header(req, SVN_DAV_PROTOCOL_VERSION_HEADER)))
+ {
+ ras->protocol_version = atoi(val);
+ }
   if ((val = ne_get_response_header(req, SVN_DAV_ME_RESOURCE_HEADER)))
     {
       ras->me_resource = apr_pstrdup(ras->pool, val);
+
+ /* As a kindness to folks using some 1.7-dev Subversion servers,
+ if we get the "me resource" header without a protocol_version
+ header, we'll pretend we got protocol_version=2. */
+ if (ras->protocol_version < SVN_DAV_MIN_HTTPV2_VERSION)
+ ras->protocol_version = SVN_DAV_MIN_HTTPV2_VERSION;
     }
   if ((val = ne_get_response_header(req, SVN_DAV_REV_ROOT_STUB_HEADER)))
     {
Index: subversion/libsvn_ra_neon/get_dated_rev.c
===================================================================
--- subversion/libsvn_ra_neon/get_dated_rev.c (revision 957402)
+++ subversion/libsvn_ra_neon/get_dated_rev.c (working copy)
@@ -131,7 +131,7 @@
 
   /* Got HTTP v2 support? We'll report against the "me resource".
      Otherwise, we'll use the VCC. */
- if (SVN_RA_NEON__HAVE_HTTPV2_SUPPORT(ras))
+ if (SVN_RA_NEON__IS_VERSION_SUPPORTED(ras, SVN_DAV_MIN_HTTPV2_VERSION))
     {
       report_target = ras->me_resource;
     }

Received on 2010-06-24 18:42:44 CEST

This is an archived mail posted to the Subversion Dev mailing list.