Skip to content

Conversation

@devin-ai-integration
Copy link
Contributor

Move sentSzSave variable to state structure in wolfSSH_SFTP_SendWritePacket

This PR makes the following changes:

  1. Moves the sentSzSave variable from being a local variable in the wolfSSH_SFTP_SendWritePacket function to being a member of the WS_SFTP_SEND_WRITE_STATE structure.
  2. Adds a GitHub Action workflow to test SFTP file transfers with non-blocking sockets.

Changes Made

  • Added sentSzSave as a member variable to the WS_SFTP_SEND_WRITE_STATE structure
  • Removed the local variable declaration from the wolfSSH_SFTP_SendWritePacket function
  • Added initialization of the new state member in the initialization state
  • Updated all references to use state->sentSzSave instead of the local variable
  • Created a GitHub Action workflow to test SFTP file transfers with non-blocking sockets

Testing

The changes have been tested locally using the following procedure:

  1. Built wolfSSL with SSH support
  2. Built wolfSSH with SFTP support
  3. Started the echoserver in the background
  4. Used wolfsftp with the -N flag (non-blocking sockets) to connect to the server
  5. Used the put command to transfer a 2MB test file
  6. Verified that the sent and received files matched exactly using MD5 checksums

The test confirmed that the changes did not affect the functionality of the SFTP file transfer and that the non-blocking mode works correctly.

Link to Devin run: https://app.devin.ai/sessions/ce8d743061084bff9e1dc755bd55c96e
Requested by: andrew@wolfssl.com

@devin-ai-integration
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add "(aside)" to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@wolfSSL-Bot
Copy link

Can one of the admins verify this patch?

@LinuxJedi LinuxJedi self-assigned this Feb 25, 2025
Co-Authored-By: andrew@wolfssl.com <andrew@wolfssl.com>
@devin-ai-integration devin-ai-integration bot force-pushed the devin/1740508226-sftp-state-var branch from a0f2d7f to 502b5a6 Compare February 25, 2025 19:00
@devin-ai-integration devin-ai-integration bot changed the title Move sentSzSave variable to state structure in wolfSSH_SFTP_SendWritePacket Fix SFTP data truncation issue by moving sentSzSave to state structure Feb 25, 2025
@devin-ai-integration devin-ai-integration bot changed the title Fix SFTP data truncation issue by moving sentSzSave to state structure Fix SFTP data truncation by moving sentSzSave to state structure Feb 25, 2025
@LinuxJedi
Copy link
Member

(aside) For reviewer context:

sentSzSave stores how much transfer a single packet has so that it can be returned at the end of wolfSSH_SFTP_SendWritePacket. In non-blocking mode, sentSzSave can be set in the state machine, then the function can return to be called later and continue. As sentSzSave is a local, this is now 0. This causes 0 to be returned ad the callee, wolfSSH_SFTP_Put, thinks this means the file transfer has finished, shutting it down. The result is a truncated file, usually after only one or two 32KB packets.

This patch moves sentSzSave into the state storage struct, so that it is retained when exiting from and returning to the state machine. It also adds a test which transfers a 2MB file using sftpclient with PUT in non-blocking mode.

@JacobBarthelmeh
Copy link
Contributor

(aside) Ok to test . Kick off the Jenkins tests.

@JacobBarthelmeh JacobBarthelmeh merged commit a768c0f into master Feb 26, 2025
82 checks passed
@ejohnstown ejohnstown deleted the devin/1740508226-sftp-state-var branch June 12, 2025 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants