handle ptexts that actually use every byte

src/ProxyCommands.cc

@@ -397,8 +397,9 @@ static bool process_server_44_A6(shared_ptr<ServerState>,
     const auto& cmd = check_size_t<S_OpenFile_PC_GC_44_A6>(data);
     bool is_download_quest = (command == 0xA6);
 
+    string filename = cmd.filename;
     string output_filename = string_printf("%s.%s.%" PRIu64,
-        cmd.filename.c_str(),
+        filename.c_str(),
         is_download_quest ? "download" : "online", now());
     for (size_t x = 0; x < output_filename.size(); x++) {
       if (output_filename[x] < 0x20 || output_filename[x] > 0x7E || output_filename[x] == '/') {
@@ -426,7 +427,8 @@ static bool process_server_13_A7(shared_ptr<ServerState>,
     try {
       sf = &session.saving_files.at(cmd.filename);
     } catch (const out_of_range&) {
-      session.log(WARNING, "Received data for non-open file %s", cmd.filename.c_str());
+      string filename = cmd.filename;
+      session.log(WARNING, "Received data for non-open file %s", filename.c_str());
       return true;
     }
 

src/Text.cc

@@ -91,19 +91,21 @@ std::string encode_sjis(const char16_t* src, size_t src_count) {
   return ret;
 }
 
-void encode_sjis(
+size_t encode_sjis(
     char* dest,
     size_t dest_count,
     const char16_t* src,
-    size_t src_count) {
+    size_t src_count,
+    bool allow_skip_terminator) {
   const auto& table = unicode_to_sjis_table();
 
   if (dest_count == 0) {
     throw logic_error("cannot encode into zero-length buffer");
   }
 
+  const char* dest_start = dest;
   const char16_t* src_end = src + src_count;
-  const char* dest_end = dest + (dest_count - 1);
+  const char* dest_end = dest + (allow_skip_terminator ? dest_count : (dest_count - 1));
   while ((dest != dest_end) && (src != src_end) && *src) {
     uint16_t ch = *(src++);
     uint16_t translated_c = table[ch];
@@ -122,7 +124,11 @@ void encode_sjis(
       *(dest++) = translated_c & 0xFF;
     }
   }
+  if (!allow_skip_terminator || (dest != dest_end)) {
     *dest = 0;
+    dest++;
+  }
+  return dest - dest_start;
 }
 
 std::u16string decode_sjis(const char* src, size_t src_count) {
@@ -146,19 +152,21 @@ std::u16string decode_sjis(const char* src, size_t src_count) {
   return ret;
 }
 
-void decode_sjis(
+size_t decode_sjis(
     char16_t* dest,
     size_t dest_count,
     const char* src,
-    size_t src_count) {
+    size_t src_count,
+    bool allow_skip_terminator) {
   const auto& table = sjis_to_unicode_table();
 
   if (dest_count == 0) {
     throw logic_error("cannot decode into zero-length buffer");
   }
 
+  const char16_t* dest_start = dest;
   const char* src_end = src + src_count;
-  const char16_t* dest_end = dest + (dest_count - 1);
+  const char16_t* dest_end = dest + (allow_skip_terminator ? dest_count : (dest_count - 1));
   while ((dest != dest_end) && (src != src_end) && *src) {
     uint16_t src_char = *(src++);
     if (src_char & 0x80) {
@@ -172,4 +180,8 @@ void decode_sjis(
     }
     *(dest++) = table[src_char];
   };
+  if (!allow_skip_terminator || (dest != dest_end)) {
+    *(dest++) = 0;
+  }
+  return dest - dest_start;
 }

src/Text.hh

@@ -18,12 +18,20 @@
 
 // (1a) Conversion functions
 
-void encode_sjis(
+// These return the number of characters written, including the terminating null
+// character. In the case of encode_sjis, two-byte characters count as two
+// characters, so the returned number is the number of bytes written.
+// allow_skip_terminator means no null byte will be written if dest_count
+// characters are written to the output. If this argument is false, a null
+// terminator is always written, even if the string is truncated.
+size_t encode_sjis(
     char* dest, size_t dest_count,
-    const char16_t* src, size_t src_count);
+    const char16_t* src, size_t src_count,
-void decode_sjis(
+    bool allow_skip_terminator = false);
+size_t decode_sjis(
     char16_t* dest, size_t dest_count,
-    const char* src, size_t src_count);
+    const char* src, size_t src_count,
+    bool allow_skip_terminator = false);
 
 std::string encode_sjis(const char16_t* source, size_t src_count);
 std::u16string decode_sjis(const char* source, size_t src_count);
@@ -45,6 +53,13 @@ size_t text_strlen_t(const T* s) {
   return ret;
 }
 
+template <typename T>
+size_t text_strnlen_t(const T* s, size_t count) {
+  size_t ret = 0;
+  for (; s[ret] != 0 && ret < count; ret++) { }
+  return ret;
+}
+
 template <typename T>
 size_t text_streq_t(const T* a, const T* b) {
   for (;;) {
@@ -74,15 +89,28 @@ size_t text_strneq_t(const T* a, const T* b, size_t count) {
   return true;
 }
 
+template <typename T>
+size_t text_strncpy_t(T* dest, const T* src, size_t count) {
+  size_t x;
+  for (x = 0; x < count && src[x] != 0; x++) {
+    dest[x] = src[x];
+  }
+  if (x < count) {
+    dest[x++] = 0;
+  }
+  return x;
+}
+
 // Like strncpy, but *always* null-terminates the string, even if it has to
 // truncate it.
 template <typename T>
-void text_strnzcpy_t(T* dest, const T* src, size_t count) {
+size_t text_strnzcpy_t(T* dest, const T* src, size_t count) {
   size_t x;
   for (x = 0; x < count - 1 && src[x] != 0; x++) {
     dest[x] = src[x];
   }
-  dest[x] = 0;
+  dest[x++] = 0;
+  return x;
 }
 
 
@@ -90,40 +118,72 @@ void text_strnzcpy_t(T* dest, const T* src, size_t count) {
 // (2) Type conversion functions
 
 template <typename DestT, typename SrcT = DestT>
-void text_strnzcpy_t(DestT*, size_t, const SrcT*, size_t) {
+size_t text_strncpy_t(DestT*, size_t, const SrcT*, size_t) {
+  static_assert(always_false<DestT, SrcT>::v,
+      "unspecialized text_strncpy_t should never be called");
+}
+
+template <>
+inline size_t text_strncpy_t<char>(
+    char* dest, size_t dest_count, const char* src, size_t src_count) {
+  size_t count = std::min<size_t>(dest_count, src_count);
+  return text_strncpy_t(dest, src, count);
+}
+
+template <>
+inline size_t text_strncpy_t<char, char16_t>(
+    char* dest, size_t dest_count, const char16_t* src, size_t src_count) {
+  return encode_sjis(dest, dest_count, src, src_count, true);
+}
+
+template <>
+inline size_t text_strncpy_t<char16_t, char>(
+    char16_t* dest, size_t dest_count, const char* src, size_t src_count) {
+  return decode_sjis(dest, dest_count, src, src_count, true);
+}
+
+template <>
+inline size_t text_strncpy_t<char16_t>(
+    char16_t* dest, size_t dest_count, const char16_t* src, size_t src_count) {
+  size_t count = std::min<size_t>(dest_count, src_count);
+  return text_strncpy_t(dest, src, count);
+}
+
+template <typename DestT, typename SrcT = DestT>
+size_t text_strnzcpy_t(DestT*, size_t, const SrcT*, size_t) {
   static_assert(always_false<DestT, SrcT>::v,
       "unspecialized text_strnzcpy_t should never be called");
 }
 
 template <>
-inline void text_strnzcpy_t<char>(
+inline size_t text_strnzcpy_t<char>(
     char* dest, size_t dest_count, const char* src, size_t src_count) {
   size_t count = std::min<size_t>(dest_count, src_count);
-  text_strnzcpy_t(dest, src, count);
+  return text_strnzcpy_t(dest, src, count);
 }
 
 template <>
-inline void text_strnzcpy_t<char, char16_t>(
+inline size_t text_strnzcpy_t<char, char16_t>(
     char* dest, size_t dest_count, const char16_t* src, size_t src_count) {
-  encode_sjis(dest, dest_count, src, src_count);
+  return encode_sjis(dest, dest_count, src, src_count);
 }
 
 template <>
-inline void text_strnzcpy_t<char16_t, char>(
+inline size_t text_strnzcpy_t<char16_t, char>(
     char16_t* dest, size_t dest_count, const char* src, size_t src_count) {
-  decode_sjis(dest, dest_count, src, src_count);
+  return decode_sjis(dest, dest_count, src, src_count);
 }
 
 template <>
-inline void text_strnzcpy_t<char16_t>(
+inline size_t text_strnzcpy_t<char16_t>(
     char16_t* dest, size_t dest_count, const char16_t* src, size_t src_count) {
   size_t count = std::min<size_t>(dest_count, src_count);
-  text_strnzcpy_t(dest, src, count);
+  return text_strnzcpy_t(dest, src, count);
 }
 
 
 
-// (3) Packed text object for use in protocol structs
+// (3) Packed text objects for use in protocol structs
 
 template <typename ItemT, size_t Count>
 struct parray {
@@ -207,9 +267,20 @@ struct parray {
       this->items[x] = v;
     }
   }
+  void clear_after(size_t position, ItemT v = 0) {
+    for (size_t x = position; x < Count; x++) {
+      this->items[x] = v;
+    }
+  }
 } __attribute__((packed));
 
 
+// TODO: It appears that these actually do not have to be null-terminated in PSO
+// commands some of the time. As an example, creating a game with a name with
+// the maximum length results in a C1 command with no null byte between the game
+// name and the password. We should be able to handle this by making ptexts not
+// required to be null-terminated in storage - this will still be safe if we
+// limit all operations by Count.
 template <typename CharT, size_t Count>
 struct ptext : parray<CharT, Count> {
   ptext() {
@@ -236,57 +307,54 @@ struct ptext : parray<CharT, Count> {
   }
 
   size_t len() const {
-    return text_strlen_t(this->items);
+    return text_strnlen_t(this->items, Count);
-  }
-  const CharT* c_str() const {
-    return this->data();
   }
 
-  // TODO: These can be made faster by only clearing the unused space after the
+  // Q: Why is there no c_str() here?
-  // strncpy_t (if any) instead of clearing all the space every time
+  // A: Because the contents of a ptext don't have to be null-terminated.
+
   ptext& operator=(const ptext& s) {
-    this->clear();
+    memcpy(this->items, s.items, sizeof(CharT) * Count);
-    text_strnzcpy_t(this->items, Count, s.items, Count);
     return *this;
   }
   ptext& operator=(ptext&& s) = delete;
 
   template <typename OtherCharT>
   ptext& operator=(const OtherCharT* s) {
-    this->clear();
+    size_t chars_written = text_strncpy_t(this->items, Count, s, Count);
-    text_strnzcpy_t(this->items, Count, s, Count);
+    this->clear_after(chars_written);
     return *this;
   }
   template <typename OtherCharT>
   ptext& assign(const OtherCharT* s, size_t s_count) {
-    this->clear();
+    size_t chars_written = text_strncpy_t(this->items, Count, s, s_count);
-    text_strnzcpy_t(this->items, Count, s, s_count);
+    this->clear_after(chars_written);
     return *this;
   }
   template <typename OtherCharT>
   ptext& operator=(const std::basic_string<OtherCharT>& s) {
-    this->clear();
+    size_t chars_written = text_strncpy_t(this->items, Count, s.c_str(), s.size());
-    text_strnzcpy_t(this->items, Count, s.c_str(), s.size() + 1);
+    this->clear_after(chars_written);
     return *this;
   }
   template <typename OtherCharT, size_t OtherCount>
   ptext& operator=(const ptext<OtherCharT, OtherCount>& s) {
-    this->clear();
+    size_t chars_written = text_strncpy_t(this->items, Count, s.items, OtherCount);
-    text_strnzcpy_t(this->items, Count, s.items, OtherCount);
+    this->clear_after(chars_written);
     return *this;
   }
 
   template <typename OtherCharT>
   bool operator==(const OtherCharT* s) const {
-    return text_streq_t(this->items, s);
+    return text_strneq_t(this->items, s, Count);
   }
   template <typename OtherCharT>
   bool operator==(const std::basic_string<OtherCharT>& s) const {
-    return text_streq_t(this->items, s.c_str());
+    return text_strneq_t(this->items, s.c_str(), Count);
   }
   template <typename OtherCharT, size_t OtherCount>
   bool operator==(const ptext<OtherCharT, OtherCount>& s) const {
-    return text_streq_t(this->items, s.items);
+    return text_strneq_t(this->items, s.items, std::min<size_t>(Count, OtherCount));
   }
   template <typename OtherCharT>
   bool operator!=(const OtherCharT* s) const {