Re: What's cooking in git.git (Jan 2017, #02; Sun, 15)

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 17, 2017, 19:17 UTC
Message-ID: <xmqqo9z5fqdj.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170116214411.a6wnp66vxydmpmgw@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 19 quoted lines

> On Mon, Jan 16, 2017 at 09:33:07PM +0100, Johannes Sixt wrote:
>
>> However, Jeff's patch is intended to catch exactly these cases (not for the
>> cases where this happens accidentally, but when they happen with malicious
>> intent).
>> 
>> We are talking about user-provided data that is reproduced by die() or
>> error(). I daresay that we do not have a single case where it is intended
>> that this data is intentionally multi-lined, like a commit message. It can
>> only be an accident or malicious when it spans across lines.
>> 
>> I know we allow CR and LF in file names, but in all cases where such a name
>> appears in an error message, it is *not important* that the data is
>> reproduced exactly. On the contrary, it is usually more helpful to know that
>> something strange is going on. The question marks are a strong indication to
>> the user for this.
>
> Yes, exactly. Thanks for explaining this better than I obviously was
> doing. :)

Yup. In the "funny filename" case, the updated code indeed is doing the right thing. So one remaining possible issue is if we ourselves deliberately feed a raw CR (or any non-filtered control code) to error() and friends because their native effect (like returning the carriage to overwrite what we have already said with the remainder of the message by using CR) to functions that go through vreportf() interface. I personally do not think there is any---the progress meter code does use CR for that but they don't do vreportf().

I do not think we write "message\r\n" even for Windows; we rely on vfprintf() and other stdio layer to turn "message\n" into "message\r\n" before it hits write(2) or its equivalent? So I understand that this should affect LF and CRLF systems the same way (meaning, no negative effect).

So... can we move this forward?