Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rustls seems to have unexpected behavior when used in tokio::io::copy #66

Closed
3andero opened this issue Jul 11, 2021 · 2 comments
Closed

Comments

@3andero
Copy link

3andero commented Jul 11, 2021

Hi, I'm building a proxy. The server side accepts a TlsStream (from the client side) and makes a Tcp connection to the target website. Then we just need to copy everything we get from the TlsStream to the TcpStream and collect the response vice versa.
I was initially using tokio::io::copy for this task. However, I encountered a weird bug that my client never received nearly half of the response from the target website, and the connection just stuck until timeout. I then replace the io::copy with my simplified version:

// (didn't work, same as tokio::io::copy)
pub async fn copy_tcp<R: AsyncRead + Unpin, W: AsyncWrite + Unpin>(
    r: &mut R,
    w: &mut W,
) -> Result<()> {
    let mut buf = [0; 2048];
    loop {
        let len = r.read(&mut buf).await?;
        if len == 0 {
            return Ok(());
        }
        w.write(&buf[..len]).await?;
    }
}

And this didn't work until I added a flush after the write call,

pub async fn copy_tcp<R: AsyncRead + Unpin, W: AsyncWrite + Unpin>(
    r: &mut R,
    w: &mut W,
) -> Result<()> {
    let mut buf = [0; 2048];
    loop {
        let len = r.read(&mut buf).await?;
        if len == 0 {
            return Ok(());
        }
        w.write(&buf[..len]).await?;
    --> w.flush().await?;
    }
}

I found that in tokio-tls/rustls,

/// Note: that it does not guarantee the final data to be sent.
/// To be cautious, you must manually call `flush`.

that we are required to manually call flush.
While in tokio::io::copy,
https://github.com/tokio-rs/tokio/blob/c306bf853a1f8423b154f17fa47926f04eecd9b4/tokio/src/io/util/copy.rs#L69-L74
only when EOF is seen does it call poll_flush.

So, should we avoid using tokio::io::copy for this purpose?

@quininer
Copy link
Member

You're right. I think tokio::io::copy can be simply modified to deal with this problem. we only need to call poll_flush when poll_read is pending.

cc @Darksonn

@quininer
Copy link
Member

This problem has been fixed in tokio. tokio-rs/tokio#4001

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants