Skip to content

Commit 4ed01d3

Browse files
dgarskedgarske
authored
Merge pull request #905 from yosuke-wolfssl/f_1678
Fix the resources management
2 parents a0e501b + 494b1da commit 4ed01d3

File tree

2 files changed

+117
-44
lines changed

2 files changed

+117
-44
lines changed

‎scripts/sftp.test‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,32 @@ if [ $nonblockingOnly = 0 ]; then
139139
fi
140140
fi
141141

142+
# Test SFTP status failure for non-existing file
143+
if [ $nonblockingOnly = 0 ]; then
144+
echo "Test SFTP status failure for non-existing file"
145+
./examples/echoserver/echoserver -N -1 -R $ready_file &
146+
server_pid=$!
147+
create_port
148+
./examples/sftpclient/wolfsftp -N -u jill -P upthehill -p $port -G -r $PWD/this_file_does_not_exist_12345.txt -l $PWD/test_output.txt > sftp_status_failure.log 2>&1
149+
RESULT=$?
150+
grep "Unable to copy remote file" sftp_status_failure.log > /dev/null
151+
RESULT_MSG=$?
152+
remove_ready_file
153+
rm -f sftp_status_failure.log
154+
rm -f $PWD/test_output.txt
155+
if [ $RESULT -eq 0 ]; then
156+
echo -e "\n\nERROR: Should have failed for non-existing file"
157+
do_cleanup
158+
exit 1
159+
fi
160+
if [ $RESULT_MSG -ne 0 ]; then
161+
echo -e "\n\nERROR: Unexpected failure path while testing missing remote file"
162+
do_cleanup
163+
exit 1
164+
fi
165+
echo "Successfully received failure status packet"
166+
fi
167+
142168
echo -e "\nALL Tests Passed"
143169

144170
exit 0

‎src/wolfsftp.c‎

Lines changed: 91 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -2036,6 +2036,8 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
20362036
word32 idx = 0;
20372037
int m = 0;
20382038
int ret = WS_SUCCESS;
2039+
int fdOpened = 0;
2040+
int outOwnedBySsh = 0;
20392041

20402042
word32 outSz = sizeof(WFD) + UINT32_SZ + WOLFSSH_SFTP_HEADER;
20412043
byte* out = NULL;
@@ -2044,6 +2046,9 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
20442046
char ier[] = "Internal Failure";
20452047
char oer[] = "Open File Error";
20462048
char naf[] = "Not A File";
2049+
#ifdef WOLFSSH_STOREHANDLE
2050+
int handleStored = 0;
2051+
#endif
20472052

20482053
if (ssh == NULL) {
20492054
return WS_BAD_ARGUMENT;
@@ -2075,7 +2080,8 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
20752080
if (GetAndCleanPath(ssh->sftpDefaultPath,
20762081
data + idx, sz, dir, sizeof(dir)) != WS_SUCCESS) {
20772082
WLOG(WS_LOG_SFTP, "Creating path for file to open failed");
2078-
return WS_FATAL_ERROR;
2083+
ret = WS_FATAL_ERROR;
2084+
goto cleanup;
20792085
}
20802086
idx += sz;
20812087

@@ -2128,7 +2134,8 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
21282134
res = naf;
21292135
if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId,
21302136
res, "English", NULL, &outSz) != WS_SIZE_ONLY) {
2131-
return WS_FATAL_ERROR;
2137+
ret = WS_FATAL_ERROR;
2138+
goto cleanup;
21322139
}
21332140
ret = WS_FATAL_ERROR;
21342141
}
@@ -2156,10 +2163,14 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
21562163
res = oer;
21572164
if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, res,
21582165
"English", NULL, &outSz) != WS_SIZE_ONLY) {
2159-
return WS_FATAL_ERROR;
2166+
ret = WS_FATAL_ERROR;
2167+
goto cleanup;
21602168
}
21612169
ret = WS_BAD_FILE_E;
21622170
}
2171+
else {
2172+
fdOpened = 1;
2173+
}
21632174
}
21642175

