FTP connections preserve passwords and filenames, with CI checking every change #1

Merged
temeddix merged 15 commits from fix-directory-listing into main 2026-09-30 13:10:40 +00:00
3 changed files with 68 additions and 9 deletions
Showing only changes of commit e789a2ee6d - Show all commits
+26 -6
View File
2
@@ -259,17 +259,37 @@ struct FtpWriter {
written: Cell<u64>,
}
impl FtpWriter {
/// A failed data write can leave both the remote file and its pending
/// completion reply in an unknown state. Close both channels so the next
/// filesystem operation must start a fresh session.
fn discard_uncertain_upload(&self) -> ErrorCode {
drop(self.socket.take());
Outdated
Review

A data write can fail after the server accepted a prefix. Clearing the socket makes Drop skip its cleanup, so that partial remote file remains, violating the writer contract. Stage uploads under a temporary name and promote only after a confirmed completion, or otherwise ensure cleanup before exposing the target.

Profile: codex · gpt-5.6-terra

A data write can fail after the server accepted a prefix. Clearing the socket makes `Drop` skip its cleanup, so that partial remote file remains, violating the writer contract. Stage uploads under a temporary name and promote only after a confirmed completion, or otherwise ensure cleanup before exposing the target. > Profile: `codex` · `gpt-5.6-terra`
CONTROL.set(None);
ErrorCode::OutcomeUnknown
}
Outdated
Review

An upload data-write failure returns OutcomeUnknown but leaves CONTROL cached. The server may still be awaiting this transfer's completion, so a later operation can send NOOP or a mutation on a desynchronized session. Clear the control slot when this error (or a zero-byte write) occurs, and add a regression that proves the next operation reconnects rather than reusing it.

Profile: codex · gpt-5.6-terra

An upload data-write failure returns `OutcomeUnknown` but leaves `CONTROL` cached. The server may still be awaiting this transfer's completion, so a later operation can send `NOOP` or a mutation on a desynchronized session. Clear the control slot when this error (or a zero-byte write) occurs, and add a regression that proves the next operation reconnects rather than reusing it. > Profile: `codex` · `gpt-5.6-terra`
}
impl GuestWriter for FtpWriter {
fn push(&self, data: Vec<u8>) -> Result<(), ErrorCode> {
let socket = self.socket.borrow();
let socket = socket.as_ref().ok_or(ErrorCode::InvalidInput)?;
if self.socket.borrow().is_none() {
return Err(ErrorCode::InvalidInput);
}
let mut sent = 0;
while sent < data.len() {
let written = socket
.write(&data[sent..])
.map_err(|_| ErrorCode::OutcomeUnknown)? as usize;
let result = {
let socket = self.socket.borrow();
let Some(socket) = socket.as_ref() else {
return Err(ErrorCode::InvalidInput);
};
socket.write(&data[sent..])
};
let written = match result {
Ok(written) => written as usize,
Err(_) => return Err(self.discard_uncertain_upload()),
};
if written == 0 {
return Err(ErrorCode::OutcomeUnknown);
return Err(self.discard_uncertain_upload());
}
sent += written;
}
1
+29 -2
View File
@@ -1,8 +1,9 @@
//! FTP command recovery against an in-memory protocol transport. Live QA
//! separately exercises the compiled WASM component over real host sockets.
use super::{Ftp, mutation_transfer, settle_mutation, test_host};
use memona_plugin_sdk::{ErrorCode, Guest, Locator, Setting};
use super::{Ftp, FtpWriter, mutation_transfer, settle_mutation, test_host};
use memona_plugin_sdk::{ErrorCode, Guest, GuestWriter, Locator, Setting};
use std::cell::{Cell, RefCell};
type TestResult<T = ()> = Result<T, Box<dyn std::error::Error + Send + Sync>>;
@@ -91,3 +92,29 @@ async fn a_lost_upload_completion_reports_an_unknown_outcome() -> TestResult {
assert_eq!(summary.uploaded_bytes, 4);
Ok(())
}
#[tokio::test]
async fn an_uncertain_upload_write_discards_the_control_session() -> TestResult
{
let fixture = test_host::install(test_host::Scenario::LostUploadData);
Outdated
Review

This runs the guest on Tokio's blocking thread pool, contrary to the repository's event-driven/async-only rule; the same pattern is repeated in the other recovery test. Replace the native blocking socket test transport with an async one.

Profile: codex · gpt-5.6-terra

This runs the guest on Tokio's blocking thread pool, contrary to the repository's event-driven/async-only rule; the same pattern is repeated in the other recovery test. Replace the native blocking socket test transport with an async one. > Profile: `codex` · `gpt-5.6-terra`
guest(Ftp::open("ftp".into(), config()))?;
Outdated
Review

This thread-pool handoff violates the repository's event-driven-only/no-thread-model rule. Exercise the synchronous guest with a compliant transport, or add an explicit approved exception to the repository rules.

Profile: codex · gpt-5.6-terra

This thread-pool handoff violates the repository's event-driven-only/no-thread-model rule. Exercise the synchronous guest with a compliant transport, or add an explicit approved exception to the repository rules. > Profile: `codex` · `gpt-5.6-terra`
let writer = FtpWriter {
path: "/broken".into(),
socket: RefCell::new(Some(guest(mutation_transfer("STOR /broken"))?)),
written: Cell::new(0),
};
assert!(matches!(
writer.push(b"body".to_vec()),
Err(ErrorCode::OutcomeUnknown)
));
let page = guest(Ftp::list(Locator::Path("/".into()), None, 10))?;
assert_eq!(page.entries[0].name, " report.md ");
drop(writer);
Ftp::close();
let summary = fixture.summary();
assert_eq!(summary.control_connections, 2);
assert_eq!(summary.upload_commands, 1);
assert_eq!(summary.uploaded_bytes, 0);
assert_eq!(summary.listings, 1);
Ok(())
}
+13 -1
View File
@@ -15,6 +15,7 @@ const LISTING: &[u8] = b"type=file;size=3; report.md \r\n";
pub enum Scenario {
CancelledPreflight,
ExpiredListing,
LostUploadData,
LostMutationReply,
LostUploadCompletion,
}
@@ -156,7 +157,12 @@ impl Tcp {
"TYPE I" => "200 Binary\r\n",
"QUIT" => "221 Bye\r\n",
"EPSV" => "229 Passive (|||31310|)\r\n",
"MLSD /" if matches!(state.scenario, Scenario::ExpiredListing) => {
"MLSD /"
if matches!(
state.scenario,
Scenario::ExpiredListing | Scenario::LostUploadData
) =>
{
state.listings += 1;
"150 Listing\r\n226 Complete\r\n"
}
@@ -177,6 +183,12 @@ impl Tcp {
state.upload_commands += 1;
"150 Upload started\r\n"
}
"STOR /broken"
if matches!(state.scenario, Scenario::LostUploadData) =>
{
state.upload_commands += 1;
"150 Upload started\r\n"
}
"MKD /new"
if index == 1
&& matches!(state.scenario, Scenario::LostMutationReply) =>