On 2/25/14, 4:21 PM, Leo Davis wrote: > I recently discovered that the old 'check-mime-type.pl' contrib script is I > had installed on my server was broken when using subversion 1.8.5. I looked > for an updated script in the 1.8.5 branch and in trunk and discovered the > script was also broken in those places as well. After some digging, I found > out when this change happened and patched the script. I suspect (but don't > know for sure) that this only affects subversion 1.8.x. > > This patch is from trunk. > > [[[ > Fix check-mime-type.pl for output changes to 'svnlook proplist'. > > The output format of 'svnlook proplist' was changed in revision 1416637. > > See also http://svn.haxx.se/dev/archive-2012-11/0510.shtml > > * contrib/hook-scripts/check-mime-type.pl: Fix reading process output > from > 'svnlook proplist'. > ]]]
First of all thanks for your contribution. You're quite right that this is broken for 1.8.x and trunk. Unfortunately, your patch isn't quite right either. If you set a property with contains "svn:mime-type" or "svn:eol-style" at the start of a line or with only leading whitespace your parser will read that as the name of a property and will read the next line as the property value. I'd also avoid the assumption that svn:mime-type and svn:eol-style properties are always only a single line or that they have no leading spaces in the values. While the svn client makes efforts to avoid you setting invalid things, the repository backend makes no effort to block this. To that end I'd suggest that you implement a finite state machine to parse this. With the following states: Starting state is fileheader. fileheader: Sanity check state by checking that line has zero whitespace at start (tempting to use /^Properties/ but that breaks with non-English locales). Next state is propname. propname: Sanity check state by checking that line has at exactly 2 leading spaces (not sure if we allow leading whitespace in property names, the client disallows it and I think it's hard to marshal it over the network proptocols but might still be possible with repos/fs layer). Next state is propval. propval: Has at least 4 leading spaces. Only 4 leading spaces are removed from the start of each line before adding to the property value. Next state is either propval, propname, or eof. Determined by looking at next line (number of leading spaces or eof result). eof: End parsing. Can you take on implementing the above? Note that the above states does not account for --show-inherited-props, but I don't think that's a problem since the hook script doesn't use that. To support it you'd need to allow multiple fileheader lines (Inherited line is followed by from line) and empty line ahead of fileheader lines. So you'd have to look ahead up to two lines for that. Side note if it is possible to get property names with leading spaces in them it makes the output of svnlook proplist --verbose impossible to parse reliably. Sadly our old format had a clearer problem with this because setting another property with multiple lines could end up being interpreted as a property. E.G. say I sent a property named "zzz-bypass-mime-check" with the value: line1 svn:mime-type : expected/type The name zzz is to make it be the last property the script sees. At least with fsfs we seem to always return the properties in alphabetical order. I'm not seeing any sorting but I suspect we're putting property values in the representation in a stable order and so we end up with them coming out in the same order.
