On Wed, Mar 27, 2013 at 10:52:04AM -0400, C. Michael Pilato wrote:
> > I think we'll have to block these paths at the FS layer.
>
> If FSFS is fundamentally unable to support these types of paths, then sure,
> let's go ahead and protect against the failure at that level. But please
> don't overreach here -- block only the paths that FSFS simply cannot deal
> with. There have been other tools built atop the FS layer in the past
> (wikis, etc.) and could be in the future -- this is, after all, why we have
> distinct FS and repos APIs -- and we shouldn't be artificially limiting what
> folks can do with the that API.
That's fine. The fix I've committed and proposed for backport applies
the large hammer and blocks any control characters. If there's a case
to be made for relaxing this check I'm happy to do that.
But I think that if we do so, we also need to find a way to make the repos
layer perform this check as well. Subversion clients have always refused
paths which contain control characters, and it makes sense to have
consistent checks at the client and server level, as far as Subversion
is concerned.
Perhaps my idea of using repos_commit_txn() was bad because paths have
already been handed off to the FS layer at that point. It should be
possible to add the check elsewhere in the repos layer, before the FS
gets involved.
> As an aside, though, we keep talking about paths with trailing newlines. Is
> it only *trailing* newlines that cause the problem here? I would think that
> newlines appearing anywhere in the path could be problematic in at least
> some of the same places we've discussed on this thread thus far.
It's just that I've observed the trailing newlines problem in the wild.
And it's indeed a particularly nasty case since 'svnadmin verify' doesn't
complain about it as much as I believe it would when faced with a newline
somewhere in the middle of a filename (the cpath: and the changed-paths
list would also be corrupt in that case, but aren't with a trailing \n).
Received on 2013-03-27 16:04:15 CET