Rework sftp packet decoding#62
Merged
Merged
Conversation
mkj
force-pushed
the
pr/sftp-decode
branch
2 times, most recently
from
July 13, 2026 14:53
168655a to
3142787
Compare
Owner
Author
|
@jubeormk1 I've changed the API for |
Not widely used, and probably should be plain "en" anyway.
Reduced some other logging
Was missed in previous OutputProducer changes, no functional change.
Can be used for validating that the length is consistent.
Previously SftpHandler had separate paths for decoding full packets (Init state) and partial packets (ProcessFragment). Packet decoding used either the input buffer directly, or the RequestHolder buffer. Unknown packet handling was incorrect in some cases, triggerable by repeated rename commands. When one occurred across a fragmented packet, that would fail. This can be simplified by making packet boundary detection separate from the packet decoding. Now a single buffer is stored in a SftpSource, with the SftpSource::fill() function reading from input based only (mostly) on the packet length field. Once a full packet is accumulated, SftpSource::decode() will decode it to either a packet or an appropriate error. Some special handling is needed for UnknownPackets and Write packets which don't decode to the full length. RequestHolder is removed, with its functionality and the proto::decode functions moved into SftpSource. The byte-at-a-time decoding of RequestHolder isn't needed any more. The BBQueue input queue size is now adjusted with some documentation on sizing, though requires further work. The input buffer (previously used by RequestHolder) is now allocated internally as MAX_REQUEST_LEN. That should perhaps be configurable or passed as a buffer.
Both sides run in the same future, so only one waker is needed. Avoids maitake-sync dependencies.
This now stores buffers within a SftpServerHandler, allowing it to be kept in static storage, with sizes provided by the application. Internal handling is renamed to Handler. Tidy various documentation. sftp demo is changed to reuse a SftpHandlerServer as an example.
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.
Previously SftpHandler had separate paths for decoding full packets
(Init state) and partial packets (ProcessFragment). Packet decoding
used either the input buffer directly, or the RequestHolder buffer.
Unknown packet handling was incorrect in some cases, triggerable by
repeated rename commands. When one occurred across a fragmented packet,
that would fail.
This can be simplified by making packet boundary detection separate from
the packet decoding. Now a single buffer is stored in a SftpSource, with
the SftpSource::fill() function reading from input based only (mostly)
on the packet length field. Once a full packet is accumulated,
SftpSource::decode() will decode it to either a packet or an appropriate
error.
Some special handling is needed for UnknownPackets and Write packets
which don't decode to the full length.
RequestHolder is removed, with its functionality and the proto::decode
functions moved into SftpSource. The byte-at-a-time decoding of
RequestHolder isn't needed any more.
The BBQueue input queue size is now adjusted with some documentation on
sizing, though requires further work.
The input buffer (previously used by RequestHolder) is now allocated
internally as MAX_REQUEST_LEN. That should perhaps be configurable
or passed as a buffer.