21652176
#ifdef WOLFSSH_STOREHANDLE
@@ -2170,58 +2181,59 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
21702181
res = ier;
21712182
if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, res,
21722183
"English", NULL, &outSz) != WS_SIZE_ONLY) {
2173-
#ifdef MICROCHIP_MPLAB_HARMONY
2174-
WFCLOSE(ssh->fs, &fd);
2175-
#else
2176-
WCLOSE(ssh->fs, fd);
2177-
#endif
2178-
return WS_FATAL_ERROR;
2184+
ret = WS_FATAL_ERROR;
2185+
goto cleanup;
21792186
}
21802187
ret = WS_FATAL_ERROR;
21812188
}
2189+
else {
2190+
handleStored = 1;
2191+
}
21822192
}
21832193
#endif
21842194

2185-
if (ret == WS_SUCCESS) {
2186-
/* create packet */
2187-
out = (byte*)WMALLOC(outSz, ssh->ctx->heap, DYNTYPE_BUFFER);
2188-
if (out == NULL) {
2189-
#ifdef MICROCHIP_MPLAB_HARMONY
2190-
WFCLOSE(ssh->fs, &fd);
2191-
#else
2192-
WCLOSE(ssh->fs, fd);
2193-
#endif
2194-
return WS_MEMORY_E;
2195-
}
2195+
/* create packet */
2196+
out = (byte*)WMALLOC(outSz, ssh->ctx->heap, DYNTYPE_BUFFER);
2197+
if (out == NULL) {
2198+
ret = WS_MEMORY_E;
2199+
goto cleanup;
21962200
}
2201+
21972202
if (ret == WS_SUCCESS) {
21982203
if (SFTP_CreatePacket(ssh, WOLFSSH_FTP_HANDLE, out, outSz,
21992204
(byte*)&fd, sizeof(WFD)) != WS_SUCCESS) {
2200-
#ifdef MICROCHIP_MPLAB_HARMONY
2201-
WFCLOSE(ssh->fs, &fd);
2202-
#else
2203-
WCLOSE(ssh->fs, fd);
2204-
#endif
2205-
return WS_FATAL_ERROR;
2205+
ret = WS_FATAL_ERROR;
2206+
goto cleanup;
22062207
}
22072208
}
22082209
else {
22092210
if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, res,
22102211
"English", out, &outSz) != WS_SUCCESS) {
2211-
WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER);
2212-
if (fd >= 0) {
2213-
#ifdef MICROCHIP_MPLAB_HARMONY
2214-
WFCLOSE(ssh->fs, &fd);
2215-
#else
2216-
WCLOSE(ssh->fs, fd);
2217-
#endif
2218-
}
2219-
return WS_FATAL_ERROR;
2212+
ret = WS_FATAL_ERROR;
2213+
goto cleanup;
22202214
}
22212215
}
22222216

22232217
/* set send out buffer, "out" is taken by ssh */
22242218
wolfSSH_SFTP_RecvSetSend(ssh, out, outSz);
2219+
outOwnedBySsh = 1;
2220+
2221+
cleanup:
2222+
#ifdef WOLFSSH_STOREHANDLE
2223+
if (ret != WS_SUCCESS && handleStored) {
2224+
(void)SFTP_RemoveHandleNode(ssh, (byte*)&fd, sizeof(WFD));
2225+
}
2226+
#endif
2227+
if (!outOwnedBySsh && out != NULL) {
2228+
WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER);
2229+
}
2230+
if (ret != WS_SUCCESS && fdOpened) {
2231+
#ifdef MICROCHIP_MPLAB_HARMONY
2232+
WFCLOSE(ssh->fs, &fd);
2233+
#else
2234+
WCLOSE(ssh->fs, fd);
2235+
#endif
2236+
}
22252237

22262238
WOLFSSH_UNUSED(ier);
22272239
return ret;
@@ -2238,13 +2250,20 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
22382250
DWORD creationDisp = 0;
22392251
DWORD flagsAndAttrs = 0;
22402252
int ret = WS_SUCCESS;
2253+
int fileHandleOpened = 0;
2254+
int outOwnedBySsh = 0;
22412255

22422256
word32 outSz = sizeof(HANDLE) + UINT32_SZ + WOLFSSH_SFTP_HEADER;
22432257
byte* out = NULL;
22442258

