s...@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", >
