veeso / suppaftp

a super FTP/FTPS client library for Rust with support for both passive and active mode
Apache License 2.0
121 stars 31 forks source link

[BUG] - put_file does not work with Async SSL Native TLS #77

Closed mousetail closed 6 months ago

mousetail commented 7 months ago

Description

If I upload a file using async mode and native TLS, I get the following error:

Err(UnexpectedResponse(Response { status: TransferAborted, body: [52, 50, 54, 32, 70, 97, 105, 108, 117, 114, 101, 32, 114, 101, 97, 100, 105, 110, 103, 32, 110, 101, 116, 119, 111, 114, 107, 32, 115, 116, 114, 101, 97, 109, 46, 13, 10] }))

which translates as:

>>> ''.join(map(chr,[52, 50, 54, 32, 70, 97, 105, 108, 117, 114, 101, 32, 114, 101, 97, 100, 105, 110, 103, 32, 110, 101, 116, 119, 111, 114, 107, 32, 115, 116, 114, 101, 97, 109, 46, 13, 10]))
'426 Failure reading network stream.\r\n'

If I look at the logs on the server, I see:

Sun Mar 24 09:57:00 2024 [pid 27668] [xxx] FTP command: Client "censored", "PASV"
Sun Mar 24 09:57:00 2024 [pid 27668] [xxx] FTP response: Client "censored "227 Entering Passive Mode (213,32,70,64,158,39)."
Sun Mar 24 09:57:00 2024 [pid 27668] [xxx] FTP command: Client "censored", "STOR Welcome.md"
Sun Mar 24 09:57:00 2024 [pid 27668] [xxx] FTP response: Client "censored", "150 Ok to send data."
Sun Mar 24 09:57:00 2024 [pid 27667] [xxx] DEBUG: Client "censored", "DATA connection terminated without SSL shutdown. Buggy client! Integrity of upload cannot be asserted."

Seems the SSL connection isn't properly terminated. Need an async drop :)

If I disable SSL or switch to the sync version, it works fine.

Steps to reproduce

let mut ftp_stream = AsyncNativeTlsFtpStream::connect(&config.remote_origin).await.unwrap_or_else(|err| {
        panic!("{err}");
    });
let mut ftp_stream = ftp_stream
        .into_secure(
            ctx,
            &config.remote_domain,
        ).await
        .unwrap();
    ftp_stream
        .login(
            &config.remote_user,
            &config.remote_password,
        ).await
        .unwrap();

let mut file = tokio::fs::OpenOptions::new()
            .read(true)
            .open(local_filename.clone())
            .await
            .unwrap().compat();
        let bytes_written = ftp_stream.put_file(parts[parts.len() - 1], &mut file).await;
println!("wrote {bytes_written:?} bytes");

Expected behaviour

The file to be uploaded normally.

Environment

Additional information

Might need a better shutdown for the data stream than drop. Drop probably can't work because it can't be async and shutting down the SSL connection requires an async frame.

MrToph commented 6 months ago

I'm getting the same error using the sync version (with rustls):

Invalid response: [426] 426 Failure reading network stream.

The uploaded file misses some bytes at the end.

mousetail commented 6 months ago

I'm getting the same error using the sync version (with rustls):

Invalid response: [426] 426 Failure reading network stream.

The uploaded file misses some bytes at the end

Can you check if adding .close() to the sync_ftp like I did with the async version fixes it?

MrToph commented 6 months ago

I'm not sure what the .close() equivalent is for the sync_ftp. I tried calling flush and shutdown on the underlying TcpStream but it didn't fix the issue.

let mut stream = ftp_stream
    .put_with_stream(file_name)?;
copy(bytes, &mut stream)
stream.flush()?;
stream
    .get_ref()
    .shutdown(std::net::Shutdown::Both)?;
ftp_stream
    .finalize_put_stream(stream)?;