From a02268a25423931fe8a0af8f4a864bec811b41b9 Mon Sep 17 00:00:00 2001 From: Eshan Kelkar Date: Fri, 13 Sep 2024 00:52:40 +0530 Subject: [PATCH] Add helper to receive sftp response messages For the sake of reducing code repetition, this commit adds a helper function to receive sftp response messages. The function can operate in both blocking and non-blocking modes. Signed-off-by: Eshan Kelkar Reviewed-by: Jakub Jelen --- include/libssh/sftp_priv.h | 35 ++++++ src/sftp.c | 229 +++++++++++++------------------------ src/sftp_aio.c | 45 +++----- src/sftp_common.c | 50 ++++++++ 4 files changed, 183 insertions(+), 176 deletions(-) diff --git a/include/libssh/sftp_priv.h b/include/libssh/sftp_priv.h index 8470a8ad..4b578257 100644 --- a/include/libssh/sftp_priv.h +++ b/include/libssh/sftp_priv.h @@ -21,6 +21,8 @@ #ifndef SFTP_PRIV_H #define SFTP_PRIV_H +#include + #ifdef __cplusplus extern "C" { #endif @@ -62,6 +64,39 @@ int sftp_read_and_dispatch(sftp_session sftp); sftp_message sftp_dequeue(sftp_session sftp, uint32_t id); +/** + * @brief Receive the response of an sftp request + * + * In blocking mode, if the response hasn't arrived at the time of call, this + * function waits for the response to arrive. + * + * @param sftp The sftp session via which the request was sent. + * + * @param id The request identifier of the request whose + * corresponding response is required. + * + * @param blocking Flag to indicate the operating mode. true indicates + * blocking mode and false indicates non-blocking mode + * + * @param msg_ptr Pointer to the location to store the response message. + * In case of success, the message is allocated + * dynamically and must be freed (using + * sftp_message_free()) by the caller after usage. In case + * of failure, this is left untouched. + * + * @returns SSH_OK on success + * @returns SSH_ERROR on failure with the sftp and ssh errors set + * @returns SSH_AGAIN in case of non-blocking mode if the response hasn't + * arrived yet. + * + * @warning In blocking mode, this may block indefinitely for an invalid request + * identifier. + */ +int sftp_recv_response_msg(sftp_session sftp, + uint32_t id, + bool blocking, + sftp_message *msg_ptr); + /* * Assigns a new SFTP ID for new requests and assures there is no collision * between them. diff --git a/src/sftp.c b/src/sftp.c index 154de480..f9001693 100644 --- a/src/sftp.c +++ b/src/sftp.c @@ -701,12 +701,9 @@ sftp_dir sftp_opendir(sftp_session sftp, const char *path) return NULL; } - while (msg == NULL) { - if (sftp_read_and_dispatch(sftp) < 0) { - /* something nasty has happened */ - return NULL; - } - msg = sftp_dequeue(sftp, id); + rc = sftp_recv_response_msg(sftp, id, true, &msg); + if (rc != SSH_OK) { + return NULL; } switch (msg->packet_type) { @@ -798,12 +795,9 @@ sftp_attributes sftp_readdir(sftp_session sftp, sftp_dir dir) SSH_LOG(SSH_LOG_PACKET, "Sent a ssh_fxp_readdir with id %" PRIu32, id); - while (msg == NULL) { - if (sftp_read_and_dispatch(sftp) < 0) { - /* something nasty has happened */ - return NULL; - } - msg = sftp_dequeue(sftp, id); + rc = sftp_recv_response_msg(sftp, id, true, &msg); + if (rc != SSH_OK) { + return NULL; } switch (msg->packet_type){ @@ -928,12 +922,9 @@ static int sftp_handle_close(sftp_session sftp, ssh_string handle) return -1; } - while (msg == NULL) { - if (sftp_read_and_dispatch(sftp) < 0) { - /* something nasty has happened */ - return -1; - } - msg = sftp_dequeue(sftp,id); + rc = sftp_recv_response_msg(sftp, id, true, &msg); + if (rc != SSH_OK) { + return -1; } switch (msg->packet_type) { @@ -1073,12 +1064,9 @@ sftp_file sftp_open(sftp_session sftp, return NULL; } - while (msg == NULL) { - if (sftp_read_and_dispatch(sftp) < 0) { - /* something nasty has happened */ - return NULL; - } - msg = sftp_dequeue(sftp, id); + rc = sftp_recv_response_msg(sftp, id, true, &msg); + if (rc != SSH_OK) { + return NULL; } switch (msg->packet_type) { @@ -1197,18 +1185,17 @@ ssize_t sftp_read(sftp_file handle, void *buf, size_t count) { } SSH_BUFFER_FREE(buffer); - while (msg == NULL) { - if (handle->nonblocking) { - if (ssh_channel_poll(handle->sftp->channel, 0) == 0) { - /* we cannot block */ - return 0; - } - } - if (sftp_read_and_dispatch(handle->sftp) < 0) { - /* something nasty has happened */ + rc = sftp_recv_response_msg(handle->sftp, id, !handle->nonblocking, &msg); + if (rc == SSH_ERROR) { return -1; - } - msg = sftp_dequeue(handle->sftp, id); + } + + if (rc == SSH_AGAIN) { + /* + * file opened in non blocking mode and the response has not arrived yet. + * Since we cannot block, return 0 as the number of bytes read. + */ + return 0; } switch (msg->packet_type) { @@ -1309,7 +1296,7 @@ int sftp_async_read(sftp_file file, void *data, uint32_t size, uint32_t id){ sftp_message msg = NULL; sftp_status_message status; ssh_string datastring; - int err = SSH_OK; + int rc, err = SSH_OK; uint32_t len; if (file == NULL) { @@ -1322,20 +1309,9 @@ int sftp_async_read(sftp_file file, void *data, uint32_t size, uint32_t id){ } /* handle an existing request */ - while (msg == NULL) { - if (file->nonblocking){ - if (ssh_channel_poll(sftp->channel, 0) == 0) { - /* we cannot block */ - return SSH_AGAIN; - } - } - - if (sftp_read_and_dispatch(sftp) < 0) { - /* something nasty has happened */ - return SSH_ERROR; - } - - msg = sftp_dequeue(sftp,id); + rc = sftp_recv_response_msg(sftp, id, !file->nonblocking, &msg); + if (rc == SSH_ERROR || rc == SSH_AGAIN) { + return rc; } switch (msg->packet_type) { @@ -1446,12 +1422,10 @@ ssize_t sftp_write(sftp_file file, const void *buf, size_t count) { "Could not write as much data as expected"); } - while (msg == NULL) { - if (sftp_read_and_dispatch(file->sftp) < 0) { - /* something nasty has happened */ + /* Wait for the response in blocking mode */ + rc = sftp_recv_response_msg(sftp, id, true, &msg); + if (rc != SSH_OK) { return -1; - } - msg = sftp_dequeue(file->sftp, id); } switch (msg->packet_type) { @@ -1558,11 +1532,9 @@ int sftp_unlink(sftp_session sftp, const char *file) { } SSH_BUFFER_FREE(buffer); - while (msg == NULL) { - if (sftp_read_and_dispatch(sftp)) { + rc = sftp_recv_response_msg(sftp, id, true, &msg); + if (rc != SSH_OK) { return -1; - } - msg = sftp_dequeue(sftp, id); } if (msg->packet_type == SSH_FXP_STATUS) { @@ -1632,11 +1604,9 @@ int sftp_rmdir(sftp_session sftp, const char *directory) { } SSH_BUFFER_FREE(buffer); - while (msg == NULL) { - if (sftp_read_and_dispatch(sftp) < 0) { + rc = sftp_recv_response_msg(sftp, id, true, &msg); + if (rc != SSH_OK) { return -1; - } - msg = sftp_dequeue(sftp, id); } /* By specification, this command returns SSH_FXP_STATUS */ @@ -1718,11 +1688,9 @@ int sftp_mkdir(sftp_session sftp, const char *directory, mode_t mode) return -1; } - while (msg == NULL) { - if (sftp_read_and_dispatch(sftp) < 0) { - return -1; - } - msg = sftp_dequeue(sftp, id); + rc = sftp_recv_response_msg(sftp, id, true, &msg); + if (rc != SSH_OK) { + return -1; } /* By specification, this command only returns SSH_FXP_STATUS */ @@ -1842,11 +1810,9 @@ int sftp_rename(sftp_session sftp, const char *original, const char *newname) return -1; } - while (msg == NULL) { - if (sftp_read_and_dispatch(sftp) < 0) { - return -1; - } - msg = sftp_dequeue(sftp, id); + rc = sftp_recv_response_msg(sftp, id, true, &msg); + if (rc != SSH_OK) { + return -1; } /* By specification, this command only returns SSH_FXP_STATUS */ @@ -1931,11 +1897,9 @@ int sftp_setstat(sftp_session sftp, const char *file, sftp_attributes attr) return -1; } - while (msg == NULL) { - if (sftp_read_and_dispatch(sftp) < 0) { - return -1; - } - msg = sftp_dequeue(sftp, id); + rc = sftp_recv_response_msg(sftp, id, true, &msg); + if (rc != SSH_OK) { + return -1; } /* By specification, this command only returns SSH_FXP_STATUS */ @@ -2016,11 +1980,9 @@ sftp_lsetstat(sftp_session sftp, const char *file, sftp_attributes attr) return -1; } - while (msg == NULL) { - if (sftp_read_and_dispatch(sftp) < 0) { - return -1; - } - msg = sftp_dequeue(sftp, id); + rc = sftp_recv_response_msg(sftp, id, true, &msg); + if (rc != SSH_OK) { + return -1; } /* By specification, this command only returns SSH_FXP_STATUS */ @@ -2156,11 +2118,9 @@ int sftp_symlink(sftp_session sftp, const char *target, const char *dest) } SSH_BUFFER_FREE(buffer); - while (msg == NULL) { - if (sftp_read_and_dispatch(sftp) < 0) { + rc = sftp_recv_response_msg(sftp, id, true, &msg); + if (rc != SSH_OK) { return -1; - } - msg = sftp_dequeue(sftp, id); } /* By specification, this command only returns SSH_FXP_STATUS */ @@ -2244,11 +2204,9 @@ char *sftp_readlink(sftp_session sftp, const char *path) return NULL; } - while (msg == NULL) { - if (sftp_read_and_dispatch(sftp) < 0) { - return NULL; - } - msg = sftp_dequeue(sftp, id); + rc = sftp_recv_response_msg(sftp, id, true, &msg); + if (rc != SSH_OK) { + return NULL; } if (msg->packet_type == SSH_FXP_NAME) { @@ -2336,11 +2294,9 @@ int sftp_hardlink(sftp_session sftp, const char *oldpath, const char *newpath) return -1; } - while (msg == NULL) { - if (sftp_read_and_dispatch(sftp) < 0) { - return -1; - } - msg = sftp_dequeue(sftp, id); + rc = sftp_recv_response_msg(sftp, id, true, &msg); + if (rc != SSH_OK) { + return -1; } /* By specification, this command only returns SSH_FXP_STATUS */ @@ -2459,11 +2415,9 @@ sftp_statvfs_t sftp_statvfs(sftp_session sftp, const char *path) return NULL; } - while (msg == NULL) { - if (sftp_read_and_dispatch(sftp) < 0) { - return NULL; - } - msg = sftp_dequeue(sftp, id); + rc = sftp_recv_response_msg(sftp, id, true, &msg); + if (rc != SSH_OK) { + return NULL; } if (msg->packet_type == SSH_FXP_EXTENDED_REPLY) { @@ -2533,15 +2487,10 @@ int sftp_fsync(sftp_file file) goto done; } - do { - rc = sftp_read_and_dispatch(sftp); - if (rc < 0) { - ssh_set_error_oom(sftp->session); - rc = -1; - goto done; - } - msg = sftp_dequeue(sftp, id); - } while (msg == NULL); + rc = sftp_recv_response_msg(sftp, id, true, &msg); + if (rc != SSH_OK) { + return -1; + } /* By specification, this command only returns SSH_FXP_STATUS */ if (msg->packet_type == SSH_FXP_STATUS) { @@ -2634,11 +2583,9 @@ sftp_statvfs_t sftp_fstatvfs(sftp_file file) return NULL; } - while (msg == NULL) { - if (sftp_read_and_dispatch(sftp) < 0) { - return NULL; - } - msg = sftp_dequeue(sftp, id); + rc = sftp_recv_response_msg(sftp, id, true, &msg); + if (rc == -1) { + return NULL; } if (msg->packet_type == SSH_FXP_EXTENDED_REPLY) { @@ -2747,11 +2694,9 @@ static sftp_limits_t sftp_limits_use_extension(sftp_session sftp) return NULL; } - while (msg == NULL) { - if (sftp_read_and_dispatch(sftp) < 0) { - return NULL; - } - msg = sftp_dequeue(sftp, id); + rc = sftp_recv_response_msg(sftp, id, true, &msg); + if (rc != SSH_OK) { + return NULL; } if (msg->packet_type == SSH_FXP_EXTENDED_REPLY) { @@ -2897,11 +2842,9 @@ char *sftp_canonicalize_path(sftp_session sftp, const char *path) return NULL; } - while (msg == NULL) { - if (sftp_read_and_dispatch(sftp) < 0) { - return NULL; - } - msg = sftp_dequeue(sftp, id); + rc = sftp_recv_response_msg(sftp, id, true, &msg); + if (rc == -1) { + return NULL; } if (msg->packet_type == SSH_FXP_NAME) { @@ -2988,11 +2931,9 @@ static sftp_attributes sftp_xstat(sftp_session sftp, return NULL; } - while (msg == NULL) { - if (sftp_read_and_dispatch(sftp) < 0) { - return NULL; - } - msg = sftp_dequeue(sftp, id); + rc = sftp_recv_response_msg(sftp, id, true, &msg); + if (rc != SSH_OK) { + return NULL; } if (msg->packet_type == SSH_FXP_ATTRS) { @@ -3066,11 +3007,9 @@ sftp_attributes sftp_fstat(sftp_file file) return NULL; } - while (msg == NULL) { - if (sftp_read_and_dispatch(file->sftp) < 0) { - return NULL; - } - msg = sftp_dequeue(file->sftp, id); + rc = sftp_recv_response_msg(file->sftp, id, true, &msg); + if (rc != SSH_OK) { + return NULL; } if (msg->packet_type == SSH_FXP_ATTRS){ @@ -3146,12 +3085,9 @@ char *sftp_expand_path(sftp_session sftp, const char *path) return NULL; } - while (msg == NULL) { - rc = sftp_read_and_dispatch(sftp); - if (rc < 0) { - return NULL; - } - msg = sftp_dequeue(sftp, id); + rc = sftp_recv_response_msg(sftp, id, true, &msg); + if (rc != SSH_OK) { + return NULL; } if (msg->packet_type == SSH_FXP_NAME) { @@ -3233,12 +3169,9 @@ sftp_home_directory(sftp_session sftp, const char *username) return NULL; } - while (msg == NULL) { - rc = sftp_read_and_dispatch(sftp); - if (rc < 0) { - return NULL; - } - msg = sftp_dequeue(sftp, id); + rc = sftp_recv_response_msg(sftp, id, true, &msg); + if (rc != SSH_OK) { + return NULL; } if (msg->packet_type == SSH_FXP_NAME) { diff --git a/src/sftp_aio.c b/src/sftp_aio.c index c1c54561..0ed4b390 100644 --- a/src/sftp_aio.c +++ b/src/sftp_aio.c @@ -202,21 +202,15 @@ ssize_t sftp_aio_wait_read(sftp_aio *aio, } /* handle an existing request */ - while (msg == NULL) { - if (file->nonblocking) { - if (ssh_channel_poll(sftp->channel, 0) == 0) { - /* we cannot block */ - return SSH_AGAIN; - } - } + rc = sftp_recv_response_msg(sftp, (*aio)->id, !file->nonblocking, &msg); + if (rc == SSH_ERROR) { + SFTP_AIO_FREE(*aio); + return SSH_ERROR; + } - if (sftp_read_and_dispatch(sftp) < 0) { - /* something nasty has happened */ - SFTP_AIO_FREE(*aio); - return SSH_ERROR; - } - - msg = sftp_dequeue(sftp, (*aio)->id); + if (rc == SSH_AGAIN) { + /* return without freeing the (*aio) */ + return SSH_AGAIN; } /* @@ -410,6 +404,7 @@ ssize_t sftp_aio_wait_write(sftp_aio *aio) sftp_session sftp = NULL; sftp_message msg = NULL; sftp_status_message status = NULL; + int rc; /* * This function releases the memory of the structure @@ -446,21 +441,15 @@ ssize_t sftp_aio_wait_write(sftp_aio *aio) return SSH_ERROR; } - while (msg == NULL) { - if (file->nonblocking) { - if (ssh_channel_poll(sftp->channel, 0) == 0) { - /* we cannot block */ - return SSH_AGAIN; - } - } + rc = sftp_recv_response_msg(sftp, (*aio)->id, !file->nonblocking, &msg); + if (rc == SSH_ERROR) { + SFTP_AIO_FREE(*aio); + return SSH_ERROR; + } - if (sftp_read_and_dispatch(sftp) < 0) { - /* something nasty has happened */ - SFTP_AIO_FREE(*aio); - return SSH_ERROR; - } - - msg = sftp_dequeue(sftp, (*aio)->id); + if (rc == SSH_AGAIN) { + /* Return without freeing the (*aio) */ + return SSH_AGAIN; } /* diff --git a/src/sftp_common.c b/src/sftp_common.c index 13512b8d..c0345ce2 100644 --- a/src/sftp_common.c +++ b/src/sftp_common.c @@ -859,6 +859,56 @@ int sftp_read_and_dispatch(sftp_session sftp) return 0; } +int sftp_recv_response_msg(sftp_session sftp, + uint32_t id, + bool blocking, + sftp_message *msg_ptr) +{ + sftp_message msg = NULL; + int rc; + + if (sftp == NULL) { + return SSH_ERROR; + } + + if (msg_ptr == NULL) { + ssh_set_error_invalid(sftp->session); + sftp_set_error(sftp, SSH_FX_FAILURE); + return SSH_ERROR; + } + + SSH_LOG(SSH_LOG_PACKET, + "Trying to receive response of request id %" PRIu32 " in %s mode", + id, + blocking ? "blocking" : "non-blocking"); + + do { + if (!blocking) { + rc = ssh_channel_poll(sftp->channel, 0); + if (rc == SSH_ERROR) { + sftp_set_error(sftp, SSH_FX_FAILURE); + return SSH_ERROR; + } + + if (rc == 0) { + /* nothing available and we cannot block */ + return SSH_AGAIN; + } + } + + rc = sftp_read_and_dispatch(sftp); + if (rc == -1) { + /* something nasty has happened */ + return SSH_ERROR; + } + + msg = sftp_dequeue(sftp, id); + } while (msg == NULL); + + *msg_ptr = msg; + return SSH_OK; +} + sftp_status_message parse_status_msg(sftp_message msg) { sftp_status_message status = NULL;