Re: Problems with commit feature negotiation and write-through proxies

Evgeny Kotkov <evgeny.kotkov_at_visualsvn.com> writes:

> Philip Martin <philip_at_codematters.co.uk> writes:
>
>> In the past we supported different versions on the master and slave and
>> we introduced the SVNMasterVersion directive to configure it.
>> Unfortunately that information is not immediately available in
>> get_vsn_options() where the server advertises some commit features. If
>> I simply remove the SVN_DAV_NS_DAV_SVN_SVNDIFF2 header from
>> get_vsn_options() then I can commit.
>>
>> I'm not sure how we fix this. Do we delay the svndiff2 negotiation
>> until later in the commit? Do we abandon mixed master/slave versions?
>
> I think that we might be able to selectively stop advertising the new-
> in-1.10 capabilities based on the SVNMasterVersion value, similarly to
> how we handle SVN_DAV_NS_DAV_SVN_EPHEMERAL_TXNPROPS.
>
> Perhaps, we could do something along the lines of the attached patch?

We already have system for handling create-txn and create-txn-with-props
so I was thinking of extending that code:

Index: subversion/mod_dav_svn/version.c
===================================================================
--- subversion/mod_dav_svn/version.c (revision 1820704)
+++ subversion/mod_dav_svn/version.c (working copy)
@@ -152,9 +152,6 @@ get_vsn_options(apr_pool_t *p, apr_text_header *ph
   apr_text_append(p, phdr, SVN_DAV_NS_DAV_SVN_INHERITED_PROPS);
   apr_text_append(p, phdr, SVN_DAV_NS_DAV_SVN_INLINE_PROPS);
   apr_text_append(p, phdr, SVN_DAV_NS_DAV_SVN_REVERSE_FILE_REVS);
- apr_text_append(p, phdr, SVN_DAV_NS_DAV_SVN_SVNDIFF1);
- apr_text_append(p, phdr, SVN_DAV_NS_DAV_SVN_SVNDIFF2);
- apr_text_append(p, phdr, SVN_DAV_NS_DAV_SVN_PUT_RESULT_CHECKSUM);
   apr_text_append(p, phdr, SVN_DAV_NS_DAV_SVN_LIST);
   /* Mergeinfo is a special case: here we merely say that the server
    * knows how to handle mergeinfo -- whether the repository does too
@@ -297,6 +294,19 @@ get_option(const dav_resource *resource,
         { "create-txn-with-props", { 1, 8, 0, "" } },
       };
 
+ /* These capabilities are used during commit and when acting as
+ a WebDAV slave (SVNMasterURI) their availablity depends on
+ the master version (SVNMasterVersion) rather than our own
+ (slave) version. */
+ struct capability_versions_t {
+ const char *capability_name;
+ svn_version_t min_version;
+ } capabilities[] = {
+ { SVN_DAV_NS_DAV_SVN_SVNDIFF1, { 1, 7, 0, ""} },
+ { SVN_DAV_NS_DAV_SVN_SVNDIFF2, { 1, 10, 0, ""} },
+ { SVN_DAV_NS_DAV_SVN_PUT_RESULT_CHECKSUM, { 1, 10, 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",
@@ -347,6 +357,21 @@ get_option(const dav_resource *resource,
           apr_table_addn(r->headers_out, SVN_DAV_SUPPORTED_POSTS_HEADER,
                          apr_pstrdup(r->pool, posts_versions[i].post_name));
         }
+
+ /* Report the capabilites that rely on the master version. */
+ for (i = 0; i < sizeof(capabilities)/sizeof(capabilities[0]); ++i)
+ {
+ if (master_version
+ && (!svn_version__at_least(master_version,
+ capabilities[i].min_version.major,
+ capabilities[i].min_version.minor,
+ capabilities[i].min_version.patch)))
+ continue;
+
+ apr_table_addn(r->headers_out, "DAV",
+ capabilities[i].capability_name);
+
+ }
     }
 
   return NULL;

-- 
Philip