On 10.12.2012 11:31, Daniel Shahaf wrote: > Branko Čibej wrote on Mon, Dec 10, 2012 at 00:26:20 +0100: >> On 10.12.2012 00:08, Johan Corveleyn wrote: >>> On Sun, Dec 9, 2012 at 11:43 PM, Daniel Shahaf <d...@daniel.shahaf.name> >>> wrote: >>>> Johan Corveleyn wrote on Sun, Dec 09, 2012 at 21:15:24 +0100: >>>>> 2) Am I the only one who wants to protect his repository against this >>>>> corruption? Judging from [1], I don't think so. It doesn't make sense >>>>> that everyone starts writing this pre-commit hook, for something that >>>>> IMHO is an obvious anti-corruption protection. I think every >>>>> repository out there deserves to be protected against this. >>>>> >>>> FWIW, I suggested a hook because you could implement that in about >>>> 5 minutes, whereas doing a C code change would take at least 10 times >>>> that (and several weeks if you have to wait for it to appear in a 1.7.x >>>> release that you can install at $WORK). I won't object to C code >>>> verifying the svn:eol-style invariant. >>> Thanks. And your pre-commit hook example is much appreciated. >>> >>> For the moment I get the impression that it's not really doable / >>> desirable to implement this in the repository. At least until now >>> no-one has suggested how it could be done, and I don't know enough >>> myself about the server-side / back-end to figure it out :-). >> The first obstacle is that the server does not interpret properties.[1] >> Therefore, you'd have to implement this check at transaction-commit >> time, because there's no earlier point where you're guaranteed to have >> all node properties at their final values. This implies that the time to >> reject a commit would be proportional to the size of the commit (which >> isn't the case when it comes to conflict detection). > Can't you do what stefan2 suggested, but in libsvn_repos? The repos > layer commit editor would remember for each file whether its textdelta > has added a new 0x0D byte, then at close_edit() it would iterate all > files in the commit --- and for each file only compare its svn:eol-style > property to the by-now-precomputed "did it it contain a 0x0D" bit. > > I'm not sure how efficient it would be to parse for 0x0D's in > libsvn_repos, though. Maybe we should make this optional.
It's not enough to just look for \r in the delta stream. Certainly wouldn't help with historically broken files. Moreover, if libsvn_repos started looking at svn:eol-style, that'd only make sense if you verified all normalizations, not just "native". And why should it just be svn:eol-style, when svn:keywords has potentially the same problem? Eventually you start verifying everything that affects file contents. Maybe that's a good idea, but /why/ put it in libsvn_repos when it's much easier and cleaner to just provide some standard hooks, written in C and distributed with releases, that the admin plug in if she feels like it? Surely that's what hooks are for. -- Brane -- Branko Čibej Director of Subversion | WANdisco | www.wandisco.com
