[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] check name svn special properties

From: <kfogel_at_collab.net>
Date: 2005-06-22 22:34:11 CEST

Fabien COELHO <fabien@coelho.net> writes:
> Dear SVN developpers,
>
> Please find attached a patch which checks the name of svn special
> properties and refuses unexpected ones. The check are performed in the
> client library, where it seems to belong. The error can be overridden
> with --force if necessary.
>
> The new feature is tested in the regression stuff. I had to remove 6
> lines of some tests which were setting invalid svn special properties,
> which are
> now forbidden by default.

Absence of log message makes this patch very hard to review.

What happens when an older client has to commit a newly-invented
property? Say, after this patch is applied, we later invent a new
property "svn:foobar", and someone wants to use that without upgrading
their client? I think having to use --force would be annoying...

-Karl

> Index: subversion/include/svn_props.h
> ===================================================================
> --- subversion/include/svn_props.h (revision 15138)
> +++ subversion/include/svn_props.h (working copy)
> @@ -308,6 +308,19 @@
> SVN_PROP_REVISION_AUTOVERSIONED, \
> SVN_PROP_REVISION_ORIG_DATE,
>
> +
> +/**
> + * This is a list of all svn properties (i.e. svn:* stuff)
> + */
> +#define SVN_PROP_ALL_PROPS SVN_PROP_REVISION_ALL_PROPS \
> + SVN_PROP_MIME_TYPE, \
> + SVN_PROP_IGNORE, \
> + SVN_PROP_EOL_STYLE, \
> + SVN_PROP_KEYWORDS, \
> + SVN_PROP_EXECUTABLE, \
> + SVN_PROP_NEEDS_LOCK, \
> + SVN_PROP_EXTERNALS
> +
> /** @} */
>
>
> Index: subversion/libsvn_client/prop_commands.c
> ===================================================================
> --- subversion/libsvn_client/prop_commands.c (revision 15138)
> +++ subversion/libsvn_client/prop_commands.c (working copy)
> @@ -91,7 +91,29 @@
> return FALSE;
> }
>
> +/* Check whether NAME is an allowed svn:* property name.
> + *
> + * Return TRUE if it is.
> + * Return FALSE if it is not.
> + */
> +static svn_boolean_t
> +is_svn_prop_name (const char *name)
> +{
> + apr_size_t i;
> + const char *svn_props[] =
> + {
> + SVN_PROP_ALL_PROPS
> + };
>
> + for (i = 0; i < sizeof (svn_props) / sizeof (svn_props[0]); i++)
> + {
> + if (strcmp (name, svn_props[i]) == 0)
> + return TRUE;
> + }
> + return FALSE;
> +}
> +
> +
> /* Return an SVN_ERR_CLIENT_PROPERTY_NAME error if NAME is a wcprop,
> else return SVN_NO_ERROR. */
> static svn_error_t *
> @@ -204,6 +226,14 @@
> return svn_error_createf (SVN_ERR_CLIENT_PROPERTY_NAME, NULL,
> _("Bad property name: '%s'"), propname);
>
> + if (! skip_checks &&
> + strncmp(SVN_PROP_PREFIX, propname, strlen(SVN_PROP_PREFIX))==0 &&
> + ! is_svn_prop_name(propname))
> + return svn_error_createf
> + (SVN_ERR_CLIENT_PROPERTY_NAME, NULL,
> + _("Unexpected svn property name: '%s', use --force to override"),
> + propname);
> +
> SVN_ERR (svn_wc_adm_probe_open3 (&adm_access, NULL, target, TRUE,
> recurse ? -1 : 0, ctx->cancel_func,
> ctx->cancel_baton, pool));
> Index: subversion/tests/clients/cmdline/prop_tests.py
> ===================================================================
> --- subversion/tests/clients/cmdline/prop_tests.py (revision 15138)
> +++ subversion/tests/clients/cmdline/prop_tests.py (working copy)
> @@ -560,6 +560,13 @@
> 'Thu Jan 1 01:00:00 1970',
> iota_path)
>
> +
> + # mispelled svn special properties are rejected
> + svntest.actions.run_and_verify_svn('Unexpected svn property', None,
> + svntest.SVNAnyOutput, 'propset',
> + 'svn:bad-name', '1',
> + iota_path)
> +
> # Status unchanged
> svntest.actions.run_and_verify_status(wc_dir, expected_status)
>
> @@ -567,7 +574,14 @@
> svntest.actions.run_and_verify_svn(None, None, [], 'propset', '-R',
> 'svn:executable', 'on', E_path)
>
> - expected_status.tweak('A/B/E/alpha', 'A/B/E/beta', status=' M')
> + # mispelled svn special properties can be forced
> + mu_path = os.path.join(wc_dir, 'A', 'mu')
> + svntest.actions.run_and_verify_svn(None, None, None,
> + 'propset', '--force',
> + 'svn:misc-name', '1',
> + mu_path)
> +
> + expected_status.tweak('A/B/E/alpha', 'A/B/E/beta', 'A/mu', status=' M')
> svntest.actions.run_and_verify_status(wc_dir, expected_status)
>
> # Issue #920. Don't allow setting of svn:eol-style on binary files or files
> @@ -809,9 +823,9 @@
> set_prop('svn:executable', ' ', mu_path)
>
> # Anything else should be untouched
> - set_prop('svn:some-prop', 'bar', lambda_path)
> - set_prop('svn:some-prop', ' bar baz', mu_path)
> - set_prop('svn:some-prop', 'bar\n', iota_path)
> + #set_prop('svn:some-prop', 'bar', lambda_path)
> + #set_prop('svn:some-prop', ' bar baz', mu_path)
> + #set_prop('svn:some-prop', 'bar\n', iota_path)
> set_prop('some-prop', 'bar', lambda_path)
> set_prop('some-prop', ' bar baz', mu_path)
> set_prop('some-prop', 'bar\n', iota_path)
> @@ -857,9 +871,9 @@
> check_prop('svn:executable', mu_path, ['*'])
>
> # Check other props
> - check_prop('svn:some-prop', lambda_path, ['bar'])
> - check_prop('svn:some-prop', mu_path, [' bar baz'])
> - check_prop('svn:some-prop', iota_path, ['bar'+os.linesep])
> + #check_prop('svn:some-prop', lambda_path, ['bar'])
> + #check_prop('svn:some-prop', mu_path, [' bar baz'])
> + #check_prop('svn:some-prop', iota_path, ['bar'+os.linesep])
> check_prop('some-prop', lambda_path, ['bar'])
> check_prop('some-prop', mu_path,[' bar baz'])
> check_prop('some-prop', iota_path, ['bar\n'])
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jun 22 23:40:20 2005

This is an archived mail posted to the Subversion Dev mailing list.