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

Re: [PATCH] Issue #2748: non-UTF-8 filenames in the repository

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Wed, 17 Sep 2008 16:46:38 +0300 (Jerusalem Daylight Time)

Stefan Sperling wrote on Wed, 17 Sep 2008 at 15:17 +0200:
> On Wed, Sep 17, 2008 at 03:42:21PM +0300, Daniel Shahaf wrote:
> > Patch for issue #2748 ("clients can create non UTF-8 filenames in the
> > repository"). It prevents non-UTF-8 paths from entering the repository,
> > but it doesn't affect existing files in existing repositories, svnsync,
> > or 'svnadmin load'.
> >
> > I'm running 'make check' now, and I'll commit later today or tomorrow if
> > I don't hear anything.
>
> Drive-by question:
>
> I thought we always handle strings in UTF-8 internally.
> So why not just make svn_path_check_valid check for UTF-8 by default?
>

My original reason was that the current contract actively promises that
it *won't* check validity:

     * ASSUMPTION: @a path is a valid UTF-8 string. This function does
     * not check UTF-8 validity.

> Is it because the function may be fed input from existing repositories
> which isn't valid UTF-8?
>

Well, yes, being nice to existing invalid filenames is a consideration.

Another is that this function is used all over the code -- including by
svn_wc_add3(), svn_fs_make_file(), and others. Changing the semantics
in all these places may have unexpected side effects.

    include/svn_path.h|449 col 1| svn_path_check_valid2(const char *path, svn_boolean_t assume_utf8,
    include/svn_path.h|453 col 15| * Similar to svn_path_check_valid2(), but with @a assume_utf8 set to @c TRUE.
    include/svn_path.h|459 col 1| svn_path_check_valid(const char *path, apr_pool_t *pool);
    libsvn_client/add.c|592 col 11| SVN_ERR(svn_path_check_valid(path, pool));
    libsvn_client/commit.c|188 col 11| SVN_ERR(svn_path_check_valid(path, pool));
    libsvn_client/commit.c|294 col 11| SVN_ERR(svn_path_check_valid(path, pool));
    libsvn_client/copy.c|640 col 15| SVN_ERR(svn_path_check_valid(path, pool));
    libsvn_fs/fs-loader.c|869 col 11| SVN_ERR(svn_path_check_valid(path, pool));
    libsvn_fs/fs-loader.c|883 col 11| SVN_ERR(svn_path_check_valid(to_path, pool));
    libsvn_fs/fs-loader.c|953 col 11| SVN_ERR(svn_path_check_valid(path, pool));
    libsvn_repos/commit.c|299 col 11| SVN_ERR(svn_path_check_valid2(path, FALSE, pool));
    libsvn_repos/commit.c|447 col 11| SVN_ERR(svn_path_check_valid2(path, FALSE, pool));
    libsvn_subr/path.c|1465 col 1| svn_path_check_valid2(const char *path, svn_boolean_t assume_utf8,
    libsvn_subr/path.c|1495 col 1| svn_path_check_valid(const char *path, apr_pool_t *pool)
    libsvn_subr/path.c|1497 col 10| return svn_path_check_valid2(path, TRUE, pool);
    libsvn_wc/adm_ops.c|1395 col 11| SVN_ERR(svn_path_check_valid(path, pool));
    tests/libsvn_subr/path-test.c|893 col 16| *msg = "test svn_path_check_valid";
    tests/libsvn_subr/path-test.c|900 col 26| svn_error_t *err = svn_path_check_valid(tests[i].path, pool);
    tests/libsvn_subr/path-test.c|907 col 13| "svn_path_check_valid (%s) returned %s instead of %s",

Daniel

> Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-09-17 15:47:02 CEST

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