https://gitlab.synchro.net/main/sbbs/-/commit/b795545afa5e0cc6250e0fb2
Modified Files:
src/sbbs3/ftpsrvr.cpp
Log Message:
This is C++ now, value-initialize since we can't zero-initialize

Same-same, but different, but still same.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/commit/83252750de3b2354f2039a18
Modified Files:
src/sbbs3/ftpsrvr.cpp
Log Message:
Log better errors/warnings when upload directory doesn't exist

or doesn't have enough free space

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/commit/815458f875915bbb4cfb2b18
Modified Files:
src/sbbs3/ftpsrvr.cpp
Log Message:
s/Unallowed/Disallowed

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/commit/9d440000f790304469d309db
Modified Files:
src/sbbs3/ftpsrvr.cpp
Log Message:
load file data from filebase when updating (uploading) an existing file

updatefile() assumes the file was already loaded. If it was not, you would get an error updating the file, e.g.
ftp 3220 <Guest> !DATA ERROR -101 updating file (filename.txt) in database

Log the restart offset both when the client sets it (with the "REST" command) and in the "uploading" log message, helpful in debugging unique upload issues.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/commit/cd30db7872f8ad631aabc16f
Modified Files:
src/sbbs3/ftpsrvr.cpp
Log Message:
Case insensitive file-existence check before allowing upload

The Synchronet filebases are index-searched case-insensitively, so don't allow a dupe file (only differing in case) to be uploaded.

This would have already been disallowed/prevented on Windows, but not case sensitive file systems (e.g. *nix).

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/commit/16a9d6c01d1a25c8158210ed
Modified Files:
src/sbbs3/ftpsrvr.cpp
Log Message:
More accurate log/response messages about maximum file size allowed

The max file size allowed for upload can change based on available disk space and the sysop-configured minimum amount of free disk space. Report the correct numbers.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/commit/bb760f2009bfc0b46c250c05
Modified Files:
src/sbbs3/ftpsrvr.cpp
Log Message:
Fix the delete permissions flags in the MLST/MLSD support

In commit d4deb4b3, the 'R' and 'D' restrictions were conflated.

I just notice this in code review: the 'D' restriction has nothing to do with removing/deleting files.

Also, a non-R-restricted user can delete any files they've uploaded.
Fixed that too.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/commit/f28f210b217239031baa9af1
Modified Files:
src/sbbs3/ftpsrvr.cpp
Log Message:
sbbs3 ftpsrvr: modernize legacy BOOL/TRUE/FALSE to bool/true/false

Mechanical conversion across the file: function signatures, local
variables (incl. volatile flags shared across xfer threads), xfer_t
struct fields, and all literal arguments/comparisons. External callees (load_cfg, matchuser, getnodedat, wildmatchi, seteuid callback) already
declare bool parameters, so no ABI change. Also collapse direxist()'s
if/else to a single return.

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/commit/8a7ff359a230d7820816acb5
Modified Files:
src/sbbs3/ftpsrvr.cpp
Log Message:
Use int (32-bits) instead of bool (8-bits) for setsockopt(... REUSEADDR)

Fix issue #1137 (bug introduced in commit f28f210b)

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/commit/f1f4ac3fce5a360a4f2446ed
Modified Files:
src/sbbs3/ftpsrvr.cpp
Log Message:
Use int (32-bit) instead of bool (8-bit) for setsockopt(... NODELAY)

Similar to fix for issue #1137

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/commit/769599ebba788f26253334f3
Modified Files:
src/sbbs3/ftpsrvr.cpp
Log Message:
ftpsrvr: handle return values in send/receive_thread and sock_recvbyte (CIDs 643130, 643142, 643143)

- send_thread/receive_thread: log a warning if fseeko to xfer.filepos
fails so a seek error is no longer silent (downstream I/O would
proceed at the wrong offset). Continues into the transfer loop
either way to preserve existing behavior.
- sock_recvbyte: cast the inner cryptSetAttribute (re-arming the read
timeout) to void; this is best-effort and the matching call above
it is already error-checked.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/commit/3f0508da6bce06fb59f76358
Modified Files:
src/sbbs3/ftpsrvr.cpp
Log Message:
ftpsrvr: fix file-descriptor leaks in directory/index handlers

Two distinct leaks of per-command temp-file descriptors, both surfacing
as "ERROR 24 (Too many open files)" once the process exhausts its
descriptor table -- after which every temp-file open (LIST/MLSD .lst,
index .ndx, QWK packets, and even unrelated subsystems sharing the
process) fails in a storm.

