Implement SftpSession API surface#5030
Draft
ricab wants to merge 36 commits into
Draft
Conversation
ricab
force-pushed
the
sftp-session-type
branch
2 times, most recently
from
bf2cf04 to
7229700
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## sftp-session-type #5030 +/- ##
=====================================================
- Coverage 87.35% 86.77% -0.57%
=====================================================
Files 277 279 +2
Lines 14706 14810 +104
=====================================================
+ Hits 12845 12850 +5
- Misses 1861 1960 +99 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Rename SSHSession::force_shutdown to express its true intent: shutting down custom sockets, which libssh won't disconnect on its own. While we're not ssh-ing over custom sockets yet, that is planned for VSOCK.
Add an SftpMessage interface and a concrete PlainSftpMessage implementation backed by libssh. This allows will allow the SftpServer to read messages off of an SftpSession without having to assume a particular implementation.
Declare a private RawSftpSessionDeleter and use it to define, RawSftpSessionUptr, avoiding a libssh include in a header and matching the approach used for RawSftpMsgUptr.
Add a cancellable method to obtain the next SftpMessage from an SftpSession. Implement in PlainSftpSession returning a PlainSftpMessage.
Our server side expects only output that is covered by the SFTP protocol, so explicitly run sudo non-interactively to be sure it doesn't ask for a password (failing instead if it needs one).
Constructing an SFTP session involves obtaining and processing a first SFTP protocol message. If the remote endpoint fails to send data for any reason, construction would block the calling thread before there was ever an object to `request_stop` from. Protect against that by adding an initial bounded poll.
Match the deleter scheme used in Sftp types and avoid including libssh in its header.
Match the deleter scheme already used for the other SSH/SFTP raw handles, and keep ChannelUPtr private now that nothing outside the class needs it, turning the channel factory into a private static.
Extract a dedicated SftpInitException to avoid repetition. Keep the new exception in the anonymous namespace until we need a dedicated catch.
Introduce a header for SFTP (v3) protocol vocabulary. This will allow consumers of SFTP to rely on the protocol without relying on specific implementations.
Add a compilation unit with static asserts to make sure, at build time, that our protocol vocabulary is aligned with libssh's.
Remove doc-comments with only a copydoc to reduce redundancy scrolling and make the code read better.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Provide the interface that the SftpServer needs to replace SSHSession, SSHProcess, and raw libssh.
Note: on the SftpMessage/PlainSftpMessage split and protocol vocabulary
This PR continues the trend of an interface/implementation split for the new Plain/SftpMessage types, abstracting libssh away from the interface level. Users of SftpSession can interact with the client side via SftpMessage objects and use an implementation-independent vocabulary, defined in sftp_protocol.h, instead of having to depend on libssh.
I debated this a lot. Although I don't fancy having to repeat constants that the program already has, and despite the few additional int copies per transaction (negligible next to encryption and network round-trips), I went with this design because:
std::errcdoes something similar with POSIX errno values,std::filesystem::permshardcodesowner_read = 0400,owner_write = 0200, etc.Related Issue(s)
Closes #4952
Closes #4953
MULTI-2622
MULTI-2647
MULTI-2720
Testing
Unit tests
Manual testing steps:
Screenshots (if applicable)
Checklist
Additional Notes