Peaker/git-mediate

incorrect handling of filenames containing spaces or line endings

pabs3 opened this issue · 3 comments

pabs3 commented

When dealing with file names containing a space, according to strace, git-mediate instead tries to access the filename with double quotes at the start and end of the filename.

When dealing with file names containing an LF characer, according to strace, git-mediate instead tries to access the filename with LF converted to a literal \n and also double quotes at the start and end of the filename.

This is because the machine-readable git status output format does filename mangling in order to escape characters not representable in that format.

$ git status --porcelain 
UU "a b"
UU "foo\nbar"

To avoid having to unescape characters, I suggest using the NUL-separated git status output format instead.

$ git status --porcelain -z | hd
00000000  55 55 20 61 20 62 00 3f  3f 20 2e 67 69 74 63 6f  |UU a b.?? .gitco|
00000010  6e 66 69 67 00 3f 3f 20  66 6f 6f 0a 62 61 72 00  |nfig.?? foo.bar.|
00000020  3f 3f 20 66 6f 6f 5c 6e  62 61 72 00 3f 3f 20 74  |?? foo\nbar.?? t|
00000030  65 73 74 00                                       |est.|
00000034

This NUL-separated format has some other differences too, a quote from the docs:

There is also an alternate -z format recommended for machine parsing.
In that format, the status field is the same, but some other things change.
First, the -> is omitted from rename entries and the field order is reversed
(e.g from -> to becomes to from). Second, a NUL (ASCII 0) follows each filename,
replacing space as a field separator and the terminating newline (but a space
still separates the status field from the first filename). Third, filenames
containing special characters are not specially formatted; no quoting or
backslash-escaping is performed.

Made a work-in-progress implementation for this in https://github.com/Peaker/git-mediate/compare/statusz
It's working for me so far but the git status -z docs isn't too clear on which status types have a second parameter so I'm not sure it's solid.

pabs3 commented

After a period of time using the change it seems like it works. We'll see if any unsupported cases are reported but felt merging it in is suitable :) Sorry for the delay