On Tue, Mar 26, 2013 at 06:42:27PM +0100, Stefan Sperling wrote:
> On Tue, Mar 26, 2013 at 12:40:56PM -0400, C. Michael Pilato wrote:
> > Certainly, libsvn_repos should be disallowing newline-bearing paths. We
> > weren't able to support these paths when our .svn/entries file was
> > plaintext, we've never been able to support them in our dump format (which
> > is a libsvn_repos construct), and so on.
> >
> > I'm not so sure about libsvn_fs. BDB should be able to handle them just
> > fine, but it sounds like FSFS can't. Do we just make a post-facto
> > declaration that, going forward, we won't try to support these paths in the
> > FS layer?
>
> I don't really see how supporting such paths in the FS layer makes
> sense if none of the other layers can deal with them.
>
> Are there any other tools built on top of our FS layer that rely on
> being able to use newlines in filenames?
>
> But still, I was planning on making the repos layer enforce restrictions
> on filenames anyway to avoid having each filesystem deal with it separately.
I think we'll have to block these paths at the FS layer.
It looks like the repos layer cannot perform this kind of verification.
When I use the FS API to add paths with trailing \n to a transaction,
the FS layer accepts them, and silently write out a corrupt changed
path list (in case of FSFS):
================
$ cat test-filename-trailing-newline/db/transactions/1-1.txn/changes
0-1._0.t1-1 add-dir false false /bar
1 /foo
================
When I try to make the repos layer iterate over changed paths to
perform verification, svn_fs_paths_changed2() returns an error:
(gdb) p *err->child->child->child->child
$8 = {apr_err = 160004,
message = 0xef7143880f0 "Invalid changes line in rev-file", child = 0x0,
pool = 0xef714388028,
file = 0xef71a7d6538 "subversion/libsvn_fs_fs/fs_fs.c", line = 6024}
I'd say we cannot allow the FS API to break the FS like that.
Let's face it, the FS API promise people are upholding in this discussion
has been broken for years.
Received on 2013-03-27 13:36:15 CET