22452259
char* res = NULL;
22462260
char ier[] = "Internal Failure";
22472261
char oer[] = "Open File Error";
2262+
#ifdef WOLFSSH_STOREHANDLE
2263+
int handleStored = 0;
2264+
#endif
2265+
2266+
fileHandle = INVALID_HANDLE_VALUE;
22482267

22492268
if (ssh == NULL) {
22502269
return WS_BAD_ARGUMENT;
@@ -2270,7 +2289,8 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
22702289
if (GetAndCleanPath(ssh->sftpDefaultPath, data + idx, sz, dir, sizeof(dir))
22712290
!= WS_SUCCESS) {
22722291
WLOG(WS_LOG_SFTP, "Creating path for file to open failed");
2273-
return WS_FATAL_ERROR;
2292+
ret = WS_FATAL_ERROR;
2293+
goto cleanup;
22742294
}
22752295
idx += sz;
22762296

@@ -2315,45 +2335,72 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
23152335
res = oer;
23162336
if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, res,
23172337
"English", NULL, &outSz) != WS_SIZE_ONLY) {
2318-
return WS_FATAL_ERROR;
2338+
ret = WS_FATAL_ERROR;
2339+
goto cleanup;
23192340
}
23202341
ret = WS_BAD_FILE_E;
23212342
}
2343+
else {
2344+
fileHandleOpened = 1;
2345+
}
23222346

23232347
#ifdef WOLFSSH_STOREHANDLE
2324-
if (SFTP_AddHandleNode(ssh,
2325-
(byte*)&fileHandle, sizeof(HANDLE), dir) != WS_SUCCESS) {
2348+
if (ret == WS_SUCCESS) {
2349+
if (SFTP_AddHandleNode(ssh,
2350+
(byte*)&fileHandle, sizeof(HANDLE), dir) != WS_SUCCESS) {
23262351
WLOG(WS_LOG_SFTP, "Unable to store handle");
23272352
res = ier;
23282353
if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, res,
23292354
"English", NULL, &outSz) != WS_SIZE_ONLY) {
2330-
return WS_FATAL_ERROR;
2355+
ret = WS_FATAL_ERROR;
2356+
goto cleanup;
23312357
}
23322358
ret = WS_FATAL_ERROR;
2359+
}
2360+
else {
2361+
handleStored = 1;
2362+
}
23332363
}
23342364
#endif
23352365

23362366
/* create packet */
23372367
out = (byte*)WMALLOC(outSz, ssh->ctx->heap, DYNTYPE_BUFFER);
23382368
if (out == NULL) {
2339-
return WS_MEMORY_E;
2369+
ret = WS_MEMORY_E;
2370+
goto cleanup;
23402371
}
2372+
23412373
if (ret == WS_SUCCESS) {
23422374
if (SFTP_CreatePacket(ssh, WOLFSSH_FTP_HANDLE, out, outSz,
23432375
(byte*)&fileHandle, sizeof(HANDLE)) != WS_SUCCESS) {
2344-
return WS_FATAL_ERROR;
2376+
ret = WS_FATAL_ERROR;
2377+
goto cleanup;
23452378
}
23462379
}
23472380
else {
23482381
if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, res,
23492382
"English", out, &outSz) != WS_SUCCESS) {
2350-
WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER);
2351-
return WS_FATAL_ERROR;
2383+
ret = WS_FATAL_ERROR;
2384+
goto cleanup;
23522385
}
23532386
}
23542387

23552388
/* set send out buffer, "out" is taken by ssh */
23562389
wolfSSH_SFTP_RecvSetSend(ssh, out, outSz);
2390+
outOwnedBySsh = 1;
2391+
2392+
cleanup:
2393+
#ifdef WOLFSSH_STOREHANDLE
2394+
if (ret != WS_SUCCESS && handleStored) {
2395+
(void)SFTP_RemoveHandleNode(ssh, (byte*)&fileHandle, sizeof(HANDLE));
2396+
}
2397+
#endif
2398+
if (!outOwnedBySsh && out != NULL) {
2399+
WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER);
2400+
}
2401+
if (ret != WS_SUCCESS && fileHandleOpened) {
2402+
CloseHandle(fileHandle);
2403+
}
23572404

23582405
WOLFSSH_UNUSED(ier);
23592406
return ret;

0 commit comments

Comments
 (0)