Close package fs review findings: readRange bounds, picker ext, non-ASCII tests

Cap readRange's allocation and reject size_t overflow instead of truncating;
re-append .rsbank when the export picker omits it; add cafe coverage for
writeFileExclusive and writeLandedFile; loop write() on EINTR.

src/shell/package/CLAUDE.md

@@ -53,8 +53,12 @@ belong to the verbs.
   "Did this call create it" is structural: only exclusively-created paths are
   recorded, resolved absolute at record time so a later CWD change cannot re-aim the
   delete. "Did anything ever reference it" is a **contract the import verb must
-  honour**: it MUST call `markIndexCommitted()` at the moment it commits the index,
-  after which `rollback()` refuses and `writeLandedFile` refuses.
+  honour**: it MUST call `markIndexCommitted()` only AFTER the index write has
+  returned success — calling it before, then having that write fail, strands the
+  landed files with no index entry and a journal that now refuses to roll them
+  back — after which `rollback()` refuses and `writeLandedFile` refuses. (Destroying
+  an armed journal without calling either does NOT roll it back — see
+  `LandedFileJournal`'s own doc comment.)
 - **Both pickers ride `GetUserFileName`** — mode 1 for import, mode 0 for export.
   There is no platform split and no fallback: `main.cpp` defines `REAPERAPI_IMPLEMENT`
   without `REAPERAPI_MINIMAL` and aborts the extension load if any single name fails
@@ -80,7 +84,11 @@ belong to the verbs.
   pair format but are `[verify — DAW]` on all three platforms — neither picker is
   exercised outside a live REAPER session. `GetUserFileName` also takes no owner
   window, so dialog parenting is REAPER's to do; the superseded Win32 path passed
-  `GetMainHwnd()` explicitly.
+  `GetMainHwnd()` explicitly. Also `[verify — DAW]`: whether mode 0's picker appends
+  an extension from `extension_list` when the user omits one — `pickPackageSavePath`
+  re-appends `.rsbank` itself so the returned path is correct regardless of how that
+  lands (the superseded Win32 path had `ofn.lpstrDefExt` for this; `GetUserFileName`
+  has no equivalent parameter).
 - `pickPackageSavePath`'s `suggestedPath` doubles as the dialog's starting directory
   when it is a full path. The verbs should seed it from the project directory —
   passing a bare name leaves the dialog on REAPER's process working directory, which

src/shell/package/package_io.cpp

@@ -1,11 +1,13 @@
 // package_io.cpp — see package_io.h for the seam's contract. Non-throwing at the
-// boundary: every filesystem call uses the error_code form, so no filesystem_error
-// crosses into a REAPER action body.
+// boundary about filesystem_error: every filesystem call uses the error_code form.
+// Allocation can still throw bad_alloc — readRange's sanity ceiling exists to keep
+// that surface small, not to remove it.
 
 #include "shell/package/package_io.h"
 
 #include <algorithm>
 #include <atomic>
+#include <limits>
 #include <utility>
 
 #include "shell/package/package_path.h"
@@ -16,6 +18,7 @@
 #include <share.h>
 #include <sys/stat.h>
 #else
+#include <cerrno>
 #include <fcntl.h>
 #include <unistd.h>
 #endif
@@ -147,7 +150,15 @@ PackageFileReader::PackageFileReader(const std::string& srcAbsPath) {
 PayloadBuffer PackageFileReader::readRange(std::uint64_t offset, std::uint64_t length) {
     // Overflow-safe range check: length is capped by the real file size before any
     // allocation happens, so a hostile offset/length pair cannot demand the moon.
-    if (!ok_ || length == 0 || length > size_ || offset > size_ - length) {
+    // Also rejected here rather than truncated: a length that would not fit in
+    // size_t (possible on a 32-bit build, where streamsize below stays 64-bit and
+    // so would read past a truncated allocation) and a length past the sanity
+    // ceiling, which exists so a merely large-but-real file size can't still hand
+    // std::vector a multi-gigabyte demand.
+    constexpr std::uint64_t kMaxReadRangeBytes = std::uint64_t{4} << 30;  // 4 GiB
+    if (!ok_ || length == 0 || length > size_ || offset > size_ - length ||
+        length > kMaxReadRangeBytes ||
+        length > static_cast<std::uint64_t>(std::numeric_limits<std::size_t>::max())) {
         return PayloadBuffer{};
     }
     in_.clear();  // a prior failed read must not poison this one
@@ -208,6 +219,7 @@ bool writeFileExclusive(const std::string& absPath, const PayloadBuffer& payload
                              static_cast<unsigned int>(chunk));
 #else
         const ssize_t n = ::write(fd, payload.data() + written, chunk);
+        if (n < 0 && errno == EINTR) continue;  // a signal on the UI thread isn't a failure
 #endif
         if (n <= 0) {
             ok = false;
@@ -241,7 +253,7 @@ std::vector<std::string> listFolderFileNames(const std::string& dirAbsPath) {
         const auto& entry = *it;
         std::error_code reg_ec;
         if (!entry.is_regular_file(reg_ec)) continue;  // skip subdirs / specials
-        names.push_back(entry.path().filename().u8string());  // never .string(): ANSI
+        names.push_back(pathToUtf8(entry.path().filename()));  // never .string(): ANSI
     }
     std::sort(names.begin(), names.end());
     return names;

src/shell/package/package_io.h

@@ -89,7 +89,9 @@ public:
     bool ok() const { return ok_; }
     std::uint64_t fileSize() const { return size_; }
     // Bytes [offset, offset+length). Range-checked against the real file size, so a
-    // hostile layout can never demand an allocation past the file's end.
+    // hostile layout can never demand an allocation past the file's end, and capped
+    // against a 4 GiB sanity ceiling so a merely large-but-real file can't still
+    // force a multi-gigabyte allocation out of one call.
     PayloadBuffer readRange(std::uint64_t offset, std::uint64_t length);
 
 private:

src/shell/package/package_path.h

@@ -1,9 +1,9 @@
-// shell/package/package_path — the ONE narrow-string -> fs::path conversion for this
-// seam. std::filesystem decodes a narrow path through the RUNTIME ANSI code page on
-// Windows (measured: GetACP() == 1252 here), never UTF-8, so a bare
+// shell/package/package_path — the ONE narrow-string <-> fs::path conversion pair for
+// this seam. std::filesystem decodes a narrow path through the RUNTIME ANSI code page
+// on Windows (measured: GetACP() == 1252 here), never UTF-8, so a bare
 // fs::path(std::string) turns every non-ASCII path this repo's UTF-8 convention
 // produces into mojibake. u8path is the C++17 spelling; it is deprecated in C++20, so
-// a standard bump replaces the body here rather than at every call site.
+// a standard bump replaces both bodies here rather than at every call site.
 
 #pragma once
 
@@ -16,4 +16,11 @@ inline std::filesystem::path utf8Path(const std::string& utf8) {
     return std::filesystem::u8path(utf8);
 }
 
+// u8string() returns std::u8string in C++20 — this is the one place that narrows it
+// back to std::string, so a standard bump only widens this one body.
+inline std::string pathToUtf8(const std::filesystem::path& path) {
+    const auto u8 = path.u8string();
+    return std::string(u8.begin(), u8.end());
+}
+
 }  // namespace reasampler

src/shell/package/package_pickers.cpp

@@ -5,6 +5,9 @@
 
 #include "shell/package/package_pickers.h"
 
+#include <algorithm>
+#include <cctype>
+
 #define REAPERAPI_MINIMAL
 #define REAPERAPI_WANT_GetUserFileName
 #include "reaper_plugin_functions.h"
@@ -30,6 +33,14 @@ bool runPicker(int mode, const char* caption, const char* initial,
     return !outAbsPath.empty();
 }
 
+bool hasCaseInsensitiveSuffix(const std::string& path, const std::string& suffix) {
+    if (path.size() < suffix.size()) return false;
+    return std::equal(suffix.rbegin(), suffix.rend(), path.rbegin(),
+                      [](unsigned char a, unsigned char b) {
+                          return std::tolower(a) == std::tolower(b);
+                      });
+}
+
 }  // namespace
 
 bool pickPackageForImport(std::string& outAbsPath) {
@@ -39,7 +50,14 @@ bool pickPackageForImport(std::string& outAbsPath) {
 bool pickPackageSavePath(const std::string& suggestedPath, std::string& outAbsPath) {
     // [verify — DAW] GetUserFileName takes no owner window, so the dialog's parenting
     // is REAPER's to do; the previous Win32 path passed GetMainHwnd() explicitly.
-    return runPicker(0, "Export bank package", suggestedPath.c_str(), outAbsPath);
+    if (!runPicker(0, "Export bank package", suggestedPath.c_str(), outAbsPath)) {
+        return false;
+    }
+    // GetUserFileName has no lpstrDefExt equivalent (the old Win32 picker's
+    // ofn.lpstrDefExt = L"rsbank"); whether mode 0 appends one itself from
+    // kExtList is [verify — DAW], so append it ourselves whenever it's missing.
+    if (!hasCaseInsensitiveSuffix(outAbsPath, ".rsbank")) outAbsPath += ".rsbank";
+    return true;
 }
 
 }  // namespace reasampler

src/shell/package/package_pickers.h

@@ -1,7 +1,10 @@
 // shell/package/package_pickers — the two package file pickers, both on REAPER's own
 // GetUserFileName (mode 1 = existing file, mode 0 = new file). No native/platform
 // split: the SDK's save mode is not optional on any build that can load this
-// extension. Paths in and out are UTF-8, per the REAPER API contract.
+// extension. Paths in and out are UTF-8, matching this tree's established practice
+// for narrow strings crossing the REAPER API (see instrument_drop_win.cpp's
+// path.u8string() to TrackFX_SetPreset, or prune_fs.cpp's CP_UTF8 conversion) — the
+// SDK header itself never says "UTF-8".
 
 #pragma once
 

src/shell/package/package_rollback.cpp

@@ -24,7 +24,7 @@ bool LandedFileJournal::writeLandedFile(const std::string& destPath,
     std::error_code ec;
     const fs::path resolved = fs::absolute(utf8Path(destPath), ec);
     if (ec) return false;
-    const std::string absPath = resolved.u8string();
+    const std::string absPath = pathToUtf8(resolved);
 
     if (!writeFileExclusive(absPath, payload)) return false;
     paths_.push_back(absPath);

src/shell/package/package_rollback.h

@@ -19,6 +19,10 @@ struct RollbackResult {
     bool refused = false;        // markIndexCommitted() ran: nothing was deleted
 };
 
+// Destroying an armed (uncommitted, un-rolled-back) journal is NOT an implicit
+// rollback — the caller must call rollback() itself on the failure path it wants
+// to undo. That's the fail-safe direction: a journal dropped by an unrelated early
+// return leaves the landed files in place rather than silently deleting them.
 class LandedFileJournal {
 public:
     // Lands one payload at destPath through the exclusive create (which refuses an
@@ -33,7 +37,9 @@ public:
     // Disarms the journal: the index mutation these files back is committed, so they
     // are now referenced bytes and the carve-out no longer covers them. This is the
     // half of prune's discriminator the journal cannot make structural on its own —
-    // the import verb MUST call it at the moment the index is committed.
+    // the import verb MUST call this only AFTER the index write has returned success.
+    // Calling it before, then having that write fail, strands the landed files with
+    // no index entry and a journal that now refuses to roll them back.
     void markIndexCommitted() { indexCommitted_ = true; }
     bool indexCommitted() const { return indexCommitted_; }