fix: report failed copies and preserve text on save

CHANGELOG.md

@@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- Stream copies report failed reads and writes, and file writes check their
+  final flush. Saving a text file over its source reads the text before truncation.
+
 - Shared text helpers keep ASCII matching independent of locale and reject
   malformed UTF-8, empty replacement patterns and overflowing repetitions.
   In-memory byte writes reject offsets that overflow their bounds checks.

src/odr/file.cpp

@@ -346,8 +346,7 @@ void TextFile::save(const std::string &path) const {
   if (!is_savable()) {
     throw UnsupportedOperation();
   }
-  std::ofstream out = internal::util::file::create(path);
-  out << text();
+  internal::util::file::write(text(), path);
 }
 
 void TextFile::save(std::ostream &out) const {

src/odr/internal/common/temporary_file.cpp

@@ -1,5 +1,7 @@
 #include <odr/internal/common/temporary_file.hpp>
 
+#include <odr/exceptions.hpp>
+
 #include <odr/internal/common/random.hpp>
 #include <odr/internal/util/stream_util.hpp>
 
@@ -87,14 +89,26 @@ TemporaryDiskFile TemporaryDiskFileFactory::copy(std::istream &in) const {
     if (!file.is_open()) {
       file.clear();
       file.open(file_path.string(), std::ios_base::out | std::ios_base::binary);
+      if (!file.is_open()) {
+        throw FileWriteError(file_path.string());
+      }
       break;
     }
 
     file.close();
   }
 
-  util::stream::pipe(in, file);
-  file.close();
+  try {
+    util::stream::pipe(in, file);
+    file.close();
+    if (!file) {
+      throw FileWriteError(file_path.string());
+    }
+  } catch (...) {
+    file.close();
+    remove_quietly(file_path);
+    throw;
+  }
 
   return TemporaryDiskFile(file_path);
 }

src/odr/internal/encoding/detect.cpp

@@ -1,6 +1,7 @@
 #include <odr/internal/encoding/detect.hpp>
 
 #include <odr/internal/encoding/text_encoding_table.hpp>
+#include <odr/internal/util/stream_util.hpp>
 
 #include <istream>
 #include <string_view>
