diff --git a/code/cfile/cfile.cpp b/code/cfile/cfile.cpp index 12f6df421a9..f4868f104fa 100644 --- a/code/cfile/cfile.cpp +++ b/code/cfile/cfile.cpp @@ -33,7 +33,7 @@ #include "parse/encrypt.h" #include "cfilesystem.h" - +#include #include char Cfile_root_dir[CFILE_ROOT_DIRECTORY_LEN] = ""; @@ -1105,28 +1105,98 @@ void cfread_string(char *buf, int n, CFILE *file) } while (c != 0 ); } -void cfread_string_len(char *buf, int buffer_size, CFILE *file) +namespace { +struct string_len_header { + int len = 0; + int stored = 0; + int start = 0; + bool length_cut = false; +}; + +// a past-the-end length is clamped like any cfread, but a negative one has nothing to clamp to, so inside a +// limited section it throws and the loader skips the rest of the section +string_len_header read_string_len_header(CFILE* file) { - int len; - len = cfread_int(file); - Assertion( (len <= buffer_size), "String cannot fit in buffer, len: %i, buffer: %i", len, buffer_size); - if (len) { - cfread(buf, len, 1, file); - Assertion(len != buffer_size || buf[len - 1] == 0, "Unterminated string does not fit in buffer"); + string_len_header h; + if (!cf_is_valid(file)) { + return h; + } + int before = cftell(file); + h.length_cut = cfread(&h.len, sizeof(h.len), 1, file) != 1; + h.start = cftell(file); + if (h.length_cut) { + // a clean end of file reads as an empty string without a warning, as cfread_int does + h.length_cut = (h.start != before) || (cfeof(file) == 0); + return h; + } + h.len = INTEL_INT(h.len); + if ((h.len < 0) && (file->max_read_len != 0)) { + throw cfile::max_read_length("Attempted to read a string of negative length " + std::to_string(h.len)); + } + h.stored = static_cast(cf_check_read_size(file, static_cast(std::max(h.len, 0)))); + return h; +} + +// reads count of the stored characters and leaves the file just past the whole string; false if the read failed +bool read_string_len_body(const string_len_header& h, char* dest, int count, CFILE* file) +{ + bool ok = (count <= 0) || (cfread(dest, count, 1, file) == 1); + if (!ok || (h.stored > count)) { + cfseek(file, h.start + h.stored, CF_SEEK_SET); } + return ok; +} + +// warned last, so the file position is right even if a debug build stops here +void warn_string_len(const string_len_header& h, bool read_ok, bool cut, int buffer_size, CFILE* file) +{ + if (h.length_cut) { + Warning(LOCATION, "cfread_string_len: cut-off string length field in %s", cf_get_filename(file)); + } else if (!read_ok) { + Warning(LOCATION, + "cfread_string_len: could not read a string of %d characters in %s", + h.stored, + cf_get_filename(file)); + } else if (h.len != h.stored) { + Warning(LOCATION, "cfread_string_len: corrupt string length %d in %s", h.len, cf_get_filename(file)); + } else if (cut) { + Warning(LOCATION, + "cfread_string_len: a string of %d bytes was cut to fit a buffer of %d in %s", + h.len, + buffer_size, + cf_get_filename(file)); + } +} +} // namespace - buf[MIN(len, buffer_size - 1)] = 0; +void cfread_string_len(char* buf, int buffer_size, CFILE* file) +{ + Assertion(buffer_size > 0, "cfread_string_len needs room for at least the terminator, buffer: %i", buffer_size); + string_len_header h = read_string_len_header(file); + if (buffer_size <= 0) { + bool read_ok = read_string_len_body(h, buf, 0, file); + warn_string_len(h, read_ok, h.stored > 0, buffer_size, file); + return; + } + int count = std::min(h.stored, buffer_size); + bool read_ok = read_string_len_body(h, buf, count, file); + int kept = read_ok ? count : 0; + bool cut = (kept == buffer_size) && (memchr(buf, 0, buffer_size) == nullptr); + buf[std::min(kept, buffer_size - 1)] = 0; + warn_string_len(h, read_ok, cut, buffer_size, file); } SCP_string cfread_string_len(CFILE *file) { - int len = cfread_int(file); + string_len_header h = read_string_len_header(file); SCP_string str; - str.resize(len); - - if (len) - cfread(&str[0], len, 1, file); + str.resize(h.stored); + bool read_ok = read_string_len_body(h, str.data(), h.stored, file); + if (!read_ok) { + str.clear(); + } + warn_string_len(h, read_ok, false, 0, file); return str; } diff --git a/code/cfile/cfile.h b/code/cfile/cfile.h index 7c720ac2371..f5e99d8960b 100644 --- a/code/cfile/cfile.h +++ b/code/cfile/cfile.h @@ -320,20 +320,26 @@ void cfread_string(char *buf,int n, CFILE *file); /** * @brief Read a fixed length string that is not null-terminated, with the length stored in file. * - * If it is not null-terminated, the length of the string *must* be less than the size of the buffer (to make room for one) - * + * Keeps at most buffer_size - 1 characters and leaves the file just past the whole string, unless it throws. + * * @param buf Pre-allocated array to store string * @param buffer_size Size of pre-allocated array * @param file File to read from * * @note Appends NULL character to string (buf) + * @note Bad data gets a warning; at a clean end of file, or with an invalid file, the string is empty and nothing is + * warned. Inside a limited section (cf_set_max_read_len), a negative length, or a read that still passes the limit + * once clamped to the end of the file, throws cfile::max_read_length. */ void cfread_string_len(char *buf,int buffer_size, CFILE *file); /** * @brief Read a string from the file where the length is stored in the file + * * @param file The file to read the string from * @return The string that was read + * + * @note Warns and throws like the char* overload, except that nothing is cut. */ SCP_string cfread_string_len(CFILE *file); diff --git a/code/cfile/cfilearchive.cpp b/code/cfile/cfilearchive.cpp index eb34205b59b..409814b9c58 100644 --- a/code/cfile/cfilearchive.cpp +++ b/code/cfile/cfilearchive.cpp @@ -193,21 +193,8 @@ int cfseek( CFILE *cfile, int offset, int where ) } -// cfread() reads from a file -// -// returns: returns the number of full elements read -// -// -int cfread(void *buf, int elsize, int nelem, CFILE *cfile) +size_t cf_check_read_size(CFILE* cfile, size_t size) { - if(!cf_is_valid(cfile)) - return 0; - - size_t size = elsize*nelem; - - if(buf == NULL || size <= 0) - return 0; - if ( (cfile->raw_position+size) > cfile->size ) { Assertion(cfile->raw_position <= cfile->size, "Invalid raw_position value detected!"); size = cfile->size - cfile->raw_position; @@ -226,6 +213,29 @@ int cfread(void *buf, int elsize, int nelem, CFILE *cfile) } } + return size; +} + +// cfread() reads from a file +// +// returns: returns the number of full elements read +// +// +int cfread(void *buf, int elsize, int nelem, CFILE *cfile) +{ + if(!cf_is_valid(cfile)) + return 0; + + size_t size = elsize*nelem; + + if(buf == nullptr || size <= 0) + return 0; + + size = cf_check_read_size(cfile, size); + if (size < 1) { + return 0; + } + size_t bytes_read; if (cfile->data != nullptr) { // This is a file from memory diff --git a/code/cfile/cfilearchive.h b/code/cfile/cfilearchive.h index 275a95282f5..b32d9654a5b 100644 --- a/code/cfile/cfilearchive.h +++ b/code/cfile/cfilearchive.h @@ -62,4 +62,9 @@ void cf_check_compression(CFILE* cfile); // Used to clear compression info data and free dynamic memory used by compression void cf_clear_compression_info(CFILE* cfile); +// Clamps a read of size bytes to the end of the file and returns the clamped size, or 0 at the end of the file +// without checking the limit; throws cfile::max_read_length if the clamped read would pass the file's read limit +// (cf_set_max_read_len). +size_t cf_check_read_size(CFILE* cfile, size_t size); + #endif diff --git a/test/src/cfile/cfile.cpp b/test/src/cfile/cfile.cpp index f9842a55081..1a7f390e0da 100644 --- a/test/src/cfile/cfile.cpp +++ b/test/src/cfile/cfile.cpp @@ -1,7 +1,11 @@ +#include #include +#include #include #include +#include +#include #include "util/FSTestFixture.h" @@ -212,3 +216,693 @@ TEST_F(CFileTest, subfolder_list) ASSERT_EQ(2, cf_get_file_list(table_files, CF_TYPE_TABLES, "*\\*.tbl", CF_SORT_NAME)); ASSERT_TRUE(table_files.back().substr(0, 6) == "folder"); } + +namespace { +// Builds the bytes cfwrite_string_len writes (a little-endian length, then the characters) as an in-memory file. +// The file reads from this object's buffer, so the object has to outlive the CFILE. +class StringLenFile { + public: + StringLenFile& add_int(int value) + { + for (int i = 0; i < 4; ++i) { + _bytes.push_back(static_cast((value >> (8 * i)) & 0xFF)); + } + return *this; + } + + StringLenFile& add_chars(const char* chars, size_t count) + { + _bytes.insert(_bytes.end(), chars, chars + count); + return *this; + } + + CFILE* open() const + { + CFileLocation res(true); + res.name_ext = "string_len.bin"; + res.full_name = "string_len.bin"; + res.data_ptr = _bytes.data(); + res.size = _bytes.size(); + return cfopen_special(res, "rb"); + } + + private: + SCP_vector _bytes; +}; + +// Debug test builds turn Warning() into an exception, Release builds only count it +template +void expect_warning(const char* text, Read read) +{ +#ifndef NDEBUG + try { + read(); + ADD_FAILURE() << "expected a warning containing '" << text << "'"; + } catch (const os::dialogs::WarningException& e) { + EXPECT_NE(SCP_string(e.what()).find(text), SCP_string::npos) << e.what(); + } +#else + int warnings = Global_warning_count; + read(); + EXPECT_EQ(Global_warning_count, warnings + 1) << "expected one warning containing '" << text << "'"; +#endif +} + +template +void expect_no_warning(Read read) +{ + int warnings = Global_warning_count; + EXPECT_NO_THROW(read()); + EXPECT_EQ(Global_warning_count, warnings); +} +} // namespace + +TEST_F(CFileTest, string_len_fits) +{ + StringLenFile file; + file.add_int(3).add_chars("abc", 3).add_int(42); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + char buf[8]; + memset(buf, 'X', sizeof(buf)); + expect_no_warning([&] { cfread_string_len(buf, sizeof(buf), fp); }); + EXPECT_STREQ(buf, "abc"); + EXPECT_EQ(cfread_int(fp), 42); + + cfclose(fp); +} + +TEST_F(CFileTest, string_len_one_short_of_the_buffer) +{ + StringLenFile file; + file.add_int(3).add_chars("abc", 3).add_int(42); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + char buf[4]; + memset(buf, 'X', sizeof(buf)); + expect_no_warning([&] { cfread_string_len(buf, sizeof(buf), fp); }); + EXPECT_STREQ(buf, "abc"); + EXPECT_EQ(cfread_int(fp), 42); + + cfclose(fp); +} + +TEST_F(CFileTest, string_len_empty_inside_max_read_len) +{ + StringLenFile file; + file.add_int(0).add_int(42); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + cf_set_max_read_len(fp, 8); + + char buf[8]; + memset(buf, 'X', sizeof(buf)); + expect_no_warning([&] { cfread_string_len(buf, sizeof(buf), fp); }); + EXPECT_STREQ(buf, ""); + EXPECT_EQ(cfread_int(fp), 42); + cfclose(fp); + + fp = file.open(); + ASSERT_NE(fp, nullptr); + cf_set_max_read_len(fp, 4); + SCP_string str = "old"; + expect_no_warning([&] { str = cfread_string_len(fp); }); + EXPECT_EQ(str, ""); + EXPECT_EQ(cftell(fp), 4); + cfclose(fp); +} + +TEST_F(CFileTest, string_len_clean_end_of_file_is_silent) +{ + StringLenFile file; + file.add_int(42); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + ASSERT_EQ(cfread_int(fp), 42); + + char buf[8]; + memset(buf, 'X', sizeof(buf)); + expect_no_warning([&] { cfread_string_len(buf, sizeof(buf), fp); }); + EXPECT_STREQ(buf, ""); + EXPECT_EQ(cftell(fp), 4); + + SCP_string str = "old"; + expect_no_warning([&] { str = cfread_string_len(fp); }); + EXPECT_EQ(str, ""); + EXPECT_EQ(cftell(fp), 4); + + cfclose(fp); +} + +TEST_F(CFileTest, string_len_without_a_file_reads_empty) +{ + char buf[8]; + memset(buf, 'X', sizeof(buf)); + expect_no_warning([&] { cfread_string_len(buf, sizeof(buf), nullptr); }); + EXPECT_STREQ(buf, ""); + + SCP_string str = "old"; + expect_no_warning([&] { str = cfread_string_len(nullptr); }); + EXPECT_EQ(str, ""); +} + +TEST_F(CFileTest, string_len_exact_fit_with_stored_terminator) +{ + StringLenFile file; + file.add_int(4).add_chars("abc\0", 4).add_int(42); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + char buf[4]; + expect_no_warning([&] { cfread_string_len(buf, sizeof(buf), fp); }); + EXPECT_STREQ(buf, "abc"); + EXPECT_EQ(cfread_int(fp), 42); + + cfclose(fp); +} + +TEST_F(CFileTest, string_len_longer_than_buffer) +{ + StringLenFile file; + file.add_int(10).add_chars("0123456789", 10).add_int(42); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + char buf[8]; + memset(buf, 'X', sizeof(buf)); + expect_warning("was cut", [&] { cfread_string_len(buf, 4, fp); }); + EXPECT_STREQ(buf, "012"); + for (int i = 4; i < 8; ++i) { + EXPECT_EQ(buf[i], 'X') << "wrote past the buffer at " << i; + } + EXPECT_EQ(cfread_int(fp), 42); + + cfclose(fp); +} + +TEST_F(CFileTest, string_len_zero_and_negative) +{ + StringLenFile file; + file.add_int(0).add_int(-5).add_int(42); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + char buf[8] = "old"; + expect_no_warning([&] { cfread_string_len(buf, sizeof(buf), fp); }); + EXPECT_STREQ(buf, ""); + + strcpy_s(buf, "old"); + expect_warning("corrupt string length -5", [&] { cfread_string_len(buf, sizeof(buf), fp); }); + EXPECT_STREQ(buf, ""); + EXPECT_EQ(cfread_int(fp), 42); + + cfclose(fp); +} + +TEST_F(CFileTest, string_len_scp_string) +{ + StringLenFile file; + file.add_int(3).add_chars("abc", 3).add_int(-5).add_int(42).add_int(1000000).add_chars("de", 2); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + SCP_string fits; + expect_no_warning([&] { fits = cfread_string_len(fp); }); + EXPECT_EQ(fits, "abc"); + // Debug test builds throw at the warning, before the string is returned, so only the position is checked there + SCP_string negative = "old"; + expect_warning("corrupt string length -5", [&] { negative = cfread_string_len(fp); }); + EXPECT_EQ(cfread_int(fp), 42); + SCP_string past_end; + expect_warning("corrupt string length 1000000", [&] { past_end = cfread_string_len(fp); }); + EXPECT_EQ(cftell(fp), cfilelength(fp)); +#ifdef NDEBUG + EXPECT_EQ(negative, ""); + EXPECT_EQ(past_end, "de"); +#endif + + cfclose(fp); +} + +TEST_F(CFileTest, string_len_exact_fit_without_terminator) +{ + StringLenFile file; + file.add_int(4).add_chars("abcd", 4).add_int(42); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + char buf[4]; + expect_warning("was cut", [&] { cfread_string_len(buf, sizeof(buf), fp); }); + EXPECT_STREQ(buf, "abc"); + EXPECT_EQ(cfread_int(fp), 42); + + cfclose(fp); +} + +TEST_F(CFileTest, string_len_past_end_of_file) +{ + StringLenFile file; + file.add_int(20).add_chars("hello", 5); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + char buf[32]; + memset(buf, 'X', sizeof(buf)); + expect_warning("corrupt string length 20", [&] { cfread_string_len(buf, sizeof(buf), fp); }); + EXPECT_STREQ(buf, "hello"); + EXPECT_EQ(cftell(fp), cfilelength(fp)); + + cfclose(fp); +} + +TEST_F(CFileTest, string_len_respects_max_read_len) +{ + StringLenFile file; + file.add_int(12).add_chars("0123456789ab", 12).add_int(42); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + cf_set_max_read_len(fp, 10); + char buf[4]; + EXPECT_THROW(cfread_string_len(buf, sizeof(buf), fp), cfile::max_read_length); + + cfclose(fp); +} + +TEST_F(CFileTest, string_len_inside_max_read_len) +{ + StringLenFile file; + file.add_int(6).add_chars("abcdef", 6).add_int(42); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + cf_set_max_read_len(fp, 10); + + char buf[8]; + expect_no_warning([&] { cfread_string_len(buf, sizeof(buf), fp); }); + EXPECT_STREQ(buf, "abcdef"); + EXPECT_EQ(cftell(fp), 10); + EXPECT_THROW(cfread_int(fp), cfile::max_read_length); + + cfclose(fp); +} + +TEST_F(CFileTest, string_len_cut_at_max_read_len) +{ + StringLenFile file; + file.add_int(6).add_chars("abcdef", 6).add_int(42); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + cf_set_max_read_len(fp, 10); + + char buf[4]; + expect_warning("was cut", [&] { cfread_string_len(buf, sizeof(buf), fp); }); + EXPECT_STREQ(buf, "abc"); + EXPECT_EQ(cftell(fp), 10); + + cfclose(fp); +} + +TEST_F(CFileTest, string_len_negative_inside_max_read_len) +{ + StringLenFile file; + file.add_int(-5).add_chars("abcdefgh", 8).add_int(42); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + cf_set_max_read_len(fp, 8); + + char buf[8]; + EXPECT_THROW(cfread_string_len(buf, sizeof(buf), fp), cfile::max_read_length); + cfclose(fp); + + auto fp2 = file.open(); + ASSERT_NE(fp2, nullptr); + cf_set_max_read_len(fp2, 8); + EXPECT_THROW(cfread_string_len(fp2), cfile::max_read_length); + cfclose(fp2); +} + +TEST_F(CFileTest, string_len_scp_string_respects_max_read_len) +{ + StringLenFile file; + file.add_int(12).add_chars("0123456789ab", 12).add_int(42); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + cf_set_max_read_len(fp, 10); + EXPECT_THROW(cfread_string_len(fp), cfile::max_read_length); + + cfclose(fp); +} + +TEST_F(CFileTest, string_len_nul_inside_buffer_is_not_cut) +{ + StringLenFile file; + file.add_int(10).add_chars("ab\0defghij", 10).add_int(42); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + char buf[8]; + memset(buf, 'X', sizeof(buf)); + expect_no_warning([&] { cfread_string_len(buf, 4, fp); }); + EXPECT_STREQ(buf, "ab"); + EXPECT_EQ(buf[3], 0); + for (int i = 4; i < 8; ++i) { + EXPECT_EQ(buf[i], 'X') << "wrote past the buffer at " << i; + } + EXPECT_EQ(cfread_int(fp), 42); + + cfclose(fp); +} + +TEST_F(CFileTest, string_len_corrupt_and_cut_warns_once) +{ + StringLenFile file; + file.add_int(20).add_chars("hello", 5); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + char buf[4]; + memset(buf, 'X', sizeof(buf)); + expect_warning("corrupt string length 20", [&] { cfread_string_len(buf, sizeof(buf), fp); }); + EXPECT_STREQ(buf, "hel"); + EXPECT_EQ(cftell(fp), cfilelength(fp)); + + cfclose(fp); +} + +namespace { +// A file on disk that holds fewer bytes than its recorded size, as when it shrinks after it was opened, so reads come +// up short. The one-byte offset opens it the way a VP entry is opened, which takes the size from the location. +CFILE* open_short_file(const SCP_string& path, const SCP_vector& bytes, size_t recorded_size) +{ + FILE* fp = fopen(path.c_str(), "wb"); + if (fp == nullptr) { + return nullptr; + } + bool written = (fputc(0, fp) == 0) && (fwrite(bytes.data(), 1, bytes.size(), fp) == bytes.size()); + if ((fclose(fp) != 0) || !written) { + return nullptr; + } + + CFileLocation res(true); + res.name_ext = "short.bin"; + res.full_name = path; + res.offset = 1; + res.size = recorded_size; + return cfopen_special(res, "rb"); +} + +SCP_vector length_then_chars(int len, const char* chars) +{ + SCP_vector bytes; + for (int i = 0; i < 4; ++i) { + bytes.push_back(static_cast((len >> (8 * i)) & 0xFF)); + } + bytes.insert(bytes.end(), chars, chars + strlen(chars)); + return bytes; +} +} // namespace + +TEST_F(CFileTest, string_len_failed_read_skips_the_string) +{ + SCP_string path = testing::TempDir() + "cfile_short_read.bin"; + auto remove_file = util::finally([&] { remove(path.c_str()); }); + auto fp = open_short_file(path, length_then_chars(10, "hel"), 18); + ASSERT_NE(fp, nullptr); + + char buf[16] = "old"; + expect_warning("could not read", [&] { cfread_string_len(buf, sizeof(buf), fp); }); + EXPECT_STREQ(buf, ""); + EXPECT_EQ(cftell(fp), 14); + cfclose(fp); + + fp = open_short_file(path, length_then_chars(10, "hel"), 18); + ASSERT_NE(fp, nullptr); + SCP_string str = "old"; + expect_warning("could not read", [&] { str = cfread_string_len(fp); }); +#ifdef NDEBUG + EXPECT_EQ(str, ""); +#endif + EXPECT_EQ(cftell(fp), 14); + cfclose(fp); +} + +#ifdef NDEBUG +TEST_F(CFileTest, string_len_zero_buffer_warns_and_writes_nothing) +{ + StringLenFile file; + file.add_int(3).add_chars("abc", 3); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + char buf[2] = {'X', 'X'}; + int warnings = Global_warning_count; + cfread_string_len(buf + 1, 0, fp); + EXPECT_EQ(buf[0], 'X'); + EXPECT_EQ(buf[1], 'X'); + EXPECT_EQ(Global_warning_count, warnings + 1); + EXPECT_EQ(cftell(fp), cfilelength(fp)); + + cfclose(fp); +} +#endif + +TEST_F(CFileTest, string_len_negative_in_last_section_throws) +{ + StringLenFile file; + file.add_int(-5); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + cf_set_max_read_len(fp, cfilelength(fp)); + char buf[8]; + EXPECT_THROW(cfread_string_len(buf, sizeof(buf), fp), cfile::max_read_length); + cfclose(fp); + + fp = file.open(); + ASSERT_NE(fp, nullptr); + cf_set_max_read_len(fp, cfilelength(fp)); + EXPECT_THROW(cfread_string_len(fp), cfile::max_read_length); + cfclose(fp); +} + +TEST_F(CFileTest, string_len_past_end_in_last_section_warns) +{ + StringLenFile file; + file.add_int(1000000).add_chars("hi", 2); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + cf_set_max_read_len(fp, cfilelength(fp)); + SCP_string str; + expect_warning("corrupt string length 1000000", [&] { str = cfread_string_len(fp); }); +#ifdef NDEBUG + EXPECT_EQ(str, "hi"); +#endif + EXPECT_EQ(cftell(fp), cfilelength(fp)); + cfclose(fp); +} + +TEST_F(CFileTest, string_len_truncated_section_reads_what_is_left) +{ + StringLenFile file; + file.add_int(20).add_chars("abc", 3); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + // the section claims more bytes than the file still has, as in a truncated pilot file + cf_set_max_read_len(fp, 10); + char buf[8]; + memset(buf, 'X', sizeof(buf)); + expect_warning("corrupt string length 20", [&] { cfread_string_len(buf, sizeof(buf), fp); }); + EXPECT_STREQ(buf, "abc"); + EXPECT_EQ(cftell(fp), cfilelength(fp)); + cfclose(fp); +} + +TEST_F(CFileTest, string_len_cut_off_length_field_warns) +{ + StringLenFile file; + file.add_chars("\x05\x00", 2); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + char buf[8] = "old"; + expect_warning("length field", [&] { cfread_string_len(buf, sizeof(buf), fp); }); + EXPECT_STREQ(buf, ""); + EXPECT_EQ(cftell(fp), cfilelength(fp)); + cfclose(fp); + + fp = file.open(); + ASSERT_NE(fp, nullptr); + SCP_string str = "old"; + expect_warning("length field", [&] { str = cfread_string_len(fp); }); +#ifdef NDEBUG + EXPECT_EQ(str, ""); +#endif + EXPECT_EQ(cftell(fp), cfilelength(fp)); + cfclose(fp); +} + +#ifdef NDEBUG +TEST_F(CFileTest, string_len_zero_buffer_still_warns_on_corrupt_length) +{ + StringLenFile file; + file.add_int(-5); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + char buf[2] = {'X', 'X'}; + expect_warning("corrupt string length -5", [&] { cfread_string_len(buf + 1, 0, fp); }); + EXPECT_EQ(buf[0], 'X'); + EXPECT_EQ(buf[1], 'X'); + cfclose(fp); +} +#endif + +TEST_F(CFileTest, string_len_cut_off_length_field_inside_max_read_len_warns) +{ + StringLenFile file; + file.add_chars("\x05\x00", 2); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + cf_set_max_read_len(fp, cfilelength(fp)); + char buf[8] = "old"; + expect_warning("length field", [&] { cfread_string_len(buf, sizeof(buf), fp); }); + EXPECT_STREQ(buf, ""); + EXPECT_EQ(cftell(fp), cfilelength(fp)); + cfclose(fp); +} + +TEST_F(CFileTest, string_len_clamped_but_still_past_max_read_len_throws) +{ + StringLenFile file; + file.add_int(20).add_chars("0123456789", 10); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + // a 2-byte buffer keeps the body read inside the limit, so only the header's check can throw + cf_set_max_read_len(fp, 6); + char buf[2]; + EXPECT_THROW(cfread_string_len(buf, sizeof(buf), fp), cfile::max_read_length); + cfclose(fp); +} + +TEST_F(CFileTest, string_len_length_field_cut_by_max_read_len_throws) +{ + StringLenFile file; + file.add_int(5).add_chars("abcde", 5); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + cf_set_max_read_len(fp, 2); + char buf[8]; + EXPECT_THROW(cfread_string_len(buf, sizeof(buf), fp), cfile::max_read_length); + cfclose(fp); +} + +TEST_F(CFileTest, cfread_keeps_whole_elements_at_end_of_file) +{ + StringLenFile file; + file.add_chars("abcdef", 6); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + char buf[12]; + EXPECT_EQ(cfread(buf, 4, 3, fp), 1); + EXPECT_EQ(cftell(fp), 6); + cfclose(fp); +} + +TEST_F(CFileTest, cfread_at_end_of_file_does_not_check_max_read_len) +{ + StringLenFile file; + file.add_int(42); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + cf_set_max_read_len(fp, 2); + cfseek(fp, 0, CF_SEEK_END); + int value = 0; + EXPECT_NO_THROW(EXPECT_EQ(cfread(&value, sizeof(value), 1, fp), 0)); + cfclose(fp); +} + +TEST_F(CFileTest, string_len_clean_end_of_file_inside_max_read_len_is_silent) +{ + StringLenFile file; + file.add_int(42); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + cf_set_max_read_len(fp, 4); + ASSERT_EQ(cfread_int(fp), 42); + + char buf[8]; + memset(buf, 'X', sizeof(buf)); + expect_no_warning([&] { cfread_string_len(buf, sizeof(buf), fp); }); + EXPECT_STREQ(buf, ""); + cfclose(fp); +} + +TEST_F(CFileTest, string_len_missing_length_field_before_the_end_warns) +{ + SCP_string path = testing::TempDir() + "cfile_missing_length.bin"; + auto remove_file = util::finally([&] { remove(path.c_str()); }); + auto fp = open_short_file(path, {}, 8); + ASSERT_NE(fp, nullptr); + + char buf[8]; + memset(buf, 'X', sizeof(buf)); + expect_warning("length field", [&] { cfread_string_len(buf, sizeof(buf), fp); }); + EXPECT_STREQ(buf, ""); + cfclose(fp); +} + +#ifdef NDEBUG +TEST_F(CFileTest, string_len_zero_buffer_and_empty_string_is_silent) +{ + StringLenFile file; + file.add_int(0); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + char buf[2] = {'X', 'X'}; + expect_no_warning([&] { cfread_string_len(buf + 1, 0, fp); }); + EXPECT_EQ(buf[1], 'X'); + cfclose(fp); +} +#endif + +TEST_F(CFileTest, string_len_length_field_cut_short_on_disk_warns) +{ + SCP_string path = testing::TempDir() + "cfile_short_length.bin"; + auto remove_file = util::finally([&] { remove(path.c_str()); }); + auto fp = open_short_file(path, {'\x05', '\x00'}, 18); + ASSERT_NE(fp, nullptr); + + char buf[8]; + memset(buf, 'X', sizeof(buf)); + expect_warning("length field", [&] { cfread_string_len(buf, sizeof(buf), fp); }); + EXPECT_STREQ(buf, ""); + EXPECT_EQ(cftell(fp), 2); + cfclose(fp); +} + +#ifndef NDEBUG +TEST_F(CFileTest, string_len_zero_buffer_asserts) +{ + StringLenFile file; + file.add_int(3).add_chars("abc", 3); + auto fp = file.open(); + ASSERT_NE(fp, nullptr); + + char buf[2] = {'X', 'X'}; + EXPECT_THROW(cfread_string_len(buf + 1, 0, fp), os::dialogs::AssertException); + EXPECT_EQ(buf[0], 'X'); + EXPECT_EQ(buf[1], 'X'); + cfclose(fp); +} +#endif