ftpd: truncate the file on STOR instead of overwriting in place #706

2026-09-18T18:16:03Z

igorsh commented

2026-09-18 18:16:03 +00:00

cmdSTOR opens the destination with O_CREATE + O_WRITE, and libio only
truncates on write when the position is already at EOF (libio.asm:314),
which never happens here. Replacing a longer file with a shorter one
therefore kept the old tail: 4096-byte A overwritten by 1024-byte B
produced a 4096-byte file holding B followed by A[1024:4096], and
overwriting with zero bytes left the file completely untouched.

Call file.truncate (SF 70.4, size = Position = 0) right after a
successful open so that STOR replaces the file, as RFC 959 requires.
The descriptor survives the call in ebx because sendFTP clobbers the
registers. A failed truncate closes the descriptor and answers 550
through a new .truncate_error path.

The libio import list gains file_truncate, exported since 2009.

STOR: усекать файл при перезаписи, чтобы не оставался старый хвост

Проблема

cmdSTOR открывает целевой файл с O_CREATE + O_WRITE, а libio усекает файл при записи только если позиция уже находится в EOF (libio.asm:314). В этом сценарии условие никогда не выполняется. Поэтому при замене длинного файла коротким сохранялся старый хвост: 4096-байтный A, перезаписанный 1024-байтным B, давал 4096-байтный файл, содержащий B, а затем A[1024:4096]. Перезапись нулевым количеством байт вообще не изменяла файл.

Решение

Сразу после успешного открытия вызывается file.truncate (SF 70.4, size = Position = 0), чтобы STOR заменял файл, как требует RFC 959. Дескриптор сохраняется в ebx после вызова, так как sendFTP затирает регистры. Если truncate завершается ошибкой, дескриптор закрывается, а ответ 550 отправляется через новый обработчик .truncate_error.

Дополнительно

В список импортов libio добавлен file_truncate (экспортируется с 2009 года).

cmdSTOR opens the destination with O_CREATE + O_WRITE, and libio only truncates on write when the position is already at EOF (libio.asm:314), which never happens here. Replacing a longer file with a shorter one therefore kept the old tail: 4096-byte A overwritten by 1024-byte B produced a 4096-byte file holding B followed by A[1024:4096], and overwriting with zero bytes left the file completely untouched. Call file.truncate (SF 70.4, size = Position = 0) right after a successful open so that STOR replaces the file, as RFC 959 requires. The descriptor survives the call in ebx because sendFTP clobbers the registers. A failed truncate closes the descriptor and answers 550 through a new .truncate_error path. The libio import list gains file_truncate, exported since 2009. ------------------------- STOR: усекать файл при перезаписи, чтобы не оставался старый хвост Проблема cmdSTOR открывает целевой файл с O_CREATE + O_WRITE, а libio усекает файл при записи только если позиция уже находится в EOF (libio.asm:314). В этом сценарии условие никогда не выполняется. Поэтому при замене длинного файла коротким сохранялся старый хвост: 4096-байтный A, перезаписанный 1024-байтным B, давал 4096-байтный файл, содержащий B, а затем A[1024:4096]. Перезапись нулевым количеством байт вообще не изменяла файл. Решение Сразу после успешного открытия вызывается file.truncate (SF 70.4, size = Position = 0), чтобы STOR заменял файл, как требует RFC 959. Дескриптор сохраняется в ebx после вызова, так как sendFTP затирает регистры. Если truncate завершается ошибкой, дескриптор закрывается, а ответ 550 отправляется через новый обработчик .truncate_error. Дополнительно В список импортов libio добавлен file_truncate (экспортируется с 2009 года).

CODEOWNERS rules requested review from network 2026-09-18 18:16:03 +00:00

Leency commented

2026-09-19 08:21:20 +00:00

Что проверил. Сам фикс правильный: file.open ставит позицию в 0, и file.truncate усекает файл именно до позиции. Обычная запись усекает только тогда, когда позиция в конце файла, поэтому без этого вызова старый хвост оставался.