@@ -49,10 +50,7 @@ bool encoding_holds_nul(const TextEncoding encoding) {
 
 std::string encoding::read_probe(std::istream &in,
                                  const std::size_t max_bytes) {
-  std::string result(max_bytes, '\0');
-  in.read(result.data(), static_cast<std::streamsize>(max_bytes));
-  result.resize(static_cast<std::size_t>(in.gcount()));
-  return result;
+  return util::stream::read(in, max_bytes);
 }
 
 TextEncoding encoding::detect(const std::string_view probe) {

src/odr/internal/magic.cpp

@@ -7,6 +7,7 @@
 #include <odr/internal/abstract/file.hpp>
 #include <odr/internal/common/file.hpp>
 #include <odr/internal/open_strategy.hpp>
+#include <odr/internal/util/stream_util.hpp>
 #include <odr/internal/util/string_util.hpp>
 
 #include <istream>
@@ -18,14 +19,6 @@ namespace odr::internal {
 
 namespace {
 
-/// At most @p size bytes, cut back to what was actually read.
-std::string read_head(std::istream &in, const std::size_t size) {
-  std::string result(size, '\0');
-  in.read(result.data(), static_cast<std::streamsize>(size));
-  result.resize(static_cast<std::size_t>(in.gcount()));
-  return result;
-}
-
 bool match_magic(const std::string &head, const std::string &pattern) {
   const auto bytes = util::string::split(pattern, " ");
   if (bytes.size() > head.size()) {
@@ -201,7 +194,7 @@ FileType magic::file_type(std::istream &in) {
   // an enhanced metafile names itself at offset 40, so the head has to reach 44
   static constexpr std::size_t head_size = 64;
 
-  return file_type(read_head(in, head_size));
+  return file_type(util::stream::read(in, head_size));
 }
 
 FileType magic::file_type(const abstract::File &file) {

src/odr/internal/util/file_util.cpp

@@ -21,21 +21,17 @@ std::ifstream file::open(const std::string &path) {
 std::size_t file::size(const std::string &path) {
   std::ifstream in = open(path);
   in.seekg(0, std::ios::end);
-  return static_cast<std::size_t>(in.tellg());
+  const auto end = in.tellg();
+  if (end < 0) {
+    throw FileReadError();
+  }
+  return static_cast<std::size_t>(end);
 }
 
 std::string file::read(const std::string &path) {
   std::ifstream in = open(path);
 
-  std::string result;
-
-  in.seekg(0, std::ios::end);
-  result.reserve(in.tellg());
-  in.seekg(0, std::ios::beg);
-
-  result.assign(std::istreambuf_iterator(in), std::istreambuf_iterator<char>());
-
-  return result;
+  return stream::read(in);
 }
 
 void file::pipe(const std::string &path, std::ostream &out) {
@@ -54,11 +50,19 @@ std::ofstream file::create(const std::string &path) {
 void file::write(const std::string &data, const std::string &path) {
   std::ofstream out = create(path);
   out << data;
+  out.close();
+  if (!out) {
+    throw FileWriteError(path);
+  }
 }
 
 void file::write(std::istream &in, const std::string &path) {
   std::ofstream out = create(path);
   stream::pipe(in, out);
+  out.close();
+  if (!out) {
+    throw FileWriteError(path);
+  }
 }
 
 } // namespace odr::internal::util

src/odr/internal/util/stream_util.cpp

@@ -1,7 +1,7 @@
 #include <odr/internal/util/stream_util.hpp>
 
 #include <array>
-#include <iterator>
+#include <limits>
 #include <sstream>
 #include <streambuf>
 
@@ -12,12 +12,21 @@ using int_type = std::streambuf::int_type;
 static constexpr int_type eof = std::streambuf::traits_type::eof();
 
 std::string stream::read(std::istream &in) {
-  return {std::istreambuf_iterator(in), {}};
+  std::ostringstream out;
+  pipe(in, out);
+  return std::move(out).str();
 }
 
 std::string stream::read(std::istream &in, const std::size_t size) {
+  if (size >
+      static_cast<std::size_t>(std::numeric_limits<std::streamsize>::max())) {
+    throw std::length_error("stream read is too large");
+  }
   std::string result(size, '\0');
   in.read(result.data(), static_cast<std::streamsize>(size));
+  if (in.bad() || (in.fail() && !in.eof())) {
+    throw std::ios_base::failure("stream read failed");
+  }
   result.resize(static_cast<std::size_t>(in.gcount()));
   return result;
 }
@@ -29,11 +38,17 @@ void stream::pipe(std::istream &in, std::ostream &out) {
 
   while (true) {
     in.read(buffer.data(), BUFFER_SIZE);
+    if (in.bad() || (in.fail() && !in.eof())) {
+      throw std::ios_base::failure("stream read failed");
+    }
     const auto read = in.gcount();
     if (read == 0) {
       break;
     }
     out.write(buffer.data(), read);
+    if (!out) {
+      throw std::ios_base::failure("stream write failed");
+    }
   }
 }
 

test/src/file_test.cpp

@@ -6,6 +6,7 @@
 #include <odr/odr.hpp>
 
 #include <odr/internal/common/file.hpp>
+#include <odr/internal/common/random.hpp>
 #include <odr/internal/common/temporary_file.hpp>
 #include <odr/internal/util/file_util.hpp>
 #include <odr/internal/util/stream_util.hpp>
@@ -155,6 +156,29 @@ TEST(File, temporary_copies_preserve_binary_bytes) {
   EXPECT_EQ(internal::MemoryFile(copy).content(), data);
 }
 
+TEST(File, text_can_be_saved_over_its_disk_source) {
+  std::istringstream source("original text\n");
+  const auto temporary =
+      internal::TemporaryDiskFileFactory::system_default().copy(source);
+  const std::string path = temporary.disk_path()->string();
+  const TextFile text =
+      open(path, DecodeOptions::as(FileType::text_file)).as_text_file();
+  text.save(path);
+  EXPECT_EQ(internal::util::file::read(path), "original text\n");
+}
+
+TEST(File, failed_temporary_copies_leave_no_file) {
+  const internal::AbsPath directory(std::filesystem::temp_directory_path());
+  const std::string name = "odr-failed-copy-" + internal::random_string(12);
+  const internal::TemporaryDiskFileFactory factory(directory,
+                                                   [name] { return name; });
+  std::istringstream source("bytes");
+  source.setstate(std::ios::badbit);
+  EXPECT_THROW(std::ignore = factory.copy(source), std::ios_base::failure);
+  EXPECT_FALSE(
+      std::filesystem::exists(directory.join(internal::RelPath(name)).path()));
+}
+
 /// A file inside an archive is named by its entry, not by the archive.
 TEST(File, an_archive_entry_is_named_by_its_entry) {
   internal::zip::ZipArchive zip;

test/src/internal/util/stream_util_test.cpp

@@ -86,6 +86,28 @@ TEST(Stream, delimiter_reads_preserve_failed_input) {
   EXPECT_EQ(stream::read_until(in, ';', true), "second;");
 }
 
+TEST(Stream, copying_reports_input_and_output_failures) {
+  std::istringstream in(std::string(4097, 'x'));
+  std::ostringstream out;
+  stream::pipe(in, out);
+  EXPECT_EQ(out.str(), std::string(4097, 'x'));
+
+  in.clear();
+  in.seekg(0);
+  out.setstate(std::ios::badbit);
+  EXPECT_THROW(stream::pipe(in, out), std::ios_base::failure);
+
+  in.setstate(std::ios::badbit);
+  EXPECT_THROW(stream::read(in), std::ios_base::failure);
+  EXPECT_THROW(stream::read(in, 1), std::ios_base::failure);
+
+  class FailingBuffer final : public std::streambuf {
+    int_type underflow() override { throw std::runtime_error("read failure"); }
+  } buffer;
+  std::istream broken(&buffer);
+  EXPECT_THROW(stream::read(broken), std::ios_base::failure);
+}
+
 TEST(ByteStream, length_prefixed_reads_handle_chunks_and_stream_exceptions) {
   const std::string data(4097, 'x');
   std::istringstream in(data);