Remove unsafe from ReadToString

[goffrie]

We can do everything needed with only safe code.

[Darksonn]

According to the documentation of AsyncReadExt::read_to_string, the buffer should be left unchanged when the data is invalid utf-8, but this implementation empties the string in this case. To be fair, the previous implementation also emptied the string, but it'd still be nice to fix that.

You can test this behavior by creating tokio/tests/read_to_string.rs with these contents:

use tokio::io::AsyncReadExt;

#[tokio::test]
async fn to_string_does_not_truncate() {
    let data = vec![0xff, 0xff, 0xff];

    let mut s = "abc".to_string();

    match AsyncReadExt::read_to_string(&mut data.as_slice(), &mut s).await {
        Ok(len) => panic!("Should fail: {} bytes.", len),
        Err(err) if err.to_string() == "stream did not contain valid UTF-8" => {},
        Err(err) => panic!("Fail: {}.", err),
    }

    assert_eq!(s, "abc");
}

#[tokio::test]
async fn to_string_appends() {
    let data = b"def".to_vec();

    let mut s = "abc".to_string();

    let len = AsyncReadExt::read_to_string(&mut data.as_slice(), &mut s).await.unwrap();

    assert_eq!(len, 3);
    assert_eq!(s, "abcdef");
}

I've also added a test in the case when the provided buffer is non-empty, but the data is valid utf-8. This test does not currently fail, but we might as well add it now that we're adding tests.

The CI failure is due to the unused import.

[goffrie]

Thanks, good point. I've adjusted the implementation to try to put the buffer back on error. This doesn't work if the reader gets dropped, but fixing that doesn't really seem worth it.

[goffrie]

Not sure if this is kosher, but a new version of tokio-test with the read_error mock hasn't been released to crates.io yet.

[carllerche]

It should be ok to keep path dev-deps. What would be the reason to avoid them?

[goffrie]

I can't think of any reason, just wondering why it isn't already this way.

[kleimkuhler]

@goffrie You'll want to rebase this off master to fix the FreeBSD check but this looks good otherwise

[Darksonn]

I had another read-through, and everything looks good!