Re: [PATCH v2 2/4] sideband: introduce an "escape hatch" to allow control characters

[Junio C Hamano <gitster@pobox.com>]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Good idea! What do you think about something like this?

It may be easier to hack up to piggyback on the http.*.variable
infrastructure, but I do not like the smell of it very much, because
the implementation ties it too tightly to the http transport; I
think this should live in one layer up (transport?).

> If this is the direction you're thinking, I'll polish it and integrate it
> into v3.

In other words, it would be more like sideband.allowEscapeSequences
that is overridden by sideband.<url>.allowEscapeSequences was what I
had in mind.  Or even transfer.allowEscapeSequencesInSideband that
is overridden by transfer.<url>.allowEscapeSequencesInSideband.

>>  - It may no longer matter but a remote repository that may send
>>    messages as strings encoded in ISO/IEC 2022 would need to set
>>    this, merely to make the messages human-readable.  There may be
>>    other reasons the trusted repositories want to send "escape
>>    sequences".
>
> If the remote side has no way to determine whether the client side is
> connected to a terminal or not (which we have already established in this
> thread), it has even less chance to determine which character encoding is
> in use...

Then I think you need to re-read brian's

  https://lore.kernel.org/git/aS-D5lD2Kk6BHNIl@fruit.crustytoothpaste.net/

In any case, I do not think ISO/IEC 2022 matters as much as it used
to back when the reencode_string_iconv() was written (which was the
topic of another thread regarding the broken iconv on macOS wrt
2022).  But even if we limit ourselves to UTF-8, brian's point that
applications do assume certain characteristics on its clients and
implements unportable stuff.  A project targetting developers and/or
users from certain locale may use their own hooks that assumes the
clients understands strings in certain language in certain encoding.

And to serve these projects better, classes like "pass colors",
"pass cursor movements", might help than just "pass everything" vs
"deny everything", but we probably want to try to keep it as simple
as possible; trying to make it finer grained with extra complexity
would only make our efforts look like whack-a-mole X-<.

>> It might even be a good idea to make the default setting of this
>> variable "allow", except for the initial connections to repositories
>> (i.e., "git clone $URL", and "git fetch/ls-remote $URL" with an
>> explicit $URL without using a nickname recorded in our .git/config),
>> as visiting a potentially malicious remote repository you are not
>> familiar with may not be uncommon, and users may deserve protection
>> over inconvenience.
>> 
>> But once the user establishes a working relationship with a remote
>> repository, would it be a lot more common to trust the contents
>> there than be on the lookout that the repository may spew bad
>> strings of bytes at your standard error stream, I have to wonder.

>   tl;dr remote servers don't get more trustworthy just by successfully
>   serving clones.

The "successfully serving clone" has nothing to do with the reason
why I suggested to deny by default in "clone" and anything that gets
$URL not remote nickname.  I am roughly equating the fact that the
user cloned *and* *then* continues to interact with the project that
is served from that remote repository (hence using the remote
nickname) with the willingness by the user to trust that particular
remote repository.