From 4024fea9a7009e140924da30e08b1b0e8570d58c Mon Sep 17 00:00:00 2001 From: Anthony Hu Date: Fri, 29 Jul 2022 11:51:35 -0400 Subject: [PATCH 1/4] Fixes directory listing output in sftp. We were only only showing the output of the first message of a directory listing which is wrong. A directory listing can be composed of several messages so we must keep reading until there is no more to read. Fixes ZD14587 --- src/wolfsftp.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/wolfsftp.c b/src/wolfsftp.c index bdc9138a..7bbaa7b3 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -6078,6 +6078,7 @@ WS_SFTPNAME* wolfSSH_SFTP_LS(WOLFSSH* ssh, char* dir) case STATE_LS_READDIR: /* now read the dir */ + WS_SFTPNAME* names = NULL; state->name = wolfSSH_SFTP_ReadDir(ssh, state->handle, state->sz); if (state->name == NULL) { if (ssh->error == WS_WANT_READ || ssh->error == WS_WANT_WRITE) { @@ -6086,6 +6087,30 @@ WS_SFTPNAME* wolfSSH_SFTP_LS(WOLFSSH* ssh, char* dir) WLOG(WS_LOG_SFTP, "Error reading directory"); /* fall through because the handle should always be closed */ } + names = state->name; + + while (names != NULL) { + names = wolfSSH_SFTP_ReadDir(ssh, state->handle, state->sz); + if (names == NULL) { + if (ssh->error == WS_WANT_READ || + ssh->error == WS_WANT_WRITE) { + /* Error condition. Clean up previous names that we got + * and return null to indicate error. */ + wolfSSH_SFTPNAME_list_free(state->name); + state->name = NULL; + return NULL; + } + /* Got all the names. Leave the while loop and fall through + * because the handle should always be closed. */ + } else { + WS_SFTPNAME* runner = NULL; + /* Got more entries so we append them. */ + for(runner = state->name; runner->next != NULL; + runner = runner->next); + runner->next = names; + } + } + state->state = STATE_LS_CLOSE; NO_BREAK; From d29991b15c56acf7daf98a4ae707436f22cce0d1 Mon Sep 17 00:00:00 2001 From: Anthony Hu Date: Fri, 29 Jul 2022 12:23:14 -0400 Subject: [PATCH 2/4] Changes inspired by peer review feedback. --- src/wolfsftp.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/wolfsftp.c b/src/wolfsftp.c index 7bbaa7b3..d5c18c6a 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -6094,19 +6094,20 @@ WS_SFTPNAME* wolfSSH_SFTP_LS(WOLFSSH* ssh, char* dir) if (names == NULL) { if (ssh->error == WS_WANT_READ || ssh->error == WS_WANT_WRITE) { - /* Error condition. Clean up previous names that we got - * and return null to indicate error. */ + /* Clean up previous names that we got. */ wolfSSH_SFTPNAME_list_free(state->name); state->name = NULL; return NULL; } /* Got all the names. Leave the while loop and fall through * because the handle should always be closed. */ - } else { + } + else { WS_SFTPNAME* runner = NULL; /* Got more entries so we append them. */ - for(runner = state->name; runner->next != NULL; - runner = runner->next); + for (runner = state->name; + runner->next != NULL; + runner = runner->next); runner->next = names; } } From 0221a5e90fe29635ca6e1952cad96486ff5b84f3 Mon Sep 17 00:00:00 2001 From: Anthony Hu Date: Fri, 29 Jul 2022 13:49:00 -0400 Subject: [PATCH 3/4] Make clang happy --- src/wolfsftp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/wolfsftp.c b/src/wolfsftp.c index d5c18c6a..9d78332f 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -6017,6 +6017,7 @@ WS_SFTPNAME* wolfSSH_SFTP_LS(WOLFSSH* ssh, char* dir) { struct WS_SFTP_LS_STATE* state = NULL; WS_SFTPNAME* name = NULL; + WS_SFTPNAME* names = NULL; if (ssh == NULL || dir == NULL) { WLOG(WS_LOG_SFTP, "Bad argument passed in"); @@ -6078,7 +6079,6 @@ WS_SFTPNAME* wolfSSH_SFTP_LS(WOLFSSH* ssh, char* dir) case STATE_LS_READDIR: /* now read the dir */ - WS_SFTPNAME* names = NULL; state->name = wolfSSH_SFTP_ReadDir(ssh, state->handle, state->sz); if (state->name == NULL) { if (ssh->error == WS_WANT_READ || ssh->error == WS_WANT_WRITE) { @@ -6111,7 +6111,6 @@ WS_SFTPNAME* wolfSSH_SFTP_LS(WOLFSSH* ssh, char* dir) runner->next = names; } } - state->state = STATE_LS_CLOSE; NO_BREAK; From 9830846ec1c37082827e1a8ca1395ac68b41f025 Mon Sep 17 00:00:00 2001 From: Anthony Hu Date: Tue, 2 Aug 2022 17:40:31 -0400 Subject: [PATCH 4/4] Make jenkins happy. Proper cleanup and non-blocking support. --- src/wolfsftp.c | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/src/wolfsftp.c b/src/wolfsftp.c index 9d78332f..2e41fd6b 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -5723,6 +5723,9 @@ static WS_SFTPNAME* wolfSSH_SFTP_DoName(WOLFSSH* ssh) wolfSSH_SFTP_buffer_rewind(&state->buffer); wolfSSH_SFTP_DoStatus(ssh, reqId, &state->buffer); + if (ssh->error != WS_WANT_READ) { + wolfSSH_SFTP_ClearState(ssh, STATE_ID_NAME); + } return NULL; } NO_BREAK; @@ -6078,29 +6081,20 @@ WS_SFTPNAME* wolfSSH_SFTP_LS(WOLFSSH* ssh, char* dir) NO_BREAK; case STATE_LS_READDIR: - /* now read the dir */ - state->name = wolfSSH_SFTP_ReadDir(ssh, state->handle, state->sz); - if (state->name == NULL) { + /* Now read the dir. Note in non-blocking we might get here multiple + * times so we have to assign to state->name later. */ + names = wolfSSH_SFTP_ReadDir(ssh, state->handle, state->sz); + if (names == NULL) { if (ssh->error == WS_WANT_READ || ssh->error == WS_WANT_WRITE) { return NULL; } WLOG(WS_LOG_SFTP, "Error reading directory"); /* fall through because the handle should always be closed */ } - names = state->name; while (names != NULL) { - names = wolfSSH_SFTP_ReadDir(ssh, state->handle, state->sz); - if (names == NULL) { - if (ssh->error == WS_WANT_READ || - ssh->error == WS_WANT_WRITE) { - /* Clean up previous names that we got. */ - wolfSSH_SFTPNAME_list_free(state->name); - state->name = NULL; - return NULL; - } - /* Got all the names. Leave the while loop and fall through - * because the handle should always be closed. */ + if (state->name == NULL) { + state->name = names; } else { WS_SFTPNAME* runner = NULL; @@ -6110,7 +6104,19 @@ WS_SFTPNAME* wolfSSH_SFTP_LS(WOLFSSH* ssh, char* dir) runner = runner->next); runner->next = names; } + names = wolfSSH_SFTP_ReadDir(ssh, state->handle, state->sz); + if (names == NULL) { + if (ssh->error == WS_WANT_READ + || ssh->error == WS_WANT_WRITE) { + /* State does not change so we will get back to this case + * clause in non-blocking mode. */ + return NULL; + } + WLOG(WS_LOG_SFTP, "Error reading directory"); + /* fall through because the handle should always be closed. */ + } } + state->state = STATE_LS_CLOSE; NO_BREAK;