Skip to content

Commit

Permalink
Ensure 'TempFile' flushes when persisted.
Browse files Browse the repository at this point in the history
Tokio's `File::write_all()` method has an unexpected quirk: it doesn't
actually write all the requested content to the file when the returned
future resolves. Instead, the write is attempted and queued. This means
that the `persist()` method can resolve without the data being persisted
to the file system. Subsequent reads of the ostensibly written-to file
can thus fail to contain the expected data.

An call to `flush()` following `write_all()` would circumvent the issue.
Alternatively, calling `fs::write()` actually writes to the file system
before returning and requires fewer lines of code. This commit thus
swaps the call to `write_all()` with `fs::write()`.
  • Loading branch information
plietar authored and SergioBenitez committed Jan 18, 2024
1 parent 61e77c5 commit 52de9a1
Showing 1 changed file with 8 additions and 7 deletions.
15 changes: 8 additions & 7 deletions core/lib/src/fs/temp_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::fs::FileName;

use tokio::task;
use tokio::fs::{self, File};
use tokio::io::{AsyncWriteExt, AsyncBufRead, BufReader};
use tokio::io::{AsyncBufRead, BufReader};
use tempfile::{NamedTempFile, TempPath};
use either::Either;

Expand Down Expand Up @@ -189,8 +189,7 @@ impl<'v> TempFile<'v> {
}
}
TempFile::Buffered { content } => {
let mut file = File::create(&new_path).await?;
file.write_all(content).await?;
fs::write(&new_path, &content).await?;
*self = TempFile::File {
file_name: None,
content_type: None,
Expand Down Expand Up @@ -228,10 +227,12 @@ impl<'v> TempFile<'v> {
/// # let some_other_path = std::env::temp_dir().join("some-other.txt");
/// file.copy_to(&some_other_path).await?;
/// assert_eq!(file.path(), Some(&*some_path));
/// # assert_eq!(std::fs::read(some_path).unwrap(), b"hi");
/// # assert_eq!(std::fs::read(some_other_path).unwrap(), b"hi");
///
/// Ok(())
/// }
/// # let file = TempFile::Buffered { content: "hi".as_bytes() };
/// # let file = TempFile::Buffered { content: b"hi" };
/// # rocket::async_test(handle(file)).unwrap();
/// ```
pub async fn copy_to<P>(&mut self, path: P) -> io::Result<()>
Expand All @@ -257,8 +258,7 @@ impl<'v> TempFile<'v> {
}
TempFile::Buffered { content } => {
let path = path.as_ref();
let mut file = File::create(path).await?;
file.write_all(content).await?;
fs::write(&path, &content).await?;
*self = TempFile::File {
file_name: None,
content_type: None,
Expand Down Expand Up @@ -393,10 +393,11 @@ impl<'v> TempFile<'v> {
/// # let some_path = std::env::temp_dir().join("some-path.txt");
/// file.persist_to(&some_path).await?;
/// assert_eq!(file.path(), Some(&*some_path));
/// # assert_eq!(std::fs::read(some_path).unwrap(), b"hi");
///
/// Ok(())
/// }
/// # let file = TempFile::Buffered { content: "hi".as_bytes() };
/// # let file = TempFile::Buffered { content: b"hi" };
/// # rocket::async_test(handle(file)).unwrap();
/// ```
pub fn path(&self) -> Option<&Path> {
Expand Down

0 comments on commit 52de9a1

Please sign in to comment.