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.
Received on 2014-03-01 04:08:11 CET