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.