1) MDTM <index-file> (getdate) opened a temp .ndx file via ftp_tmpfname()
but never closed it: the lone fclose(fp) lives only in the !getdate
(download) branch. Every MDTM on the dynamically-generated index file
(startup->index_file_name, e.g. 00index) leaked one descriptor and
orphaned an sbbstemp/SBBS_FTP.*.ndx file. FTP clients and crawlers
MDTM each listed entry, so these accumulated steadily.
Fix: don't open the temp file for a getdate/MDTM request at all (it is
only needed to generate listing content); move the fopen into the
!getdate branch.
Introduced in 63e5e08f1c (2007-02-11), which made the index fopen
unconditional and added early-outs for SIZE (continue) and MDTM
(fall-through) that both bypassed the fclose. ff9ae78f44 (2007-11-28)
fixed the SIZE half by moving its check above the fopen, but left the
MDTM path leaking -- for ~18 years.

2) The MLSD, LIST and index-generation handlers "continue" out of the
command loop on smb_open_dir() failure without closing the already-
opened temp fp (and, for LIST/MLSD, without servicing the already-
announced data connection). Triggers whenever a directory's .shd is
locked or contended.
Fix: close fp on the failure path; for LIST/MLSD also run the normal
filexfer() cleanup so the data connection is serviced and the temp
file removed.
Introduced in 925e3b0a2 (2021-04-04), which migrated FTP directory
listings to smb_open_dir()/loadfiles().

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/commit/19c8264107387b59dfe2dc70
Modified Files:
src/sbbs3/ftpsrvr.cpp
Log Message:
ftpsrvr: add errprintf() and route LOG_ERR/LOG_CRIT through it for de-dup

The FTP server logged every error via lprintf(LOG_ERR/LOG_CRIT, ...),
which writes each record to error.log with no repeat suppression. Under file-descriptor exhaustion (EMFILE) a single busy server flooded
error.log with thousands of near-identical entries.

Mirror the mailsrvr/websrvr/services pattern: add a static errprintf()
that calls repeated_error(line, function) and demotes a consecutive
repeat of the same site to LOG_WARNING (which falls out of error.log).
Convert all 48 LOG_ERR/LOG_CRIT lprintf() call sites to
errprintf(..., WHERE, ...).

This is log de-duplication only; it does not address the underlying
descriptor exhaustion. Note repeated_error() tracks only the single
most-recent (line, function), so interleaved failures from many
concurrent sessions are only partially collapsed.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/commit/308b735e39c6aaefec799fd0
Modified Files:
src/sbbs3/ftpsrvr.cpp
Log Message:
ftpsrvr: remove duplicate ndx temp-file fopen() from de-dup commit

Commit 19c826410 ("add errprintf() and route LOG_ERR/LOG_CRIT through it
for de-dup") was meant only to convert the index-file fopen() failure's lprintf(LOG_ERR,...) to errprintf(..., WHERE, ...). A rebase merge
resolution instead inserted a second, redundant fopen() of the "ndx"
temp file ahead of "success = true;" (giving the new copy the errprintf)
while leaving the original lprintf() open untouched.

That left two opens:
- the getdate (MDTM-style) path opened a temp file it never uses --
contradicting the "No temp file needed for a modification-time query"
comment -- and never set tmpfile/delfile, leaking both the descriptor
and the temp file (ironic in an EMFILE-motivated change);
- the !getdate path opened the file twice, leaking the first descriptor.

Drop the spurious block and keep the de-dup conversion on the surviving
open inside the else branch, restoring the original single-open logic.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/commit/795b48fcf3c4c6d6f7dbd2f5
Modified Files:
src/sbbs3/ftpsrvr.cpp
Log Message:
ftpsrvr: don't use a file record that loadfile() failed to fill

Three sites loaded a file record and used it without checking whether
the load had succeeded. Two of them left the record uninitialized, so a
failed load meant reading and then freeing indeterminate memory:

- the download credit check took the file's cost from an uninitialized
struct, giving an arbitrary verdict, and then called
smb_freefilemem() on its wild pointers
- the upload resume check left the same struct uninitialized along the
path that clears 'append' and falls through to smb_freefilemem()
- the delete uploader check zero-initialized the record but ignored the
result, leaving 'from' NULL for stricmp() to dereference

loadfile() re-opens the message base, so it can fail even though
findfile() has just succeeded: a lock timeout, or the record removed by
another session in between.

Each site now reports the failure and refuses the request. The record's
'from' pointer is checked before use as well, since
set_convenience_ptr() assigns it only when the header carries a SENDER
field: it can be NULL after a successful load, and the existing
"unknown user" fallback dereferenced it to test for an empty string.

The 'filedat' guards around two of those loads go with them. It is
assigned once and the request is already refused where it is false, so
the guarded fallback that charged by file length could never run, and
the guard's only remaining effect was to let an unloaded record reach
the checks.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/commit/f650fc4d0d59745c52173537
Modified Files:
src/sbbs3/ftpsrvr.cpp
Log Message:
ftpsrvr: reset filedat for each RETR/SIZE/MDTM/DELE command

filedat records whether the requested file was found in the directory's database. It is assigned only in the branch that serves a file base
directory, but read further down in code common to every branch,
including those serving a QWK packet, a generated index file, or an
aliased local path.

A session that had already run a file base command therefore left it
set, so a later DELE of one of those non-database files reached
removefile() with a stale true and removed whichever record in the
session's current directory happened to carry the deleted file's name.
An invalid directory number is rejected by smb_init_dir(), so this
deleted a record rather than crashing.

The other per-command flags (getdate, getsize, delecmd) are cleared
where the command is recognized. Clear filedat with them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/commit/b73ac2b9c7affe55b51c47c5
Modified Files:
src/sbbs3/ftpsrvr.cpp
Log Message:
ftpsrvr: pack QWK and import REP packets in-process via qwkjob

A QWK download (RETR <qwk-id>.qwk, and SIZE/MDTM of that name) used to
create a data/pack<user#>.<host>.now semaphore and poll for up to
QwkTimeout seconds while the Terminal Server's event thread built the
packet. An FTP-only instance, or one whose event thread was busy or
disabled, timed out with "451 Time-out waiting for packet creation."
The control thread now calls qwkjob_pack() itself:

- no new messages: 550 (unchanged); logged-in at a terminal node: 550
Logged-in elsewhere
- the user's pack lock is held (another session, script or host is
packing or importing for the same user): immediate 451 "Packet
creation already in progress" (unchanged text)
- the per-process job cap is reached: retry once a second for up to
QwkTimeout, then the unchanged time-out 451, so QwkTimeout keeps its
meaning as the longest a RETR waits
- if the client disconnects, the pack still completes and the packet
stays in data/file/ for the next RETR

