Skip to content

Commit 3d8e1e0

Browse files
wolfsftp: keep the local file when resuming a get
- The Windows local open in wolfSSH_SFTP_Get() passes OPEN_EXISTING when the write offset is nonzero and CREATE_ALWAYS otherwise, and drops FILE_APPEND_DATA from the desired access. A new DWORD creationDisp replaces the block-scoped desiredAccess. - That open reports INVALID_HANDLE_VALUE as WS_BAD_FILE_E and moves to STATE_GET_CLEANUP; the OVERLAPPED offset is set from gOfst on every open rather than only when resuming. - STATE_GET_LOOKUP_OFFSET stats the local destination when the saved offset is nonzero and clears the offset unless the reported size matches it exactly. WS_SFTP_GET_STATE carries the attributes it reads. - tests/api.c adds test_wolfSSH_SFTP_GetResume() and its sftpGetToCompletion() helper, five cases over the resume paths, built where the hosted file wrappers are available. - .gitignore covers the wolfssh_12547_*.tmp fixtures that test leaves behind on an aborted run. Issue: F-12547
1 parent bfe6fe0 commit 3d8e1e0

3 files changed

Lines changed: 186 additions & 17 deletions

File tree

.gitignore

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,10 @@ keys/*-ossh-*cert.pub
8585
random-test.txt
8686
random-test-result.txt
8787
test.dat
88-
# Scratch file the api tests write in the working directory. Removed on a
89-
# clean run, left behind when one aborts.
88+
# Scratch files the api and regression tests write in the working
89+
# directory. Removed on a clean run, left behind when one aborts.
9090
ossh-cert-line.tmp
91-
# Same for the regression tests' known_hosts fixtures, named by pid.
92-
wolfssh_kh_*.tmp
91+
wolfssh_*.tmp
9392
regress_known_hosts*.tmp
9493

9594
# test output

src/wolfsftp.c

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9775,6 +9775,9 @@ int wolfSSH_SFTP_Get(WOLFSSH* ssh, char* from,
97759775
WS_SFTP_GET_STATE* state = NULL;
97769776
int sz;
97779777
int ret = WS_SUCCESS;
9778+
#ifdef USE_WINDOWS_API
9779+
DWORD creationDisp;
9780+
#endif
97789781

97799782
WLOG(WS_LOG_SFTP, "Entering wolfSSH_SFTP_Get()");
97809783
if (ssh == NULL || from == NULL || to == NULL)
@@ -9853,6 +9856,23 @@ int wolfSSH_SFTP_Get(WOLFSSH* ssh, char* from,
98539856
/* if resuming then check for saved offset */
98549857
if (resume) {
98559858
wolfSSH_SFTP_GetOfst(ssh, from, to, state->gOfst);
9859+
9860+
/* a saved offset is only usable if the local file still
9861+
* holds exactly that many bytes */
9862+
if (state->gOfst[0] > 0 || state->gOfst[1] > 0) {
9863+
WMEMSET(&state->attrib, 0, sizeof(state->attrib));
9864+
if (SFTP_GetAttributes(ssh->fs, to, &state->attrib, 1,
9865+
ssh->ctx->heap) != WS_SUCCESS
9866+
|| (state->attrib.flags
9867+
& WOLFSSH_FILEATRB_SIZE) == 0
9868+
|| state->attrib.sz[0] != state->gOfst[0]
9869+
|| state->attrib.sz[1] != state->gOfst[1]) {
9870+
WLOG(WS_LOG_SFTP,
9871+
"Size does not match, starting over");
9872+
state->gOfst[0] = 0;
9873+
state->gOfst[1] = 0;
9874+
}
9875+
}
98569876
}
98579877
state->state = STATE_GET_OPEN_LOCAL;
98589878
FALL_THROUGH;
@@ -9865,20 +9885,23 @@ int wolfSSH_SFTP_Get(WOLFSSH* ssh, char* from,
98659885
else
98669886
ret = WFOPEN(ssh->fs, &state->fl, to, WOLFSSH_O_WRONLY);
98679887
#elif defined(USE_WINDOWS_API)
9868-
{
9869-
DWORD desiredAccess = GENERIC_WRITE;
9870-
if (state->gOfst[0] > 0 || state->gOfst[1] > 0)
9871-
desiredAccess |= FILE_APPEND_DATA;
9872-
state->fileHandle = WS_CreateFileA(to, desiredAccess,
9873-
(FILE_SHARE_DELETE | FILE_SHARE_READ |
9874-
FILE_SHARE_WRITE), CREATE_ALWAYS,
9875-
FILE_ATTRIBUTE_NORMAL, ssh->ctx->heap);
9876-
}
9877-
if (resume) {
9878-
WMEMSET(&state->offset, 0, sizeof(OVERLAPPED));
9879-
state->offset.OffsetHigh = state->gOfst[1];
9880-
state->offset.Offset = state->gOfst[0];
9888+
creationDisp = CREATE_ALWAYS;
9889+
if (state->gOfst[0] > 0 || state->gOfst[1] > 0)
9890+
creationDisp = OPEN_EXISTING;
9891+
state->fileHandle = WS_CreateFileA(to, GENERIC_WRITE,
9892+
(FILE_SHARE_DELETE | FILE_SHARE_READ |
9893+
FILE_SHARE_WRITE), creationDisp,
9894+
FILE_ATTRIBUTE_NORMAL, ssh->ctx->heap);
9895+
if (state->fileHandle == INVALID_HANDLE_VALUE) {
9896+
WLOG(WS_LOG_SFTP, "Unable to open output file");
9897+
ssh->error = WS_BAD_FILE_E;
9898+
ret = WS_FATAL_ERROR;
9899+
state->state = STATE_GET_CLEANUP;
9900+
continue;
98819901
}
9902+
WMEMSET(&state->offset, 0, sizeof(OVERLAPPED));
9903+
state->offset.OffsetHigh = state->gOfst[1];
9904+
state->offset.Offset = state->gOfst[0];
98829905
#else
98839906
if (state->gOfst[0] > 0 || state->gOfst[1] > 0)
98849907
ret = WFOPEN(ssh->fs, &state->fl, to, "ab");

tests/api.c

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5901,6 +5901,24 @@ static int sftpPutToCompletion(WOLFSSH* ssh, char* from, char* to, byte resume)
59015901

59025902
return ret;
59035903
}
5904+
5905+
5906+
/* Drives one wolfSSH_SFTP_Get() to a terminal result. */
5907+
static int sftpGetToCompletion(WOLFSSH* ssh, char* from, char* to, byte resume)
5908+
{
5909+
int ret = WS_FATAL_ERROR;
5910+
int tries;
5911+
5912+
for (tries = 0; tries < SFTP_MAX_RETRY_TRIES; tries++) {
5913+
ret = wolfSSH_SFTP_Get(ssh, from, to, resume, NULL);
5914+
if (ret == WS_SUCCESS ||
5915+
!sftp_error_is_notice(wolfSSH_get_error(ssh))) {
5916+
break;
5917+
}
5918+
}
5919+
5920+
return ret;
5921+
}
59045922
#endif /* !NO_FILESYSTEM && !WOLFSSH_USER_FILESYSTEM && !WOLFSSH_ZEPHYR */
59055923

59065924

@@ -6040,6 +6058,133 @@ static void test_wolfSSH_SFTP_PutResume(void)
60406058
}
60416059

