Skip to content

Commit ea530fe

Browse files
Emma Stenslandejohnstown
authored andcommitted
wolfsftp: Improve POSIX and Windows write/append handling
Ensures EOF appending on all POSIX systems and fails short writes on Windows. Consolidates the Windows SFTP flag matrix testing with shared helpers.
1 parent 5a1ff71 commit ea530fe

3 files changed

Lines changed: 252 additions & 353 deletions

File tree

src/wolfsftp.c

Lines changed: 41 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2347,21 +2347,27 @@ static DWORD SFTP_WinCreationDisp(word32 reason)
23472347
DWORD disp;
23482348

23492349
if (reason & WOLFSSH_FXF_CREAT) {
2350-
if (reason & WOLFSSH_FXF_EXCL)
2350+
if (reason & WOLFSSH_FXF_EXCL) {
23512351
disp = CREATE_NEW;
2352-
else if (reason & WOLFSSH_FXF_TRUNC)
2352+
}
2353+
else if (reason & WOLFSSH_FXF_TRUNC) {
23532354
disp = CREATE_ALWAYS;
2354-
else
2355+
}
2356+
else {
23552357
disp = OPEN_ALWAYS;
2358+
}
23562359
}
23572360
else {
23582361
/* TRUNCATE_EXISTING requires GENERIC_WRITE in dwDesiredAccess or
2359-
* CreateFile() fails with ERROR_INVALID_PARAMETER; without WRITE
2360-
* there is no way to truncate, so fall back to OPEN_EXISTING. */
2361-
if ((reason & WOLFSSH_FXF_TRUNC) && (reason & WOLFSSH_FXF_WRITE))
2362+
* CreateFile() fails with ERROR_INVALID_PARAMETER. TRUNC without
2363+
* WRITE is deliberately ignored and the open succeeds untruncated,
2364+
* as O_RDONLY|O_TRUNC does on most POSIX systems. */
2365+
if ((reason & WOLFSSH_FXF_TRUNC) && (reason & WOLFSSH_FXF_WRITE)) {
23622366
disp = TRUNCATE_EXISTING;
2363-
else
2367+
}
2368+
else {
23642369
disp = OPEN_EXISTING;
2370+
}
23652371
}
23662372

23672373
return disp;
@@ -2692,12 +2698,15 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
26922698
}
26932699
#endif
26942700

2695-
if (reason & WOLFSSH_FXF_READ)
2701+
if (reason & WOLFSSH_FXF_READ) {
26962702
desiredAccess |= GENERIC_READ;
2697-
if (reason & WOLFSSH_FXF_WRITE)
2703+
}
2704+
if (reason & WOLFSSH_FXF_WRITE) {
26982705
desiredAccess |= GENERIC_WRITE;
2699-
if (reason & WOLFSSH_FXF_APPEND)
2706+
}
2707+
if (reason & WOLFSSH_FXF_APPEND) {
27002708
desiredAccess |= FILE_APPEND_DATA;
2709+
}
27012710

27022711
creationDisp = SFTP_WinCreationDisp(reason);
27032712

@@ -4265,6 +4274,7 @@ int wolfSSH_SFTP_RecvWrite(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
42654274
WFD fd = 0;
42664275
int ret = WS_SUCCESS;
42674276
int rc;
4277+
int isAppend = 0;
42684278
word32 idx = 0;
42694279
word32 ofst[2] = {0,0};
42704280

@@ -4279,6 +4289,7 @@ int wolfSSH_SFTP_RecvWrite(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
42794289
if (ssh == NULL) {
42804290
return WS_BAD_ARGUMENT;
42814291
}
4292+
WOLFSSH_UNUSED(isAppend); /* only read on ports that define WWRITE */
42824293

42834294
WLOG(WS_LOG_SFTP, "Receiving WOLFSSH_FTP_WRITE");
42844295

@@ -4313,6 +4324,7 @@ int wolfSSH_SFTP_RecvWrite(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
43134324
}
43144325
else {
43154326
fd = fileEntry->fd;
4327+
isAppend = fileEntry->isAppend;
43164328
}
43174329
}
43184330
}
@@ -4335,8 +4347,20 @@ int wolfSSH_SFTP_RecvWrite(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
43354347
/* Retry while WPWRITE makes forward progress; bail on error
43364348
* or zero return to avoid spinning on a stuck backend. */
43374349
while (written < strSz) {
4338-
ret = WPWRITE(ssh->fs, fd, (byte*)str + written,
4339-
strSz - written, ofst);
4350+
#ifdef WWRITE
4351+
if (isAppend) {
4352+
/* FXF_APPEND: the offset is ignored and the O_APPEND
4353+
* fd puts the write at EOF. pwrite() would honor the
4354+
* offset on every POSIX system but Linux. */
4355+
ret = WWRITE(ssh->fs, fd, (byte*)str + written,
4356+
strSz - written);
4357+
}
4358+
else
4359+
#endif
4360+
{
4361+
ret = WPWRITE(ssh->fs, fd, (byte*)str + written,
4362+
strSz - written, ofst);
4363+
}
43404364
if (ret <= 0) {
43414365
break;
43424366
}
@@ -4459,8 +4483,8 @@ int wolfSSH_SFTP_RecvWrite(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
44594483
* non-atomic read-modify-write: the file is shared FILE_SHARE_WRITE,
44604484
* so concurrent appenders would resolve the same offset and overwrite
44614485
* each other. Setting both OVERLAPPED offset fields to 0xFFFFFFFF
4462-
* tells WriteFile() to append atomically at end of file, matching the
4463-
* POSIX O_APPEND path. */
4486+
* tells WriteFile() to append atomically at end of file, matching
4487+
* the POSIX O_APPEND path. */
44644488
if (isAppend) {
44654489
offset.Offset = 0xFFFFFFFF;
44664490
offset.OffsetHigh = 0xFFFFFFFF;
@@ -4473,7 +4497,9 @@ int wolfSSH_SFTP_RecvWrite(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
44734497
return WS_BUFFER_E;
44744498
}
44754499

4476-
if (WriteFile(fd, str, strSz, &bytesWritten, &offset) == 0) {
4500+
/* A short write is a failure too, as on the POSIX side. */
4501+
if (WriteFile(fd, str, strSz, &bytesWritten, &offset) == 0 ||
4502+
bytesWritten != strSz) {
44774503
WLOG(WS_LOG_SFTP, "Error writing to file");
44784504
res = err;
44794505
type = WOLFSSH_FTP_FAILURE;

0 commit comments

Comments
 (0)