Perhaps the better change is to make svn_repos__validate_prop() early-out
(with success, of course) on a NULL value and adjust the rest of the code
therein accordingly.  I can see the benefit in flagging the *addition* (or
modification) of a non-regular property (which is the first test that
function performs), but there's need to block the ability to cleanup after
such an errant change has already occurred.  And we don't have any
properties which are required to be present.  So, again, I say there's no
need to validate any type of property deletion.
--- Begin Message ---
Hi,

Properties are validated regardless of the action on the property. This poses a problem when it comes to very old repositories that contain "invalid" properties committed by very old clients that didn't perform these validations. My contention is that we need to check the validity of the property only during addition and modification. Validation during deletion prevents the user from removing what he recognizes as an invalid property.

Regards,
Arwin Arni
* subversion/libsvn_repos/fs-wrap.c
  (svn_repos_fs_change_rev_prop4): Don't validate properties during
                                   deletion.

Patch by Arwin Arni <arwin{_AT_}collab.net>
Index: subversion/libsvn_repos/fs-wrap.c
===================================================================
--- subversion/libsvn_repos/fs-wrap.c   (revision 1494892)
+++ subversion/libsvn_repos/fs-wrap.c   (working copy)
@@ -331,7 +331,12 @@
       char action;
       apr_hash_t *hooks_env;
 
-      SVN_ERR(svn_repos__validate_prop(name, new_value, pool));
+      /* We should validate properties only for additions and 
+         modifications. NEW_VALUE exists in both these cases. */
+      if (new_value)
+        {
+          SVN_ERR(svn_repos__validate_prop(name, new_value, pool));
+        }
 
       /* Fetch OLD_VALUE for svn_fs_change_rev_prop2(). */
       if (old_value_p)

--- End Message ---

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to