Re: svn commit: r1144316 - in /subversion/trunk/subversion/libsvn_repos: commit.c fs-wrap.c

Daniel Shahaf <d.s_at_daniel.shahaf.name> writes:

> philip_at_apache.org wrote on Fri, Jul 08, 2011 at 14:02:42 -0000:
>> Author: philip
>> Date: Fri Jul 8 14:02:42 2011
>> New Revision: 1144316
>>
>> URL: http://svn.apache.org/viewvc?rev=1144316&view=rev
>> Log:
>> Fix issue 3953, mod_dav_svn failing to reject bogus mergeinfo on commit.
>> Move server-side mergeinfo validation so that all RA layers use it.
>>
> ...
>> @@ -222,6 +255,9 @@ svn_repos_fs_change_node_prop(svn_fs_roo
>> const svn_string_t *value,
>> apr_pool_t *pool)
>> {
>> + if (value && strcmp(name, SVN_PROP_MERGEINFO) == 0)
>> + SVN_ERR(verify_mergeinfo(value, path, pool));
>> +
>> /* Validate the property, then call the wrapped function. */
>> SVN_ERR(svn_repos__validate_prop(name, value, pool));
>> return svn_fs_change_node_prop(root, path, name, value, pool);
>>
>
> Shouldn't the call to verify_mergeinfo() be made by
> svn_repos__validate_prop(), rather than here?

Perhaps. svn_repos__validate_prop is also used by
svn_repos_fs_change_txn_props, is it appropriate to validate mergeinfo
there?

We should probably split the validate function into two, one for node
props and one for revprops.

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com