svn commit: r1654791 - in /subversion/trunk/subversion: mod_dav_svn/liveprops.c tests/cmdline/prop_tests.py

[rh...@apache.org]

Author: rhuijben
Date: Mon Jan 26 13:26:50 2015
New Revision: 1654791

URL: http://svn.apache.org/r1654791
Log:
Fix issue #4415, under the assumption that Subversion clients receive the same
property twice in the same response and only use the properly escaped version.

* subversion/mod_dav_svn/liveprops.c
  (includes): Add svn_ctype.h.
  (insert_prop_internal): Stop producing invalid xml for Subversion clients.
    Provide an non-identical author name when we would fail and document why.

* subversion/tests/cmdline/prop_tests.py
  (xml_unsafe_author): Remove XFail marker. Extend test.

Modified:
    subversion/trunk/subversion/mod_dav_svn/liveprops.c
    subversion/trunk/subversion/tests/cmdline/prop_tests.py

Modified: subversion/trunk/subversion/mod_dav_svn/liveprops.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/liveprops.c?rev=1654791&r1=1654790&r2=1654791&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/liveprops.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/liveprops.c Mon Jan 26 13:26:50 2015
@@ -36,6 +36,7 @@
 #include "svn_time.h"
 #include "svn_dav.h"
 #include "svn_props.h"
+#include "svn_ctype.h"
 
 #include "private/svn_dav_protocol.h"
 
@@ -422,7 +423,43 @@ insert_prop_internal(const dav_resource
         if (last_author == NULL)
           return DAV_PROP_INSERT_NOTDEF;
 
-        value = apr_xml_quote_string(scratch_pool, last_author->data, 1);
+        if (svn_xml_is_xml_safe(last_author->data, last_author->len)
+            || !resource->info->repos->is_svn_client)
+          value = apr_xml_quote_string(scratch_pool, last_author->data, 1);
+        else
+          {
+            /* We are talking to a Subversion client, which will (like any proper
+               xml parser) error out if we produce control characters in XML.
+
+               However Subversion clients process both the generic
+               <creator-displayname /> as the custom element for svn:author.
+
+               Let's skip outputting the invalid characters here to make the XML
+               valid, so clients can see the custom element.
+
+               Subversion Clients will then either use a slightly invalid
+               author (unlikely) or more likely use the second result, which
+               will be transferred with full escaping capabilities.
+
+               We have tests in place to assert proper behavior over the RA layer.
+             */
+            apr_size_t i;
+            svn_stringbuf_t *buf;
+
+            buf = svn_stringbuf_create_from_string(last_author, scratch_pool);
+
+            for (i = 0; i < buf->len; i++)
+              {
+                char c = buf->data[i];
+
+                if (svn_ctype_iscntrl(c))
+                  {
+                    svn_stringbuf_remove(buf, i--, 1);
+                  }
+              }
+
+            value = apr_xml_quote_string(scratch_pool, buf->data, 1);
+          }
         break;
       }
 

Modified: subversion/trunk/subversion/tests/cmdline/prop_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/prop_tests.py?rev=1654791&r1=1654790&r2=1654791&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/prop_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/prop_tests.py Mon Jan 26 13:26:50 2015
@@ -2619,7 +2619,6 @@ def peg_rev_base_working(sbox):
                                      sbox.ospath('iota') + '@BASE')
 
 @Issue(4415)
-@XFail(svntest.main.is_ra_type_dav)
 def xml_unsafe_author(sbox):
   "svn:author with XML unsafe chars"
 
@@ -2646,20 +2645,28 @@ def xml_unsafe_author(sbox):
 
   # mod_dav_svn sends svn:author (via PROPFIND for DAV)
   # Since r1553367 this works correctly on ra_serf, since we now request
-  # a single property value which somehow triggers different behavior
+  # a single property value which skips creating the creator-displayname property
   svntest.actions.run_and_verify_svn(None, ['foo\bbar'], [],
                                      'propget', '--revprop', '-r', '1',
                                      'svn:author', '--strict', wc_dir)
 
+  # Ensure a stable date
+  svntest.actions.run_and_verify_svn(None, None, [],
+                                     'propset', '--revprop', '-r', '1',
+                                     'svn:date', '2015-01-01T00:00:00.0Z', wc_dir)
+
   # But a proplist of this property value still fails via DAV.
-  expected_output = [
+  expected_output = svntest.verify.UnorderedOutput([
     'Unversioned properties on revision 1:\n',
     '  svn:author\n',
+    '    foo\bbar\n',
     '  svn:date\n',
-    '  svn:log\n'
-  ]
+    '    2015-01-01T00:00:00.0Z\n',
+    '  svn:log\n',
+    '    Log message for revision 1.\n'
+  ])
   svntest.actions.run_and_verify_svn(None, expected_output, [],
-                                     'proplist', '--revprop', '-r', '1',
+                                     'proplist', '--revprop', '-r', '1', '-v',
                                      wc_dir)
 
 def dir_prop_conflict_details(sbox):