git cat-file --batch surprising carriage return behavior

I'm surprised to find that git cat-file --batch, on a Linux system, strips the \r from an input like "HEAD:foo\r\n"

It's obvious, of course, that it will remove the newline, and so this interface cannot be used to query about a filename that, for some horrible reason[1], contains a newline. But very surprising that it cannot be used for filename that contains a carriage return, at least on a non-Windows system.

The docs for cat-file --batch say the list of objects is separated by linefeeds. I don't know if updating the docs is the best fix. (I'd be happy to use a -z if it had one.)

On Tue, Oct 08, 2019 at 03:22:57PM -0400, Joey Hess wrote:

This is likely due to b42ca3dd0f (cat-file: read batch stream with strbuf_getline(), 2015-10-28), and the matching c8aa9fdf5d (strbuf: make strbuf_getline_crlf() global, 2015-10-28).

I agree it's a bit surprising (though OTOH, I imagine the old behavior surprised some people in the opposite direction).

Yeah, I agree that a -z option is the best path forward. For non-z input, I'm tempted to say we could unquote entries that start with a double-quote (the match to how we handle filenames in non-z diff output). That would mean breaking compatibility for refnames that start with a quote, though. If we just add a new "-z", that's less disruptive _and_ easier to use.

I suspect it's not entirely sufficient for clean input, though. You're not feeding filenames but rather full "object names". I wouldn't be surprised if we mis-parse "$rev:$path" when $path has "@{}" or similar in it.

So what you may actually want is some more robust input format that lets you specify the filename as an independent NUL-terminated entity.

-Peff

Jeff King wrote:

Agreed. 

Nothing I've tried along the lines of "HEAD:{yesterday}" has misparsed the part after the colon as anything but a filename.

The one I can think of where there's a parse ambiguity is that while :foo gets file foo, :1:foo does not get file "1:foo". Instead it's treated as a stage number. Using either HEAD:1:foo or :./1:foo will avoid that ambiguity.

On Wed, Oct 09, 2019 at 11:28:51AM -0400, Joey Hess wrote:

It's possible we've fixed them all. We definitely don't parse strictly left-to-right. The first thing we try to do is strip bits like ^{commit} off the end, before we even find the colon. But after doing so, we should generally be left with a resolvable name, and I think that uses the "basic" parser which will not allow colons. I.e., this:

  mkdir subdir
  echo whatever >subdir/file
  git add subdir
  git commit -m 'add directory'
  git show HEAD:subdir^{tree}

really does look for the file "subdir^{tree}" in HEAD, and not "HEAD:subdir" as a tree.

We have had bugs in the past; I'm thinking specifically of 8cd4249c4c (interpret_branch_name: always respect "namelen" parameter, 2014-01-15). But I couldn't find any problematic inputs after poking around for a few minutes.

Yeah, that makes sense.

-Peff