Re: [PATCH v2 11/11] ls-refs: reject unknown arguments

[Jeff King <peff@peff.net>]

On Wed, Sep 15, 2021 at 02:09:16AM +0200, Ævar Arnfjörð Bjarmason wrote:

> So the below patch isn't anywhere near applying, but you can see the
> test coverage (those tests are new) & what changes if we instrument a
> client to actually send this bad data.
> 
> That packet_client_error() function is new, part of what I'm doing there
> is converting all of this error emitting from die() to properly sending
> ERR packets.

Right, I noticed that none of this is likely to make it to a client
(unless they are using file:// or an ssh channel which passes back
stderr).

I agree that we should probably be passing these back via ERR packets,
but note that the client is racy here. If the server reports an error
and dies while the client is still writing, they'll see SIGPIPE and exit
without showing the user the ERR packet. The solution may be something
along these lines:

  https://public-inbox.org/git/20200422163357.27056-1-chriscool@tuxfamily.org/

Alternatively, the server can pump the client data to /dev/null until we
hit a flush, and _then_ write the ERR. But that doesn't work for some
protocol-level errors (like "oops, I'm having trouble reading your
pkt-lines).

> I think a bug in your versio is that you're using _() here, if your
> server program happens to be started in Chinese you probably still want
> to emit errors to clients in LANG=C.

I'm not sure it's a bug. If you set LANG=zh on your server, that might
be harmful (if you serve a multi-lingual international audience), or it
might be helpful (if it's a company server where everybody speaks the
language). Likewise, for a file:// or ssh:// operation, your local LANG
would kick in.

I don't really have a strong opinion either way on whether it's helpful
or hindering overall. But I did follow the convention of nearby code in
this series, so I think we don't need to worry about it now (it also
seemed inconsistent to me; serve.c does not mark for translation, but
ls-refs.c does).

> Of course that actually working is subject to various races that I may
> or may not be able to untangle...

Oh good, so you know about them. :)

-Peff