stsp_at_apache.org wrote on Tue, May 31, 2011 at 12:23:05 -0000:
> Author: stsp
> Date: Tue May 31 12:23:05 2011
> New Revision: 1129641
>
> URL: http://svn.apache.org/viewvc?rev=1129641&view=rev
> Log:
> Make 'svnadmin verify' error out if an invalid path is found in the repository.
>
> There have been reports of non-UTF-8 paths having entered repositories,
> probably due to buggy third-party clients running against pre-1.6 servers
> (pre-1.6 servers do not verify filename encoding).
> See this thread for one such report, which also mentions that
> 'svnadmin verify' didn't detect this problem:
> http://svn.haxx.se/users/archive-2011-05/0361.shtml
>
> * subversion/include/private/svn_fs_util.h
> (svn_fs__path_valid): Declare.
>
> * subversion/libsvn_fs/fs-loader.c
> (path_valid): Rename this funcion to ...
> (svn_fs__path_valid): ... this, making it available to the repos layer.
> (svn_fs_make_dir, svn_fs_copy, svn_fs_make_file): Update callers.
>
> * subversion/tests/cmdline/svnadmin_tests.py
> (verify_non_utf8_paths): New test which makes sure that 'svnadmin verify'
> will error out on non-UTF-8 paths. It also makes sure that the repository
> can still be dumped successfully so that the problem can be fixed by
> editing the dumpfile. This test is FSFS-only for now but that shouldn't
> be a problem.
>
> * subversion/libsvn_repos/dump.c
> (dump_node): If verifying, run the node's path through svn_fs__path_valid().
>
> Modified:
> subversion/trunk/subversion/include/private/svn_fs_util.h
> subversion/trunk/subversion/libsvn_fs/fs-loader.c
> subversion/trunk/subversion/libsvn_repos/dump.c
> subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py
>
> Modified: subversion/trunk/subversion/include/private/svn_fs_util.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_fs_util.h?rev=1129641&r1=1129640&r2=1129641&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/private/svn_fs_util.h (original)
> +++ subversion/trunk/subversion/include/private/svn_fs_util.h Tue May 31 12:23:05 2011
> @@ -186,6 +186,14 @@ svn_fs__path_change_create_internal(cons
> svn_fs_path_change_kind_t change_kind,
> apr_pool_t *pool);
>
> +/* Check whether PATH is valid for a filesystem, following (most of) the
> + * requirements in svn_fs.h:"Directory entry names and directory paths".
> + *
> + * Return SVN_ERR_FS_PATH_SYNTAX if PATH is not valid.
> + */
> +svn_error_t *
> +svn_fs__path_valid(const char *path, apr_pool_t *pool);
> +
> #ifdef __cplusplus
> }
> #endif /* __cplusplus */
>
> Modified: subversion/trunk/subversion/libsvn_fs/fs-loader.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs/fs-loader.c?rev=1129641&r1=1129640&r2=1129641&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_fs/fs-loader.c (original)
> +++ subversion/trunk/subversion/libsvn_fs/fs-loader.c Tue May 31 12:23:05 2011
> @@ -334,13 +334,8 @@ default_warning_func(void *baton, svn_er
> SVN_ERR_MALFUNCTION_NO_RETURN();
> }
>
> -/* Check whether PATH is valid for a filesystem, following (most of) the
> - * requirements in svn_fs.h:"Directory entry names and directory paths".
> - *
> - * Return SVN_ERR_FS_PATH_SYNTAX if PATH is not valid.
> - */
> -static svn_error_t *
> -path_valid(const char *path, apr_pool_t *pool)
> +svn_error_t *
> +svn_fs__path_valid(const char *path, apr_pool_t *pool)
> {
> /* UTF-8 encoded string without NULs. */
> if (! svn_utf__cstring_is_valid(path))
> @@ -1044,7 +1039,7 @@ svn_fs_dir_entries(apr_hash_t **entries_
> svn_error_t *
> svn_fs_make_dir(svn_fs_root_t *root, const char *path, apr_pool_t *pool)
> {
> - SVN_ERR(path_valid(path, pool));
> + SVN_ERR(svn_fs__path_valid(path, pool));
> return svn_error_return(root->vtable->make_dir(root, path, pool));
> }
>
> @@ -1058,7 +1053,7 @@ svn_error_t *
> svn_fs_copy(svn_fs_root_t *from_root, const char *from_path,
> svn_fs_root_t *to_root, const char *to_path, apr_pool_t *pool)
> {
> - SVN_ERR(path_valid(to_path, pool));
> + SVN_ERR(svn_fs__path_valid(to_path, pool));
> return svn_error_return(to_root->vtable->copy(from_root, from_path,
> to_root, to_path, pool));
> }
> @@ -1131,7 +1126,7 @@ svn_fs_file_contents(svn_stream_t **cont
> svn_error_t *
> svn_fs_make_file(svn_fs_root_t *root, const char *path, apr_pool_t *pool)
> {
> - SVN_ERR(path_valid(path, pool));
> + SVN_ERR(svn_fs__path_valid(path, pool));
> return svn_error_return(root->vtable->make_file(root, path, pool));
> }
>
>
> Modified: subversion/trunk/subversion/libsvn_repos/dump.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/dump.c?rev=1129641&r1=1129640&r2=1129641&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_repos/dump.c (original)
> +++ subversion/trunk/subversion/libsvn_repos/dump.c Tue May 31 12:23:05 2011
> @@ -36,6 +36,7 @@
> #include "svn_props.h"
>
> #include "private/svn_mergeinfo_private.h"
> +#include "private/svn_fs_util.h"
>
> #define ARE_VALID_COPY_ARGS(p,r) ((p) && SVN_IS_VALID_REVNUM(r))
>
> @@ -242,6 +243,10 @@ dump_node(struct edit_baton *eb,
> svn_fs_root_t *compare_root = NULL;
> apr_file_t *delta_file = NULL;
>
> + /* If we're verifying, validate the path. */
> + if (eb->verify)
> + SVN_ERR(svn_fs__path_valid(path, pool));
> +
Can we print a warning to stderr if eb->verify == FALSE ?
(we have a notify_func we can use)
> /* Write out metadata headers for this file node. */
> SVN_ERR(svn_stream_printf(eb->stream, pool,
> SVN_REPOS_DUMPFILE_NODE_PATH ": %s\n",
>
Received on 2011-06-10 23:02:04 CEST