Philip Martin wrote on Fri, Jul 08, 2011 at 20:39:02 +0100:
> Daniel Shahaf <[email protected]> writes:
>
> > My point wasn't the validation of svn:mergeinfo revprops, it was whether
> > the validation of svn:mergeinfo nodeprops should happen in
> > svn_repos__validate_prop().
> >
> > I'm not against telling that function whether it's validating a nodeprop
> > or a revprop, and applying different logic in either case.
>
> Is there any common validation between node props and rev props? If the
> validation is completely different it should be two functions rather
> than one function with a boolean flag.
>
These two hunks seem to be generic:
/* Disallow setting non-regular properties. */
if (kind != svn_prop_regular_kind)
return svn_error_createf
(SVN_ERR_REPOS_BAD_ARGS, NULL,
_("Storage of non-regular property '%s' is disallowed through the "
"repository interface, and could indicate a bug in your client"),
name);
/* Validate that translated props (e.g., svn:log) are UTF-8 with
* LF line endings. */
if (svn_prop_needs_translation(name))
{
if (svn_utf__is_valid(value->data, value->len) == FALSE)
{
return svn_error_createf
(SVN_ERR_BAD_PROPERTY_VALUE, NULL,
_("Cannot accept '%s' property because it is not encoded in "
"UTF-8"), name);
}
/* Disallow inconsistent line ending style, by simply looking for
* carriage return characters ('\r'). */
if (strchr(value->data, '\r') != NULL)
{
return svn_error_createf
(SVN_ERR_BAD_PROPERTY_VALUE, NULL,
_("Cannot accept non-LF line endings in '%s' property"),
name);
}
}
But, might as well split those two to a helper function that both
__validate_revprop() and __validate_nodeprop() call.
In short: not too worried. Either way is fine for now (these ARE
private API's); let's get it done rather than debate the form.
> --
> uberSVN: Apache Subversion Made Easy
> http://www.uberSVN.com
