diff --git a/tests/api.c b/tests/api.c index 1edada867..6e3359e98 100644 --- a/tests/api.c +++ b/tests/api.c @@ -2697,6 +2697,85 @@ static void sftp_client_connect(WOLFSSH_CTX** ctx, WOLFSSH** ssh, int port) } +/* Upper bound on retry iterations for the SFTP helpers below. */ +#define SFTP_MAX_RETRY_TRIES 1000 + +/* Which errors are safe to retry depends on the callee: re-calling one that + * has already torn down its state reissues a request that may be partly on + * the wire, which is the desync these loops exist to avoid. + * + * wolfSSH_SFTP_Open() and wolfSSH_SFTP_SendWritePacket() keep their state + * only for these two (src/wolfsftp.c STATE_OPEN_SEND, STATE_OPEN_GETHANDLE, + * STATE_SEND_WRITE_READ_STATUS). wolfSSH_SFTP_LS() also keeps it on + * WS_REKEYING. */ +static int sftp_error_keeps_state(int err) +{ + return err == WS_WANT_READ || err == WS_WANT_WRITE; +} + + +/* wolfSSH_SFTP_Remove() and wolfSSH_SFTP_Close() gate on NoticeError() in + * src/wolfsftp.c instead, which is broader. Mirror it exactly; the two must + * not drift apart. */ +static int sftp_error_is_notice(int err) +{ + return sftp_error_keeps_state(err) || err == WS_CHAN_RXD || + err == WS_WINDOW_FULL || err == WS_REKEYING; +} + + +/* Drive an SFTP operation to completion. Abandoning one part way leaves its + * response pending on the stream, which the next operation then reads as its + * own header. Both helpers stop on a terminal error so a real failure is + * still reported to the caller. */ +static int sftp_retry_remove(WOLFSSH* ssh, char* name) +{ + int ret = WS_FATAL_ERROR; + int tries; + + for (tries = 0; tries < SFTP_MAX_RETRY_TRIES; tries++) { + ret = wolfSSH_SFTP_Remove(ssh, name); + if (ret == WS_SUCCESS || + !sftp_error_is_notice(wolfSSH_get_error(ssh))) { + break; + } + } + + return ret; +} + + +static int sftp_retry_close(WOLFSSH* ssh, byte* handle, word32 handleSz) +{ + int ret = WS_FATAL_ERROR; + int tries; + + for (tries = 0; tries < SFTP_MAX_RETRY_TRIES; tries++) { + ret = wolfSSH_SFTP_Close(ssh, handle, handleSz); + if (ret == WS_SUCCESS || + !sftp_error_is_notice(wolfSSH_get_error(ssh))) { + break; + } + } + + return ret; +} + + +/* The staged read target is filled with 'a'; every read starts at offset 0, + * so all returned bytes must be 'a'. A macro so a failure reports the + * calling read's line. */ +#define SFTP_CHECK_READ_PAYLOAD(out, rxSz) do { \ + int _i; \ + for (_i = 0; _i < (rxSz); _i++) { \ + if ((out)[_i] != 'a') { \ + break; \ + } \ + } \ + AssertIntEQ(_i, (rxSz)); \ +} while (0) + + static void test_wolfSSH_SFTP_SendReadPacket(void) { func_args ser; @@ -2738,6 +2817,18 @@ static void test_wolfSSH_SFTP_SendReadPacket(void) int outSz = 18; int rxSz; const word32 ofst[2] = {0}; + char rdName[] = "wolfssh_5574_read.tmp"; + byte rdHandle[WOLFSSH_MAX_HANDLE]; + word32 rdHandleSz; + word32 rdOfst[2] = {0, 0}; + word32 rdSz = 0; + word32 rdChunk; + /* staging chunk, kept small for constrained targets */ + byte rdData[512]; + int rdTries; + int rdWrote; + int rdErr; + int rdRet; #ifdef WOLFSSH_TEST_INTERNAL int err; int tries; @@ -2749,23 +2840,80 @@ static void test_wolfSSH_SFTP_SendReadPacket(void) char wrName[] = "wolfssh_5574_write.tmp"; #endif - current = wolfSSH_SFTP_LS(ssh, (char*)currentDir); + /* Stage the file to read. Opening the listing's first entry raced + * with tests/testsuite.test, which creates and removes files in the + * same directory under "make -j check". The staging is asserted + * rather than skipped on failure; skipping would drop the read + * coverage below without failing the test. Note this makes a + * writable server working directory a prerequisite of the test. */ + WMEMSET(rdData, 'a', sizeof(rdData)); + /* best effort, the file is normally absent */ + (void)sftp_retry_remove(ssh, rdName); + rdRet = WS_FATAL_ERROR; + for (rdTries = 0; rdTries < SFTP_MAX_RETRY_TRIES; rdTries++) { + rdHandleSz = WOLFSSH_MAX_HANDLE; + rdRet = wolfSSH_SFTP_Open(ssh, rdName, + WOLFSSH_FXF_WRITE | WOLFSSH_FXF_CREAT | WOLFSSH_FXF_TRUNC, + NULL, rdHandle, &rdHandleSz); + if (rdRet == WS_SUCCESS) { + break; + } + rdErr = wolfSSH_get_error(ssh); + if (!sftp_error_keeps_state(rdErr)) { + break; /* create failed */ + } + } + AssertIntEQ(rdRet, WS_SUCCESS); + + /* one try per full chunk plus slack for transient errors */ + rdTries = (int)(WOLFSSH_MAX_SFTP_RW / sizeof(rdData)) + + SFTP_MAX_RETRY_TRIES; + for (; rdTries > 0 && rdSz < WOLFSSH_MAX_SFTP_RW; rdTries--) { + rdChunk = (word32)sizeof(rdData); + if (rdChunk > WOLFSSH_MAX_SFTP_RW - rdSz) { + rdChunk = WOLFSSH_MAX_SFTP_RW - rdSz; + } + rdOfst[0] = rdSz; + rdWrote = wolfSSH_SFTP_SendWritePacket(ssh, rdHandle, + rdHandleSz, rdOfst, rdData, rdChunk); + if (rdWrote > 0) { + rdSz += (word32)rdWrote; + continue; + } + rdErr = wolfSSH_get_error(ssh); + if (!sftp_error_keeps_state(rdErr)) { + break; /* unexpected error */ + } + } + rdRet = sftp_retry_close(ssh, rdHandle, rdHandleSz); + AssertIntEQ(rdSz, WOLFSSH_MAX_SFTP_RW); + AssertIntEQ(rdRet, WS_SUCCESS); + + current = NULL; + for (rdTries = 0; rdTries < SFTP_MAX_RETRY_TRIES; rdTries++) { + current = wolfSSH_SFTP_LS(ssh, (char*)currentDir); + if (current != NULL) { + break; + } + rdErr = wolfSSH_get_error(ssh); + if (!sftp_error_keeps_state(rdErr) && rdErr != WS_REKEYING) { + break; + } + } + AssertNotNull(current); tmp = current; while (tmp != NULL) { - if ((tmp->atrb.sz[0] > 0) && - (tmp->atrb.flags & WOLFSSH_FILEATRB_PERM) && - !(tmp->atrb.per & 040000)) { + if (tmp->fName != NULL && WSTRCMP(tmp->fName, rdName) == 0) { break; } tmp = tmp->next; } + AssertNotNull(tmp); - if (tmp != NULL) { - /* Allocate buffer large enough for maximum read size */ - word32 allocSz = tmp->atrb.sz[0]; - if (allocSz < WOLFSSH_MAX_SFTP_RW) - allocSz = WOLFSSH_MAX_SFTP_RW; - out = (byte*)malloc(allocSz); + { + /* The staging above wrote WOLFSSH_MAX_SFTP_RW bytes, and no read + * below asks for more, so that is the buffer size needed. */ + out = (byte*)malloc(WOLFSSH_MAX_SFTP_RW); AssertNotNull(out); AssertIntEQ(wolfSSH_SFTP_Open(ssh, tmp->fName, WOLFSSH_FXF_READ, NULL, handle, &handleSz), WS_SUCCESS); @@ -2779,13 +2927,12 @@ static void test_wolfSSH_SFTP_SendReadPacket(void) */ /* read 18 bytes */ - if (tmp->atrb.sz[0] >= 18) { - outSz = 18; - rxSz = wolfSSH_SFTP_SendReadPacket(ssh, handle, handleSz, - ofst, out, outSz); - AssertIntGT(rxSz, 0); - AssertIntLE(rxSz, outSz); - } + outSz = 18; + rxSz = wolfSSH_SFTP_SendReadPacket(ssh, handle, handleSz, + ofst, out, outSz); + AssertIntGT(rxSz, 0); + AssertIntLE(rxSz, outSz); + SFTP_CHECK_READ_PAYLOAD(out, rxSz); /* partial read */ outSz = WOLFSSH_MAX_SFTP_RW / 2; @@ -2794,6 +2941,7 @@ static void test_wolfSSH_SFTP_SendReadPacket(void) if (wolfSSH_get_error(ssh) != WS_REKEYING) { AssertIntGT(rxSz, 0); AssertIntLE(rxSz, outSz); + SFTP_CHECK_READ_PAYLOAD(out, rxSz); } /* read all */ @@ -2803,6 +2951,7 @@ static void test_wolfSSH_SFTP_SendReadPacket(void) if (wolfSSH_get_error(ssh) != WS_REKEYING) { AssertIntGT(rxSz, 0); AssertIntLE(rxSz, outSz); + SFTP_CHECK_READ_PAYLOAD(out, rxSz); } #ifdef WOLFSSH_TEST_INTERNAL @@ -2840,12 +2989,17 @@ static void test_wolfSSH_SFTP_SendReadPacket(void) AssertIntLT(tries, 1000); AssertIntGT(rxSz, 0); AssertIntLE(rxSz, outSz); + SFTP_CHECK_READ_PAYLOAD(out, rxSz); #endif /* WOLFSSH_TEST_INTERNAL */ free(out); - wolfSSH_SFTP_Close(ssh, handle, handleSz); + /* Results ignored: the reads above are skipped rather than driven + * to completion on WS_REKEYING, so the stream state here is not + * known to be clean and a failure would not mean a real fault. */ + (void)sftp_retry_close(ssh, handle, handleSz); } wolfSSH_SFTPNAME_list_free(current); + (void)sftp_retry_remove(ssh, rdName); #ifdef WOLFSSH_TEST_INTERNAL /* Issue 5574: exercise the partial-send resume path for @@ -2886,8 +3040,10 @@ static void test_wolfSSH_SFTP_SendReadPacket(void) AssertIntLT(tries, 1000); AssertIntEQ(wrRet, (int)sizeof(wrData)); - wolfSSH_SFTP_Close(ssh, whandle, whandleSz); - wolfSSH_SFTP_Remove(ssh, wrName); + /* last SFTP calls before the rekey drain and shutdown assert, so + * do not leave a response pending here */ + (void)sftp_retry_close(ssh, whandle, whandleSz); + (void)sftp_retry_remove(ssh, wrName); } #endif /* WOLFSSH_TEST_INTERNAL */ }