Stefan Sperling wrote on Wed, Mar 27, 2013 at 16:03:32 +0100:
> 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.
>
In the repos commit editor?
> > 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).
That's surprising. The changed-paths list is sensitive to whitespace, a
trailing \n should cause 3 \n's rather than 2 and I would expect the
parser to choke on that.
In the cpath case it's the next-to-last header so copyfrom:, copyroot:,
is-fresh-txn-root:, and minfo-cnt: would be missing. I guess their
absence doesn't affect verification.
Received on 2013-03-27 16:12:02 CET