A REP upload (STOR <qwk-id>.rep) is now received into <temp>/rep/<user#>.<socket>.rep and imported by the receive thread
after the file is closed and before the 226, instead of being left in data/file/ for the event thread. A packet that fails to import is kept
as data/file/<user#>.rep.<hex-time>.bad (an empty one is removed) and
the client still gets 226, since uploading it again would not help.
When the user's lock is held or the job cap is reached, the REP is
queued for the event thread's sweep as data/file/<user#>.rep, or <user#>.<hex-time>.rep if a REP is already pending; temp and data can be
on different volumes, so a failed rename falls back to a copy under a
name the sweep ignores, then a rename. The control connection's
inactivity clock is reset after the import.

QWK job log lines are prefixed "ftp" and go through the server's own
log function (so LOG_ERR lines reach error.log and MQTT).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/commit/7dd262f012ca4801e1322c22
Modified Files:
src/sbbs3/ftpsrvr.cpp
Log Message:
ftpsrvr: only treat the QWK reply packet upload as a REP

The receive thread decided an upload was a QWK reply packet from its
directory number (-1), but a sysop's local file system STOR/APPE is
also received with directory -1. Such an upload was then run through
the REP import: moved to data/file/<user#>.rep.<hex-time>.bad, deleted
if empty, or queued as data/file/<user#>.rep (or imported, if it was a
valid REP), so the file vanished from its destination while the client
got "226 Upload complete". The STOR <qwk-id>.rep branch now marks the
transfer explicitly and passes that to filexfer().

Also:
- Stage a cross-volume copy of a queued REP as <user#>.<socket>.tmp
rather than .reptmp: on Windows "*.rep" also matches 8.3 short names,
whose extension for .reptmp is .REP, so the sweep could pick up a
partial copy.
- Report EEXIST when no free queue name is found, instead of whatever
errno was left over.
- Reset the control connection's inactivity clock after a QWK pack, so
a pack that takes longer than MaxInactivity doesn't drop the session
on the next read.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net

https://gitlab.synchro.net/main/sbbs/-/commit/8371e30134a3dd298c731205
Modified Files:
src/sbbs3/ftpsrvr.cpp
Log Message:
ftpsrvr: keep a REP import off the control thread's stack

receive_thread() imports a received REP before its 226 reply, which can
take a while, and read the user number and alias through xfer->user,
which points into ctrl_thread()'s stack. When the server is going down
(ftp_set is NULL), ctrl_thread() waited only 2 seconds for the transfer
before returning, leaving the import to read a dead stack frame and, after
it, to write *xfer.lastactive there.

The transfer now carries the user number and alias by value for the
import, the post-import activity stamp is skipped during shutdown like
the existing *inprogress update, and ctrl_thread() keeps waiting (up to
60 seconds) while the transfer is in progress. The receive thread is
counted in thread_count, so the server's cleanup still waits for it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net