Что было не так. При ошибке truncate новый путь .truncate_error закрывал файл, но не закрывал data-соединение и не сбрасывал режим. Поэтому следующий STOR/RETR подхватывал старый сокет.

Что исправил (programs/network/ftpd/commands.inc):

.truncate_error:
        invoke  file.close, ebx
        mov     [ebp + thread_data.mode], MODE_NOTREADY
        mcall   close, [ebp + thread_data.datasocketnum]
        sendFTP "550 Cannot truncate file"
        ret

Так же убирает за собой abort_transfer. Текст ответа сменил на «Cannot truncate file», чтобы он называл настоящую причину.

**Что проверил.** Сам фикс правильный: `file.open` ставит позицию в 0, и `file.truncate` усекает файл именно до позиции. Обычная запись усекает только тогда, когда позиция в конце файла, поэтому без этого вызова старый хвост оставался. **Что было не так.** При ошибке truncate новый путь `.truncate_error` закрывал файл, но не закрывал data-соединение и не сбрасывал режим. Поэтому следующий STOR/RETR подхватывал старый сокет. **Что исправил** (`programs/network/ftpd/commands.inc`): ```asm .truncate_error: invoke file.close, ebx mov [ebp + thread_data.mode], MODE_NOTREADY mcall close, [ebp + thread_data.datasocketnum] sendFTP "550 Cannot truncate file" ret ``` Так же убирает за собой `abort_transfer`. Текст ответа сменил на «Cannot truncate file», чтобы он называл настоящую причину.

Leency approved these changes

2026-09-19 08:21:31 +00:00

igorsh commented

2026-09-19 15:35:27 +00:00

@Leency этот PR не решает проблемы с незакрытыми data-сокетами?
#707

@Leency этот PR не решает проблемы с незакрытыми data-сокетами? https://git.kolibrios.org/KolibriOS/kolibrios/pulls/707

Burer added 2 commits 2026-09-19 16:53:38 +00:00

ftpd: truncate the file on STOR instead of overwriting in place 313fc43ea8

cmdSTOR opens the destination with O_CREATE + O_WRITE, and libio only
truncates on write when the position is already at EOF (libio.asm:314),
which never happens here. Replacing a longer file with a shorter one
therefore kept the old tail: 4096-byte A overwritten by 1024-byte B
produced a 4096-byte file holding B followed by A[1024:4096], and
overwriting with zero bytes left the file completely untouched.

Call file.truncate (SF 70.4, size = Position = 0) right after a
successful open so that STOR replaces the file, as RFC 959 requires.
The descriptor survives the call in ebx because sendFTP clobbers the
registers. A failed truncate closes the descriptor and answers 550
through a new .truncate_error path.

The libio import list gains file_truncate, exported since 2009.

Assisted-by: ZCode:deepseek-flash

ftpd: close the data connection when STOR cannot truncate

Test PR / Build (en_US) (pull_request) Successful in 1m41s

Details

Test PR / Build (ru_RU) (pull_request) Successful in 1m50s

Details

Test PR / Build (es_ES) (pull_request) Successful in 1m53s

Details

f658a6d3ec

The .truncate_error path closed the file but left the data socket
open and the mode at MODE_PASSIVE_OK/MODE_ACTIVE, so the next
transfer reused a stale connection. Reset the mode and close the
socket as abort_transfer does, and report the actual failure.

Assisted-by: Claude Opus 5 <noreply@anthropic.com>

Burer force-pushed ftpd-truncate-the-file-on-STOR from e5b3aa5c88 to f658a6d3ec

2026-09-19 16:53:38 +00:00

Compare

CODEOWNERS rules requested review from network 2026-09-19 16:53:38 +00:00

Burer approved these changes

2026-09-19 16:56:08 +00:00

Burer merged commit 3caf2a7a72 into main

2026-09-19 16:56:32 +00:00

Burer deleted branch ftpd-truncate-the-file-on-STOR

2026-09-19 16:56:32 +00:00

Sign in to join this conversation.

No Reviewers

Leency

Burer

KolibriOS/network

3 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KolibriOS/kolibrios#706

Allow edits from maintainers