necouchman commented on code in PR #629:
URL: https://github.com/apache/guacamole-server/pull/629#discussion_r2725892654
##########
src/common-ssh/sftp.c:
##########
@@ -34,6 +34,68 @@
#include <stdlib.h>
#include <string.h>
+/*
+ * Size of the per-upload buffer, in bytes.
+ *
+ * SFTP packets carry up to 32KB of payload, but libssh2_sftp_write() can
+ * accept larger buffers and will internally split them across multiple SFTP
+ * packets.
+ */
+#define GUAC_COMMON_SSH_SFTP_UPLOAD_BUFFER_SIZE (256 * 1024)
+
+typedef struct guac_common_ssh_sftp_upload_state {
Review Comment:
Please provide overall documentation for the data structure.
##########
src/common-ssh/sftp.c:
##########
@@ -34,6 +34,68 @@
#include <stdlib.h>
#include <string.h>
+/*
+ * Size of the per-upload buffer, in bytes.
+ *
+ * SFTP packets carry up to 32KB of payload, but libssh2_sftp_write() can
+ * accept larger buffers and will internally split them across multiple SFTP
+ * packets.
+ */
+#define GUAC_COMMON_SSH_SFTP_UPLOAD_BUFFER_SIZE (256 * 1024)
+
+typedef struct guac_common_ssh_sftp_upload_state {
+ LIBSSH2_SFTP_HANDLE* file; /* Underlying SFTP file handle */
+ char buffer[GUAC_COMMON_SSH_SFTP_UPLOAD_BUFFER_SIZE]; /*
Buffered upload data */
+ int buffered_length; /* Bytes currently buffered */
+ int error; /* Non-zero if a write error
occurred */
+} guac_common_ssh_sftp_upload_state;
+
+/**
+ * Flushes any buffered upload data to the remote SFTP file.
+ *
+ * Partial writes are retried until complete or an error occurs. On error, the
+ * upload state is marked so that subsequent blobs can report failure.
+ *
+ * NOTE: libssh2_sftp_write() may return a short count even when no error has
+ * occurred. A short return is still progress. Continue calling until the
+ * entire buffer has been consumed before reusing or modifying it.
+ *
+ * @param user
+ * The user receiving the upload.
+ *
+ * @param state
+ * Per-stream upload state containing the SFTP handle and buffered data.
+ *
+ * @return
+ * Nothing.
+ */
+static void guac_common_ssh_sftp_upload_flush(guac_user* user,
+ guac_common_ssh_sftp_upload_state* state) {
+
+ if (state->error || state->buffered_length <= 0 || state->file == NULL)
+ return;
+
+ int offset = 0;
+ while (offset < state->buffered_length) {
+ ssize_t written = libssh2_sftp_write(state->file,
+ state->buffer + offset, state->buffered_length - offset);
+ if (written <= 0) {
+ guac_user_log(user, GUAC_LOG_INFO,
+ "Buffered SFTP write failed after %d bytes (attempt
returned %zd)",
+ offset, written);
+ state->error = 1;
+ break;
+ }
+ offset += (int)written;
+ }
+
+ if (!state->error)
+ guac_user_log(user, GUAC_LOG_DEBUG,
+ "Flushed %d buffered bytes to remote file",
state->buffered_length);
+
+ state->buffered_length = 0;
+}
Review Comment:
Some documentation throughout this function would be nice.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
