You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2011/05/22 17:26:10 UTC
Re: svn commit: r1125983 -
/subversion/trunk/subversion/libsvn_repos/commit.c
stsp@apache.org wrote on Sun, May 22, 2011 at 15:11:53 -0000:
> Author: stsp
> Date: Sun May 22 15:11:52 2011
> New Revision: 1125983
>
> URL: http://svn.apache.org/viewvc?rev=1125983&view=rev
> Log:
> Fix issue #3895, "repos layer does not verify mergeinfo syntax"
>
> * subversion/libsvn_repos/commit.c
> (verify_mergeinfo): New helper function.
> (change_file_prop, change_dir_prop): Use new helper to verify svn:mergeinfo
> property values. Reject the commit if verification fails.
>
> Modified:
> subversion/trunk/subversion/libsvn_repos/commit.c
>
> Modified: subversion/trunk/subversion/libsvn_repos/commit.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/commit.c?rev=1125983&r1=1125982&r2=1125983&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_repos/commit.c (original)
> +++ subversion/trunk/subversion/libsvn_repos/commit.c Sun May 22 15:11:52 2011
> @@ -36,6 +36,7 @@
> #include "svn_repos.h"
> #include "svn_checksum.h"
> #include "svn_props.h"
> +#include "svn_mergeinfo.h"
> #include "repos.h"
> #include "svn_private_config.h"
> #include "private/svn_fspath.h"
> @@ -530,6 +531,27 @@ open_file(const char *path,
> }
>
>
> +/* Verify the mergeinfo property value VALUE and return an error if it
> + * is invalid. Use SCRATCH_POOL for temporary allocations. */
> +static svn_error_t *
> +verify_mergeinfo(const svn_string_t *value, apr_pool_t *scratch_pool)
> +{
> + svn_error_t *err;
> + svn_mergeinfo_t mergeinfo;
> +
> + /* Mergeinfo is UTF-8 encoded so the number of bytes returned by strlen()
> + * should match VALUE->LEN. Prevents trailing garbage in the property. */
> + if (strlen(value->data) != value->len)
> + return svn_error_quick_wrap(err, _("Commit rejected because mergeinfo "
> + "contains unexpected string terminator"));
> +
> + err = svn_mergeinfo_parse(&mergeinfo, value->data, scratch_pool);
> + if (err)
> + return svn_error_quick_wrap(err, _("Commit rejected because of mergeinfo "
> + "syntax error"));
Can we say here "a mergeinfo syntax error on path %s"?
> + return SVN_NO_ERROR;
> +}
> +
>
> static svn_error_t *
> change_file_prop(void *file_baton,
> @@ -544,6 +566,9 @@ change_file_prop(void *file_baton,
> SVN_ERR(check_authz(eb, fb->path, eb->txn_root,
> svn_authz_write, pool));
>
> + if (value && strcmp(name, SVN_PROP_MERGEINFO) == 0)
> + SVN_ERR(verify_mergeinfo(value, pool));
> +
> return svn_repos_fs_change_node_prop(eb->txn_root, fb->path,
> name, value, pool);
> }
> @@ -602,6 +627,9 @@ change_dir_prop(void *dir_baton,
> return out_of_date(db->path, svn_node_dir);
> }
>
> + if (value && strcmp(name, SVN_PROP_MERGEINFO) == 0)
> + SVN_ERR(verify_mergeinfo(value, pool));
> +
> return svn_repos_fs_change_node_prop(eb->txn_root, db->path,
> name, value, pool);
> }
>
>
Re: svn commit: r1125983 -
/subversion/trunk/subversion/libsvn_repos/commit.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
r1125998.
Stefan Sperling wrote on Sun, May 22, 2011 at 17:29:08 +0200:
> On Sun, May 22, 2011 at 06:26:10PM +0300, Daniel Shahaf wrote:
> > stsp@apache.org wrote on Sun, May 22, 2011 at 15:11:53 -0000:
> > > +/* Verify the mergeinfo property value VALUE and return an error if it
> > > + * is invalid. Use SCRATCH_POOL for temporary allocations. */
> > > +static svn_error_t *
> > > +verify_mergeinfo(const svn_string_t *value, apr_pool_t *scratch_pool)
> > > +{
> > > + svn_error_t *err;
> > > + svn_mergeinfo_t mergeinfo;
> > > +
> > > + /* Mergeinfo is UTF-8 encoded so the number of bytes returned by strlen()
> > > + * should match VALUE->LEN. Prevents trailing garbage in the property. */
> > > + if (strlen(value->data) != value->len)
> > > + return svn_error_quick_wrap(err, _("Commit rejected because mergeinfo "
> > > + "contains unexpected string terminator"));
> > > +
> > > + err = svn_mergeinfo_parse(&mergeinfo, value->data, scratch_pool);
> > > + if (err)
> > > + return svn_error_quick_wrap(err, _("Commit rejected because of mergeinfo "
> > > + "syntax error"));
> >
> > Can we say here "a mergeinfo syntax error on path %s"?
>
> Good idea. I think we'd simply have to pass fb->path to verify_mergeinfo().
>
> Wanna take a shot at it? :)
Re: svn commit: r1125983 -
/subversion/trunk/subversion/libsvn_repos/commit.c
Posted by Stefan Sperling <st...@elego.de>.
On Sun, May 22, 2011 at 06:26:10PM +0300, Daniel Shahaf wrote:
> stsp@apache.org wrote on Sun, May 22, 2011 at 15:11:53 -0000:
> > +/* Verify the mergeinfo property value VALUE and return an error if it
> > + * is invalid. Use SCRATCH_POOL for temporary allocations. */
> > +static svn_error_t *
> > +verify_mergeinfo(const svn_string_t *value, apr_pool_t *scratch_pool)
> > +{
> > + svn_error_t *err;
> > + svn_mergeinfo_t mergeinfo;
> > +
> > + /* Mergeinfo is UTF-8 encoded so the number of bytes returned by strlen()
> > + * should match VALUE->LEN. Prevents trailing garbage in the property. */
> > + if (strlen(value->data) != value->len)
> > + return svn_error_quick_wrap(err, _("Commit rejected because mergeinfo "
> > + "contains unexpected string terminator"));
> > +
> > + err = svn_mergeinfo_parse(&mergeinfo, value->data, scratch_pool);
> > + if (err)
> > + return svn_error_quick_wrap(err, _("Commit rejected because of mergeinfo "
> > + "syntax error"));
>
> Can we say here "a mergeinfo syntax error on path %s"?
Good idea. I think we'd simply have to pass fb->path to verify_mergeinfo().
Wanna take a shot at it? :)