60426060

6061+
/* A resumed get must keep the bytes already at the local destination, and
6062+
* must only resume onto one whose size matches the saved offset. The
6063+
* echoserver shares this process, so the "remote" file is local. */
6064+
static void test_wolfSSH_SFTP_GetResume(void)
6065+
{
6066+
/* staging both files needs the hosted fopen()/getcwd() wrappers */
6067+
#if !defined(NO_FILESYSTEM) && !defined(WOLFSSH_USER_FILESYSTEM) && \
6068+
!defined(WOLFSSH_ZEPHYR)
6069+
func_args ser;
6070+
tcp_ready ready;
6071+
int argsCount;
6072+
WS_SOCKET_T clientFd;
6073+
6074+
const char* args[10];
6075+
WOLFSSH_CTX* ctx = NULL;
6076+
WOLFSSH* ssh = NULL;
6077+
6078+
THREAD_TYPE serThread;
6079+
6080+
byte src[SFTP_PUT_RESUME_SZ];
6081+
byte stale[SFTP_PUT_RESUME_SZ + SFTP_PUT_RESUME_OFST];
6082+
byte expect[SFTP_PUT_RESUME_SZ];
6083+
word32 ofst[2];
6084+
char srcName[] = "wolfssh_12547_src.tmp";
6085+
char dstName[] = "wolfssh_12547_dst.tmp";
6086+
6087+
WMEMSET(&ser, 0, sizeof(func_args));
6088+
6089+
argsCount = 0;
6090+
args[argsCount++] = ".";
6091+
args[argsCount++] = "-1";
6092+
args[argsCount++] = "-p";
6093+
args[argsCount++] = "0";
6094+
ser.argv = (char**)args;
6095+
ser.argc = argsCount;
6096+
ser.signal = &ready;
6097+
InitTcpReady(ser.signal);
6098+
ThreadStart(echoserver_test, (void*)&ser, &serThread);
6099+
WaitTcpReady(&ready);
6100+
6101+
sftp_client_connect(&ctx, &ssh, ready.port);
6102+
AssertNotNull(ctx);
6103+
AssertNotNull(ssh);
6104+
6105+
sftpPutFillPattern(src, (word32)sizeof(src));
6106+
WMEMSET(stale, 0xFF, sizeof(stale));
6107+
AssertIntEQ(sftpPutWriteFile(srcName, src, (word32)sizeof(src)), 0);
6108+
6109+
/* the saved offset matches the destination, so the download picks up
6110+
* where it left off. Staging a prefix unlike the source, and expecting it
6111+
* back untouched, is what separates a resume from a full re-download. */
6112+
AssertIntEQ(sftpPutWriteFile(dstName, stale, SFTP_PUT_RESUME_OFST), 0);
6113+
WMEMCPY(expect, stale, SFTP_PUT_RESUME_OFST);
6114+
WMEMCPY(expect + SFTP_PUT_RESUME_OFST, src + SFTP_PUT_RESUME_OFST,
6115+
sizeof(expect) - SFTP_PUT_RESUME_OFST);
6116+
ofst[0] = SFTP_PUT_RESUME_OFST;
6117+
ofst[1] = 0;
6118+
AssertIntEQ(wolfSSH_SFTP_SaveOfst(ssh, srcName, dstName, ofst),
6119+
WS_SUCCESS);
6120+
AssertIntEQ(sftpGetToCompletion(ssh, srcName, dstName, 1), WS_SUCCESS);
6121+
AssertIntEQ(sftpPutFileMatches(dstName, expect, (word32)sizeof(expect)),
6122+
0);
6123+
6124+
/* the destination is gone, so the whole file is fetched again */
6125+
WREMOVE(NULL, dstName);
6126+
ofst[0] = SFTP_PUT_RESUME_OFST;
6127+
ofst[1] = 0;
6128+
AssertIntEQ(wolfSSH_SFTP_SaveOfst(ssh, srcName, dstName, ofst),
6129+
WS_SUCCESS);
6130+
AssertIntEQ(sftpGetToCompletion(ssh, srcName, dstName, 1), WS_SUCCESS);
6131+
AssertIntEQ(sftpPutFileMatches(dstName, src, (word32)sizeof(src)), 0);
6132+
6133+
/* the destination is shorter than the saved offset, so the whole file is
6134+
* fetched again */
6135+
AssertIntEQ(sftpPutWriteFile(dstName, stale, SFTP_PUT_RESUME_OFST / 2), 0);
6136+
ofst[0] = SFTP_PUT_RESUME_OFST;
6137+
ofst[1] = 0;
6138+
AssertIntEQ(wolfSSH_SFTP_SaveOfst(ssh, srcName, dstName, ofst),
6139+
WS_SUCCESS);
6140+
AssertIntEQ(sftpGetToCompletion(ssh, srcName, dstName, 1), WS_SUCCESS);
6141+
AssertIntEQ(sftpPutFileMatches(dstName, src, (word32)sizeof(src)), 0);
6142+
6143+
/* the destination is longer than the saved offset, so the whole file is
6144+
* fetched again */
6145+
AssertIntEQ(sftpPutWriteFile(dstName, stale, SFTP_PUT_RESUME_OFST * 2), 0);
6146+
ofst[0] = SFTP_PUT_RESUME_OFST;
6147+
ofst[1] = 0;
6148+
AssertIntEQ(wolfSSH_SFTP_SaveOfst(ssh, srcName, dstName, ofst),
6149+
WS_SUCCESS);
6150+
AssertIntEQ(sftpGetToCompletion(ssh, srcName, dstName, 1), WS_SUCCESS);
6151+
AssertIntEQ(sftpPutFileMatches(dstName, src, (word32)sizeof(src)), 0);
6152+
6153+
/* a plain get still truncates a longer destination */
6154+
AssertIntEQ(sftpPutWriteFile(dstName, stale, (word32)sizeof(stale)), 0);
6155+
AssertIntEQ(sftpGetToCompletion(ssh, srcName, dstName, 0), WS_SUCCESS);
6156+
AssertIntEQ(sftpPutFileMatches(dstName, src, (word32)sizeof(src)), 0);
6157+
6158+
WREMOVE(NULL, dstName);
6159+
WREMOVE(NULL, srcName);
6160+
6161+
/* take care of re-keying state before shutdown call */
6162+
while (wolfSSH_get_error(ssh) == WS_REKEYING) {
6163+
wolfSSH_worker(ssh, NULL);
6164+
}
6165+
6166+
argsCount = AbsorbBenignReset(ssh, wolfSSH_shutdown(ssh));
6167+
#if DEFAULT_HIGHWATER_MARK < 8000
6168+
if (argsCount == WS_REKEYING) {
6169+
argsCount = WS_SUCCESS;
6170+
}
6171+
#endif
6172+
AssertIntEQ(argsCount, WS_SUCCESS);
6173+
6174+
clientFd = wolfSSH_get_fd(ssh);
6175+
WCLOSESOCKET(clientFd);
6176+
6177+
wolfSSH_free(ssh);
6178+
wolfSSH_CTX_free(ctx);
6179+
#ifdef WOLFSSH_ZEPHYR
6180+
k_sleep(Z_TIMEOUT_TICKS(100));
6181+
#endif
6182+
ThreadJoin(serThread);
6183+
FreeTcpReady(&ready);
6184+
#endif /* !NO_FILESYSTEM && !WOLFSSH_USER_FILESYSTEM && !WOLFSSH_ZEPHYR */
6185+
}
6186+
6187+
60436188
#else /* WOLFSSH_SFTP && !NO_WOLFSSH_CLIENT && !SINGLE_THREADED */
60446189
static void test_wolfSSH_SFTP_SendReadPacket(void) { ; }
60456190
static void test_wolfSSH_SFTP_PartialSend(void) { ; }
@@ -6051,6 +6196,7 @@ static void test_wolfSSH_SFTP_SetConfinePath(void) { ; }
60516196
static void test_wolfSSH_SFTP_SetDefaultPath(void) { ; }
60526197
static void test_wolfSSH_SFTP_SaveOfst(void) { ; }
60536198
static void test_wolfSSH_SFTP_PutResume(void) { ; }
6199+
static void test_wolfSSH_SFTP_GetResume(void) { ; }
60546200
#endif /* WOLFSSH_SFTP && !NO_WOLFSSH_CLIENT && !SINGLE_THREADED */
60556201

60566202

@@ -7999,6 +8145,7 @@ int wolfSSH_ApiTest(int argc, char** argv)
79998145
test_wolfSSH_SFTP_SetDefaultPath();
80008146
test_wolfSSH_SFTP_SaveOfst();
80018147
test_wolfSSH_SFTP_PutResume();
8148+
test_wolfSSH_SFTP_GetResume();
80028149

80038150
/* Either SCP or SFTP */
80048151
test_wolfSSH_RealPath();

0 commit comments

Comments
 (0)