ssh: Unify logic to handle git-upload-pack(1) and git-upload-archive(1)
The logic to run git-upload-pack(1) and git-upload-archive(1) is quite complex. Despite the need to splice several standard streams, we also need to handle the case where negotiation of the data that is to be sent does not finish, which is done to address time-of-check-time-of-use attacks.
Right now, much of the logic to do this setup is split up across the
callers and monitorStdinCommand()
. As there are multiple callers of
the latter function, this means that some of the logic is duplicated.
Furthermore, it is quite hard to change monitorStdinCommand()
to alter
its semantics, as it is not at all a self-contained building block.
Refactor this to move much of the logic into monitorStdinCommand()
,
which brings us a bunch of advantages:
- The monitoring logic is more self-contained so that we can amend
it at a later point. This was the original motivation of this
patch as it will be required to fix cases where we don't correctly
detect that the mentioned negotiation has completed.
- It allows us to use the same counting writer for SSHUploadArchive
that we already use for SSHUploadPack. We thus get better insight
into how much data we're typically returning in this RPC.
- It allows us to use the same large buffer reader that we already
use for SSHUploadPack. This reader uses a larger-than-usual buffer
so that we can avoid many syscalls and should thus help to make
SSHUploadArchive more efficient.
- We can reuse the bits to detect cancellation via our pktline
monitor in case negotiation does not finish in time.
- We can reuse the error handling where the remote side hangs up
unexpectedly, which is thrown by Git's pktline protocol in case
the remote side hangs up in the middle of a write. This allows us
to label user-cancelled requests as `codes.Canceled` instead of
`codes.Internal` in SSHUploadArchive.
So overall we gain quite a lot of benefits with this refactoring while also simplifying our code base.
Part of #5351 (closed).