Close the RSBK name-collision class: ASCII case folding, UTF-8 well-formedness, nested-path traversal
All three are format-locked and validated on encode and decode. Repeated known keys now reject at the root and inside an entry rather than last-wins.
This commit is contained in:
@@ -25,13 +25,22 @@ landing after the format.
|
||||
header (so the refusal can name the writer's semver and version) and nothing
|
||||
else — no manifest, no layout, no half-success. The fields through the writer
|
||||
semver are FROZEN for all future versions to keep that refusal producible.
|
||||
- **Path expression is structurally impossible.** Entry names are bare file
|
||||
names (`isValidEntryName`: no separators, no `..` component, no
|
||||
drive/UNC/rooted form, no control bytes, no Windows-reserved character, no
|
||||
trailing dot/space, no DOS device name), enforced on encode AND decode
|
||||
because a package can arrive from anywhere. There is no field in the format
|
||||
capable of expressing a path. The rule set is the full authority; see
|
||||
`package_format.h`'s doc comment for the itemized list.
|
||||
- **Every name and path in the format is validated on encode AND decode**,
|
||||
because a package can arrive from anywhere. Three rules, all in
|
||||
`package_format`, whose doc comments are the itemized authority:
|
||||
- `isValidEntryName` — a payload's name is a bare file name (no separators,
|
||||
no `..` component, no drive/UNC/rooted form, no control bytes, no
|
||||
Windows-reserved character, no trailing dot/space, no DOS device name,
|
||||
well-formed UTF-8 only). Path expression is impossible in this field.
|
||||
- `sameEntryName` — two entry names differing only by ASCII case are ONE
|
||||
name. Windows and macOS's default APFS would extract them onto a single
|
||||
file, and a bank authored on a case-sensitive filesystem produces the pair
|
||||
honestly.
|
||||
- `isValidNestedSamplePath` — the nested `Sample::relativePath` IS a path by
|
||||
design, and is the one field here that can express one. It refuses a `..`
|
||||
component and every absolute form; `BankModel::add` checks only the latter,
|
||||
so traversal would otherwise reach a future `import_plan` inside a record
|
||||
the format vouched for.
|
||||
- **Framing only, never a payload.** `bank_package` produces header bytes and
|
||||
an ordered `{name, offset, length}` layout; it never holds, copies, or hashes
|
||||
an entry's audio. `decodePackage` proves prefix + payload lengths equal the
|
||||
@@ -48,8 +57,8 @@ landing after the format.
|
||||
|
||||
- `package_format` — the contract: magic, `kPackageFormatVersion` /
|
||||
`kPackageMinReaderVersion`, the ladder comment, the three-way
|
||||
`classifyPackageVersion` (`Readable` / `TooNew` / `Malformed`), the
|
||||
entry-name rule, and `PackageHeader`.
|
||||
`classifyPackageVersion` (`Readable` / `TooNew` / `Malformed`), the three
|
||||
naming rules above, and `PackageHeader`.
|
||||
- `package_manifest` — the manifest model (`PackageEntry` / `PackageManifest`)
|
||||
and its JSON codec. Per entry: bare name, byte length, and a whole-file
|
||||
`capture::hashBytes` digest (deliberately NOT `hashWavContent`, which skips
|
||||
@@ -87,11 +96,36 @@ landing after the format.
|
||||
region" — both refuse whole and write nothing, so the safety property is
|
||||
unchanged, only the message. `classifyPackageVersion` and the frozen-region
|
||||
`TooNew` path are unaffected; this is the post-manifest-parse branch only.
|
||||
- **The parse branch is the ONLY one that relabels**, deliberately: a newer
|
||||
package that trips the manifest cap, a short manifest read, the layout
|
||||
overflow, or the exact-size proof still reports `Malformed` even with
|
||||
`formatVersion` above ours. The size proof clearly should — "install 1.9.0"
|
||||
does not fix a truncated download — and the other three are indistinguishable
|
||||
from ordinary corruption at the point they fail. Don't "complete" the relabel
|
||||
across them for symmetry; the split is the answer, not an omission.
|
||||
- The format carries no algorithm tag for `byteHash` — it is FNV-1a
|
||||
(`capture::hashBytes`) implicitly. Changing the digest algorithm is a
|
||||
`minReaderVersion` bump, not additive: an old reader would otherwise compare
|
||||
a stored digest against bytes hashed the new way and silently misjudge
|
||||
corruption.
|
||||
- **Obligation on the export track: sanitize, don't relay the refusal.**
|
||||
`serializeManifest` returns one indistinguishable `nullopt` for every rejection
|
||||
— an unrepresentable name, a case-folded collision, a traversing nested path, a
|
||||
zero-length entry, a record `BankModel::add` refuses — and most of the naming
|
||||
rules are Windows'. A bank ingested on macOS/Linux legitimately holds
|
||||
`Hit?.wav`, `snare .wav`, or two names differing only by case, and a nested
|
||||
`relativePath` is only checked for the absolute forms where it is written.
|
||||
Relaying the `nullopt` makes ONE such file an unactionable total failure of the
|
||||
whole export. `export_plan` must map bank entries to package
|
||||
names that satisfy these rules (and disambiguate case-folded collisions) before
|
||||
calling this layer; the codec's refusal is the backstop, not the user-facing
|
||||
behaviour.
|
||||
- **NFC/NFD normalization collisions are accepted, not solved.** macOS compares
|
||||
file names normalization-insensitively, so the NFC and NFD spellings of one
|
||||
accented name are two manifest entries that extract onto one file — the same
|
||||
collision class as the ASCII case fold, which `sameEntryName` does catch. A
|
||||
table-free fix does not exist, and restricting names to ASCII would be
|
||||
genuinely over-strict for non-English users. Left open knowingly.
|
||||
- **Cross-module contract with `src/shell/package`:** a genuinely zero-length
|
||||
entry cannot round-trip through the filesystem seam there (`appendPayload`
|
||||
refuses an empty payload — an empty buffer signals an upstream read failure,
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
#include "core/package/package_format.h"
|
||||
|
||||
#include <cctype>
|
||||
#include <cstddef>
|
||||
|
||||
#include "core/util/relative_path.h"
|
||||
|
||||
namespace reasampler::package {
|
||||
|
||||
@@ -14,21 +16,64 @@ PackageReadability classifyPackageVersion(std::uint32_t formatVersion,
|
||||
|
||||
namespace {
|
||||
|
||||
// Hand-rolled rather than std::tolower: that fold is locale-dependent, so two
|
||||
// machines reading the same package could disagree on which names collide.
|
||||
char lowerAscii(unsigned char c) {
|
||||
return (c >= 'A' && c <= 'Z') ? static_cast<char>(c - 'A' + 'a') : static_cast<char>(c);
|
||||
}
|
||||
|
||||
// Windows device names claim the whole entry regardless of extension
|
||||
// (CON, CON.wav, con.WAV are all the same reserved device) — checked against
|
||||
// the portion before the first dot only.
|
||||
// the portion before the first dot only. The trailing three pairs are the UTF-8
|
||||
// spellings of COM¹/COM²/COM³/LPT¹/LPT²/LPT³: Windows reads those ISO 8859-1
|
||||
// superscripts as digits in a device name. COM0/LPT0 are NOT reserved.
|
||||
bool isDosDeviceName(const std::string& name) {
|
||||
std::string base = name.substr(0, name.find('.'));
|
||||
for (char& c : base) c = static_cast<char>(std::toupper(static_cast<unsigned char>(c)));
|
||||
for (char& c : base) c = lowerAscii(static_cast<unsigned char>(c));
|
||||
static const std::string kReserved[] = {
|
||||
"CON", "PRN", "AUX", "NUL",
|
||||
"COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9",
|
||||
"LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9",
|
||||
"con", "prn", "aux", "nul",
|
||||
"com1", "com2", "com3", "com4", "com5", "com6", "com7", "com8", "com9",
|
||||
"lpt1", "lpt2", "lpt3", "lpt4", "lpt5", "lpt6", "lpt7", "lpt8", "lpt9",
|
||||
"com\xC2\xB9", "com\xC2\xB2", "com\xC2\xB3",
|
||||
"lpt\xC2\xB9", "lpt\xC2\xB2", "lpt\xC2\xB3",
|
||||
};
|
||||
for (const auto& r : kReserved) if (base == r) return true;
|
||||
return false;
|
||||
}
|
||||
|
||||
// Table-free UTF-8 well-formedness. Overlong encodings, surrogate halves and
|
||||
// code points past U+10FFFF are rejected as hard as a structural length error:
|
||||
// the UTF-8 -> UTF-16 conversion a host must perform maps an ill-formed
|
||||
// sequence to U+FFFD unless it opts into failing, so two names differing only
|
||||
// in invalid bytes would otherwise collapse onto one destination file.
|
||||
bool isWellFormedUtf8(const std::string& s) {
|
||||
const auto* p = reinterpret_cast<const unsigned char*>(s.data());
|
||||
const std::size_t n = s.size();
|
||||
for (std::size_t i = 0; i < n;) {
|
||||
const unsigned char c = p[i];
|
||||
std::size_t extra = 0;
|
||||
std::uint32_t cp = 0;
|
||||
if (c < 0x80) { ++i; continue; }
|
||||
else if ((c & 0xE0) == 0xC0) { extra = 1; cp = c & 0x1Fu; }
|
||||
else if ((c & 0xF0) == 0xE0) { extra = 2; cp = c & 0x0Fu; }
|
||||
else if ((c & 0xF8) == 0xF0) { extra = 3; cp = c & 0x07u; }
|
||||
else return false; // a stray continuation byte, or a 5/6-byte lead
|
||||
if (i + extra >= n) return false;
|
||||
for (std::size_t k = 1; k <= extra; ++k) {
|
||||
const unsigned char cont = p[i + k];
|
||||
if ((cont & 0xC0) != 0x80) return false;
|
||||
cp = (cp << 6) | (cont & 0x3Fu);
|
||||
}
|
||||
if (extra == 1 && cp < 0x80) return false;
|
||||
if (extra == 2 && cp < 0x800) return false;
|
||||
if (extra == 3 && cp < 0x10000) return false;
|
||||
if (cp > 0x10FFFF) return false;
|
||||
if (cp >= 0xD800 && cp <= 0xDFFF) return false;
|
||||
i += extra + 1;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
bool isValidEntryName(const std::string& name) {
|
||||
@@ -46,7 +91,29 @@ bool isValidEntryName(const std::string& name) {
|
||||
if (c == '*' || c == '?' || c == '|' || c == '<' || c == '>' || c == '"') return false;
|
||||
}
|
||||
if (isDosDeviceName(name)) return false;
|
||||
return isWellFormedUtf8(name);
|
||||
}
|
||||
|
||||
bool sameEntryName(const std::string& a, const std::string& b) {
|
||||
if (a.size() != b.size()) return false;
|
||||
for (std::size_t i = 0; i < a.size(); ++i)
|
||||
if (lowerAscii(static_cast<unsigned char>(a[i])) !=
|
||||
lowerAscii(static_cast<unsigned char>(b[i])))
|
||||
return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
bool isValidNestedSamplePath(const std::string& path) {
|
||||
if (util::isAbsolutePath(path)) return false;
|
||||
// Component-wise, not a substring scan: "take..final/a.wav" is a legal
|
||||
// relative path, "bank/../evil.wav" is not.
|
||||
for (std::size_t start = 0;; ) {
|
||||
const std::size_t sep = path.find_first_of("/\\", start);
|
||||
const std::size_t end = (sep == std::string::npos) ? path.size() : sep;
|
||||
if (path.compare(start, end - start, "..") == 0) return false;
|
||||
if (sep == std::string::npos) return true;
|
||||
start = sep + 1;
|
||||
}
|
||||
}
|
||||
|
||||
} // namespace reasampler::package
|
||||
|
||||
@@ -56,25 +56,39 @@ enum class PackageReadability {
|
||||
PackageReadability classifyPackageVersion(std::uint32_t formatVersion,
|
||||
std::uint32_t minReaderVersion);
|
||||
|
||||
// The entry-name rule that makes path expression structurally impossible: a
|
||||
// bare file name only. Rejects empty, ".", the exact ".." component (a name
|
||||
// can only ever be one component, since separators are banned below — a
|
||||
// substring scan would over-reject legal names like "take..final.wav"), any
|
||||
// control byte (NUL included — truncates at the first filesystem call and
|
||||
// collides two distinct manifest entries onto one file) or 0x7F, any '/',
|
||||
// '\\' or ':' (which also bans every absolute form — drive, UNC, rooted), any
|
||||
// Windows-reserved character (`*?|<>"`), a trailing dot or space (silently
|
||||
// stripped at file creation, so "a.wav " and "a.wav" would collide), a DOS
|
||||
// device name (CON/PRN/AUX/NUL/COM1-9/LPT1-9, case-insensitive, with or
|
||||
// without an extension), and names over kMaxEntryNameBytes. Enforced on
|
||||
// encode AND decode by package_manifest.
|
||||
// The entry-name rule: a bare file name only. Rejects empty, ".", the exact
|
||||
// ".." component (a name can only ever be one component, since separators are
|
||||
// banned below — a substring scan would over-reject legal names like
|
||||
// "take..final.wav"), any control byte (NUL included — truncates at the first
|
||||
// filesystem call and collides two distinct manifest entries onto one file) or
|
||||
// 0x7F, any '/', '\\' or ':' (which also bans every absolute form — drive, UNC,
|
||||
// rooted), any Windows-reserved character (`*?|<>"`), a trailing dot or space
|
||||
// (silently stripped at file creation, so "a.wav " and "a.wav" would collide),
|
||||
// a DOS device name (CON/PRN/AUX/NUL/COM1-9/LPT1-9 plus the superscript
|
||||
// COM/LPT 1-3 forms, case-insensitive, with or without an extension), names
|
||||
// over kMaxEntryNameBytes, and any byte sequence that is not well-formed UTF-8.
|
||||
bool isValidEntryName(const std::string& name);
|
||||
|
||||
// The format's name-equivalence rule: two entry names that differ only by ASCII
|
||||
// case are ONE name. Windows and macOS's default APFS are case-insensitive, so
|
||||
// "Kick.wav" and "kick.wav" would extract onto a single file — and a bank
|
||||
// authored on a case-sensitive filesystem produces that pair honestly. Non-ASCII
|
||||
// bytes compare exactly (see this directory's CLAUDE.md on NFC/NFD).
|
||||
bool sameEntryName(const std::string& a, const std::string& b);
|
||||
|
||||
// The one field in the format that CAN express a path: a nested Sample's
|
||||
// relativePath, which is bank-relative by design. Rejects every absolute form
|
||||
// (the shared util::isAbsolutePath test) and any ".." component — BankModel::add
|
||||
// checks only the former, so traversal reaches the format without this.
|
||||
bool isValidNestedSamplePath(const std::string& path);
|
||||
|
||||
// The fixed header, informational semver included. writerVersion is
|
||||
// version::stampVersion() on the write side — it exists so a TooNew refusal can
|
||||
// tell the user which build to install; it never gates. Defaults are 0/0, NOT
|
||||
// the current ladder pair — a caller reading `header` after a Malformed
|
||||
// verdict must see an obviously-unset value, not a plausible-looking 1/1.
|
||||
// the current ladder pair, so a header that never parsed reads as obviously
|
||||
// unset rather than as a plausible 1/1. The fields are meaningful whenever they
|
||||
// are non-zero, not only on success: a decode that got past the header and
|
||||
// failed later (a corrupt manifest) reports the real pair alongside Malformed.
|
||||
struct PackageHeader {
|
||||
std::uint32_t formatVersion = 0;
|
||||
std::uint32_t minReaderVersion = 0;
|
||||
|
||||
@@ -13,11 +13,12 @@ using json::numToStr;
|
||||
using ObjWriter = json::Writer;
|
||||
|
||||
// Shared by serializeManifest and deserializeManifest — see this directory's
|
||||
// CLAUDE.md for why duplicate names are rejected both ways.
|
||||
// CLAUDE.md for why duplicate names are rejected both ways. Equivalence is the
|
||||
// format's, not std::string's: sameEntryName folds ASCII case.
|
||||
bool duplicateName(const std::vector<PackageEntry>& entries) {
|
||||
for (std::size_t i = 0; i < entries.size(); ++i)
|
||||
for (std::size_t j = i + 1; j < entries.size(); ++j)
|
||||
if (entries[i].fileName == entries[j].fileName) return true;
|
||||
if (sameEntryName(entries[i].fileName, entries[j].fileName)) return true;
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -45,6 +46,7 @@ bool PackageManifest::operator==(const PackageManifest& o) const {
|
||||
std::optional<std::string> serializeManifest(const PackageManifest& m) {
|
||||
for (const auto& e : m.entries) {
|
||||
if (!isValidEntryName(e.fileName)) return std::nullopt;
|
||||
if (!isValidNestedSamplePath(e.sample.relativePath)) return std::nullopt;
|
||||
// Cross-module contract with src/shell/package — see this directory's
|
||||
// CLAUDE.md.
|
||||
if (e.byteLength == 0) return std::nullopt;
|
||||
@@ -122,19 +124,22 @@ bool parseEntry(json::Reader& r, PackageEntry& e) {
|
||||
std::string key;
|
||||
if (!r.parseKey(key)) return false;
|
||||
|
||||
// A repeated key is rejected here exactly as at the root — same format
|
||||
// question, one level down.
|
||||
if (key == "name") {
|
||||
if (!r.parseString(e.fileName)) return false;
|
||||
if (haveName || !r.parseString(e.fileName)) return false;
|
||||
haveName = true;
|
||||
} else if (key == "length") {
|
||||
std::int64_t v = 0;
|
||||
if (!r.parseInt64(v)) return false;
|
||||
if (haveLength || !r.parseInt64(v)) return false;
|
||||
if (v < 0) return false;
|
||||
e.byteLength = static_cast<std::uint64_t>(v);
|
||||
haveLength = true;
|
||||
} else if (key == "hash") {
|
||||
if (!r.parseString(e.byteHash)) return false;
|
||||
if (haveHash || !r.parseString(e.byteHash)) return false;
|
||||
haveHash = true;
|
||||
} else if (key == "index") {
|
||||
if (haveSample) return false;
|
||||
std::string raw;
|
||||
if (!r.captureValue(raw)) return false;
|
||||
auto idx = model::BankModel::deserialize(raw);
|
||||
@@ -150,25 +155,31 @@ bool parseEntry(json::Reader& r, PackageEntry& e) {
|
||||
|
||||
if (!r.consume('}')) return false;
|
||||
if (!haveName || !haveLength || !haveHash || !haveSample) return false;
|
||||
return isValidEntryName(e.fileName);
|
||||
return isValidEntryName(e.fileName) && isValidNestedSamplePath(e.sample.relativePath);
|
||||
}
|
||||
|
||||
bool parseManifest(json::Reader& r, PackageManifest& m) {
|
||||
if (!r.consume('{')) return false;
|
||||
r.skipWs();
|
||||
bool haveEntries = false;
|
||||
// "Which duplicate keys are legal" is a format contract, so it is answered
|
||||
// for every root key rather than only for the one that would accumulate:
|
||||
// a repeated key is rejected, never last-wins. Unknown keys may repeat —
|
||||
// they are skipped, and a future format must stay free to add them.
|
||||
bool haveBankName = false, haveExported = false, haveEntries = false, haveSlots = false;
|
||||
const auto firstTime = [](bool& seen) { const bool ok = !seen; seen = true; return ok; };
|
||||
if (!r.consume('}')) { // not the empty-object shortcut: parse the members
|
||||
do {
|
||||
std::string key;
|
||||
if (!r.parseKey(key)) return false;
|
||||
|
||||
if (key == "bankName") {
|
||||
if (!firstTime(haveBankName)) return false;
|
||||
if (!r.parseString(m.bankDisplayName)) return false;
|
||||
} else if (key == "exported") {
|
||||
if (!firstTime(haveExported)) return false;
|
||||
if (!r.parseInt64(m.exportTimestamp)) return false;
|
||||
} else if (key == "entries") {
|
||||
if (haveEntries) return false; // a repeated key must not accumulate
|
||||
haveEntries = true;
|
||||
if (!firstTime(haveEntries)) return false;
|
||||
if (!r.consume('[')) return false;
|
||||
r.skipWs();
|
||||
if (!r.consume(']')) {
|
||||
@@ -180,6 +191,7 @@ bool parseManifest(json::Reader& r, PackageManifest& m) {
|
||||
if (!r.consume(']')) return false;
|
||||
}
|
||||
} else if (key == "slots") {
|
||||
if (!firstTime(haveSlots)) return false;
|
||||
if (!parseSlots(r, m.slots)) return false;
|
||||
} else {
|
||||
if (!r.skipValue()) return false; // forward-compat unknown keys
|
||||
|
||||
@@ -40,18 +40,21 @@ struct PackageManifest {
|
||||
bool operator==(const PackageManifest& o) const;
|
||||
};
|
||||
|
||||
// Emits the manifest JSON. nullopt when the manifest cannot be represented:
|
||||
// an invalid or duplicate entry name, a zero-length entry (see this
|
||||
// directory's CLAUDE.md — the shell's payload-append seam cannot round-trip
|
||||
// one), or a sample record BankModel itself would reject (empty id, absolute
|
||||
// path) — refusing on encode so an undecodable package is never written.
|
||||
// Emits the manifest JSON. nullopt when the manifest cannot be represented: an
|
||||
// invalid or duplicate entry name (duplicate by sameEntryName, not string
|
||||
// equality), a nested relativePath isValidNestedSamplePath refuses, a
|
||||
// zero-length entry (see this directory's CLAUDE.md — the shell's payload-append
|
||||
// seam cannot round-trip one), or a sample record BankModel itself would reject
|
||||
// (empty id, absolute path) — refusing on encode so an undecodable package is
|
||||
// never written.
|
||||
std::optional<std::string> serializeManifest(const PackageManifest& m);
|
||||
|
||||
// Parses manifest JSON (nullopt on malformed input, never UB). Unknown keys are
|
||||
// skipped at every level, so an additive newer manifest still parses. Rejects
|
||||
// what encode rejects — entry names are validated on BOTH directions because a
|
||||
// package can arrive from anywhere — plus a missing per-entry field or a
|
||||
// negative length.
|
||||
// skipped at every level, so an additive newer manifest still parses; a repeated
|
||||
// KNOWN root key is rejected rather than last-wins. Rejects what encode rejects
|
||||
// except the zero-length entry — names and nested paths are validated on BOTH
|
||||
// directions because a package can arrive from anywhere — plus a missing
|
||||
// per-entry field or a negative length.
|
||||
std::optional<PackageManifest> deserializeManifest(const std::string& json);
|
||||
|
||||
} // namespace reasampler::package
|
||||
|
||||
@@ -142,9 +142,7 @@ static void testEncodeRefusesWhatManifestRefuses() {
|
||||
m.entries[0].fileName = "../evil.wav";
|
||||
CHECK(!encodePackage(m).has_value());
|
||||
|
||||
// A zero-length entry cannot round-trip through the shell's filesystem
|
||||
// seam (src/shell/package's appendPayload refuses an empty payload) — the
|
||||
// format layer must never produce one.
|
||||
// See src/core/package/CLAUDE.md for the shell seam that forces this.
|
||||
PackageManifest zeroLen = fixture(1, 1);
|
||||
zeroLen.entries[1].byteLength = 0;
|
||||
CHECK(!encodePackage(zeroLen).has_value());
|
||||
@@ -185,6 +183,41 @@ static void testTruncationAtEveryByteOffsetIsMalformed() {
|
||||
CHECK(decodePackage(file, file.size() - 1).status == PackageReadability::Malformed);
|
||||
}
|
||||
|
||||
// --- header defaults ---------------------------------------------------------
|
||||
|
||||
// The 0/0 defaults are not the current ladder pair, so a header that never
|
||||
// parsed cannot be mistaken for a plausible 1/1. They mean "unset", NOT "the
|
||||
// decode failed": a decode that got past the header reports the real pair
|
||||
// alongside its Malformed verdict.
|
||||
static void testHeaderDefaultsMeanUnparsed() {
|
||||
CHECK(PackageHeader{}.formatVersion == 0);
|
||||
CHECK(PackageHeader{}.minReaderVersion == 0);
|
||||
CHECK(PackageHeader{}.writerVersion.empty());
|
||||
|
||||
// Failed before the header: bad magic, and a semver truncated mid-string.
|
||||
std::vector<std::uint8_t> badMagic = rawHeader(1, 1, "1.0.0");
|
||||
appendManifest(badMagic, "{}");
|
||||
badMagic[0] = 'Z';
|
||||
const DecodedPackage magic = decodePackage(badMagic, badMagic.size());
|
||||
CHECK(magic.status == PackageReadability::Malformed);
|
||||
CHECK(magic.header == PackageHeader{});
|
||||
|
||||
std::vector<std::uint8_t> cutSemver = rawHeader(1, 1, "1.0.0");
|
||||
cutSemver.resize(18);
|
||||
CHECK(decodePackage(cutSemver, cutSemver.size()).header == PackageHeader{});
|
||||
|
||||
// Failed after it: a same-version package with a corrupt manifest is
|
||||
// Malformed, and its header is fully populated.
|
||||
std::vector<std::uint8_t> corrupt =
|
||||
rawHeader(kPackageFormatVersion, kPackageMinReaderVersion, "1.0.0");
|
||||
appendManifest(corrupt, "not json");
|
||||
const DecodedPackage late = decodePackage(corrupt, corrupt.size());
|
||||
CHECK(late.status == PackageReadability::Malformed);
|
||||
CHECK(late.header.formatVersion == kPackageFormatVersion);
|
||||
CHECK(late.header.minReaderVersion == kPackageMinReaderVersion);
|
||||
CHECK(late.header.writerVersion == "1.0.0");
|
||||
}
|
||||
|
||||
// --- version ladder: TooNew refuses whole ------------------------------------
|
||||
|
||||
static void testTooNewProducesNoManifest() {
|
||||
@@ -365,6 +398,7 @@ int main() {
|
||||
testEncodeDecodeRoundTrip();
|
||||
testEncodeRefusesWhatManifestRefuses();
|
||||
testTruncationAtEveryByteOffsetIsMalformed();
|
||||
testHeaderDefaultsMeanUnparsed();
|
||||
testTooNewProducesNoManifest();
|
||||
testAdditiveUnparseableManifestIsTooNew();
|
||||
testNewerAdditiveFormatReads();
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
// Standalone tests for reasampler::package's format contract — no REAPER, no
|
||||
// test framework. Pins the version-ladder classification (both integers, every
|
||||
// branch) and the entry-name rule that makes path expression structurally
|
||||
// impossible in a package.
|
||||
// branch) and the three naming rules: the entry-name rule, the ASCII-folding
|
||||
// name equivalence, and the nested-path traversal guard.
|
||||
|
||||
#include "../src/core/package/package_format.h"
|
||||
|
||||
@@ -111,8 +111,110 @@ static void testEntryNameRejectsWindowsHostileNames() {
|
||||
CHECK(!isValidEntryName("COM1"));
|
||||
CHECK(!isValidEntryName("com1.txt"));
|
||||
CHECK(!isValidEntryName("LPT1"));
|
||||
// Not a device name: a real filename that merely starts with one.
|
||||
// The superscript device forms (COM¹ COM² COM³ LPT¹ LPT² LPT³ in UTF-8):
|
||||
// Windows reads those as digits in a device name, so "COM².wav" is COM2.
|
||||
CHECK(!isValidEntryName("COM\xC2\xB9.wav"));
|
||||
CHECK(!isValidEntryName("com\xC2\xB2"));
|
||||
CHECK(!isValidEntryName("COM\xC2\xB3.wav"));
|
||||
CHECK(!isValidEntryName("LPT\xC2\xB9"));
|
||||
CHECK(!isValidEntryName("lpt\xC2\xB2.txt"));
|
||||
CHECK(!isValidEntryName("LPT\xC2\xB3.wav"));
|
||||
// Not a device name: a real filename that merely starts with one, and the
|
||||
// zero forms, which Windows does not reserve.
|
||||
CHECK(isValidEntryName("console.wav"));
|
||||
CHECK(isValidEntryName("COM0.wav"));
|
||||
CHECK(isValidEntryName("LPT0"));
|
||||
// Nor does a superscript past 3 name a device.
|
||||
CHECK(isValidEntryName("COM\xE2\x81\xB4.wav")); // U+2074 SUPERSCRIPT FOUR
|
||||
}
|
||||
|
||||
// --- isValidEntryName: UTF-8 well-formedness ---------------------------------
|
||||
|
||||
static void testEntryNameAcceptsWellFormedUtf8() {
|
||||
CHECK(isValidEntryName("caf\xC3\xA9.wav")); // 2-byte: é
|
||||
CHECK(isValidEntryName("\xE2\x99\xAA.wav")); // 3-byte: ♪
|
||||
CHECK(isValidEntryName("\xF0\x9F\x8E\xB5.wav")); // 4-byte: 🎵
|
||||
CHECK(isValidEntryName("\xEF\xBB\xBF.wav")); // U+FEFF, ugly but well-formed
|
||||
CHECK(isValidEntryName("\xF4\x8F\xBF\xBF.wav")); // U+10FFFF, the last code point
|
||||
}
|
||||
|
||||
static void testEntryNameRejectsIllFormedUtf8() {
|
||||
// Two names differing ONLY in their invalid bytes: a host converting to
|
||||
// UTF-16 substitutes U+FFFD for both by default, collapsing them onto one
|
||||
// file — the duplicate-name collision the manifest cannot otherwise see.
|
||||
CHECK(!isValidEntryName("a\x80.wav")); // stray continuation byte
|
||||
CHECK(!isValidEntryName("a\x81.wav"));
|
||||
// Structural: truncated sequences (a lead byte the name ends inside).
|
||||
CHECK(!isValidEntryName("a\xC3"));
|
||||
CHECK(!isValidEntryName("a\xE2\x99"));
|
||||
CHECK(!isValidEntryName("a\xF0\x9F\x8E"));
|
||||
// A lead byte followed by a non-continuation.
|
||||
CHECK(!isValidEntryName("a\xC3\x41.wav"));
|
||||
// Overlong encodings: an alternate spelling of an ASCII byte we ban.
|
||||
CHECK(!isValidEntryName("a\xC0\xAF.wav")); // overlong '/'
|
||||
CHECK(!isValidEntryName("a\xC0\x80.wav")); // overlong NUL
|
||||
CHECK(!isValidEntryName("a\xE0\x80\xAF.wav")); // overlong '/', 3-byte
|
||||
CHECK(!isValidEntryName("a\xF0\x80\x80\xAF.wav")); // overlong '/', 4-byte
|
||||
// Surrogate halves: no code point, and unrepresentable in UTF-16.
|
||||
CHECK(!isValidEntryName("a\xED\xA0\x80.wav")); // U+D800
|
||||
CHECK(!isValidEntryName("a\xED\xBF\xBF.wav")); // U+DFFF
|
||||
// Past U+10FFFF, and the 5/6-byte leads that never encode anything.
|
||||
CHECK(!isValidEntryName("a\xF4\x90\x80\x80.wav")); // U+110000
|
||||
CHECK(!isValidEntryName("a\xF5\x80\x80\x80.wav"));
|
||||
CHECK(!isValidEntryName("a\xFC\x80\x80\x80\x80\x80.wav"));
|
||||
CHECK(!isValidEntryName("a\xFF.wav"));
|
||||
}
|
||||
|
||||
// --- sameEntryName -----------------------------------------------------------
|
||||
|
||||
static void testSameEntryNameFoldsAsciiCase() {
|
||||
// A bank authored on a case-sensitive filesystem produces this pair
|
||||
// honestly; Windows and default APFS would extract both onto one file.
|
||||
CHECK(sameEntryName("Kick.wav", "kick.wav"));
|
||||
CHECK(sameEntryName("KICK.WAV", "kick.wav"));
|
||||
CHECK(sameEntryName("kick.wav", "kick.wav"));
|
||||
CHECK(!sameEntryName("kick.wav", "snare.wav"));
|
||||
CHECK(!sameEntryName("kick.wav", "kick.wave")); // length alone decides
|
||||
CHECK(!sameEntryName("", "a"));
|
||||
CHECK(sameEntryName("", ""));
|
||||
// ASCII only: "é" vs "É" are two names here (the NFC/NFD limitation this
|
||||
// shares — see this directory's CLAUDE.md).
|
||||
CHECK(!sameEntryName("caf\xC3\xA9.wav", "caf\xC3\x89.wav"));
|
||||
// Only the letters fold — the bytes flanking the ASCII range must not.
|
||||
CHECK(!sameEntryName("a[b", "a{b")); // 0x5B vs 0x7B, 'Z'+1 and 'z'+1
|
||||
CHECK(!sameEntryName("a@b", "a`b")); // 0x40 vs 0x60, 'A'-1 and 'a'-1
|
||||
}
|
||||
|
||||
// --- isValidNestedSamplePath -------------------------------------------------
|
||||
|
||||
static void testNestedSamplePathAcceptsRelative() {
|
||||
CHECK(isValidNestedSamplePath("a.wav"));
|
||||
CHECK(isValidNestedSamplePath("reasampler_bank/kick.wav"));
|
||||
CHECK(isValidNestedSamplePath("reasampler_bank\\kick.wav"));
|
||||
CHECK(isValidNestedSamplePath("deep/dir/tree/a.wav"));
|
||||
// A ".." that is not a whole component is an ordinary name.
|
||||
CHECK(isValidNestedSamplePath("take..final/a.wav"));
|
||||
CHECK(isValidNestedSamplePath("bank/..hidden"));
|
||||
CHECK(isValidNestedSamplePath("a..b"));
|
||||
}
|
||||
|
||||
static void testNestedSamplePathRejectsTraversalAndAbsolute() {
|
||||
// BankModel::add catches only the absolute forms, so traversal reaches the
|
||||
// format unless this rule stops it.
|
||||
CHECK(!isValidNestedSamplePath(".."));
|
||||
CHECK(!isValidNestedSamplePath("../evil.wav"));
|
||||
CHECK(!isValidNestedSamplePath("..\\evil.wav"));
|
||||
CHECK(!isValidNestedSamplePath("bank/../../evil.wav"));
|
||||
CHECK(!isValidNestedSamplePath("bank\\..\\evil.wav"));
|
||||
CHECK(!isValidNestedSamplePath("bank/.."));
|
||||
CHECK(!isValidNestedSamplePath("bank/../"));
|
||||
// Everything util::isAbsolutePath already catches.
|
||||
CHECK(!isValidNestedSamplePath("/rooted.wav"));
|
||||
CHECK(!isValidNestedSamplePath("\\rooted.wav"));
|
||||
CHECK(!isValidNestedSamplePath("C:/abs.wav"));
|
||||
CHECK(!isValidNestedSamplePath("C:\\abs.wav"));
|
||||
CHECK(!isValidNestedSamplePath("c:relative-to-drive.wav"));
|
||||
CHECK(!isValidNestedSamplePath("\\\\server\\share.wav"));
|
||||
}
|
||||
|
||||
int main() {
|
||||
@@ -123,6 +225,11 @@ int main() {
|
||||
testEntryNameRejectsSeparatorsAndDots();
|
||||
testEntryNameRejectsAbsolutePrefixes();
|
||||
testEntryNameRejectsWindowsHostileNames();
|
||||
testEntryNameAcceptsWellFormedUtf8();
|
||||
testEntryNameRejectsIllFormedUtf8();
|
||||
testSameEntryNameFoldsAsciiCase();
|
||||
testNestedSamplePathAcceptsRelative();
|
||||
testNestedSamplePathRejectsTraversalAndAbsolute();
|
||||
|
||||
if (g_fail == 0) {
|
||||
std::printf("package_format_tests: all passed\n");
|
||||
|
||||
+117
-20
@@ -1,7 +1,7 @@
|
||||
// Standalone tests for reasampler::package's manifest codec — no REAPER, no
|
||||
// test framework. The round-trip fixture exercises every manifest field and
|
||||
// every Sample optional in both present and absent states; the rejection suite
|
||||
// pins the entry-name rule on encode AND decode.
|
||||
// pins the naming, case-folding and traversal rules on encode AND decode.
|
||||
|
||||
#include "../src/core/package/package_manifest.h"
|
||||
|
||||
@@ -132,16 +132,58 @@ static void testEncodeRejectsBadEntryName() {
|
||||
}
|
||||
}
|
||||
|
||||
static void testDecodeRejectsBadEntryName() {
|
||||
for (const char* bad : {"..\\\\evil.wav", "../evil.wav", "dir/a.wav", "C:\\\\a.wav", ".."}) {
|
||||
// Hand-rolled JSON: a hostile package is not limited to what encode emits.
|
||||
std::string json =
|
||||
std::string("{\"entries\":[{\"name\":\"") + bad +
|
||||
// One entry, `name` spliced in as raw manifest text so a hostile spelling
|
||||
// (escapes included) is expressible — a package is not limited to what encode
|
||||
// emits.
|
||||
static std::string oneEntryJson(const std::string& name,
|
||||
const std::string& relativePath = "bank/a.wav") {
|
||||
return "{\"entries\":[{\"name\":\"" + name +
|
||||
"\",\"length\":1,\"hash\":\"h\","
|
||||
"\"index\":{\"version\":1,\"samples\":[{\"id\":\"s1\","
|
||||
"\"relativePath\":\"bank/a.wav\"}]}}]}";
|
||||
CHECK(!deserializeManifest(json).has_value());
|
||||
}
|
||||
"\"relativePath\":\"" + relativePath + "\"}]}}]}";
|
||||
}
|
||||
|
||||
static void testDecodeRejectsBadEntryName() {
|
||||
for (const char* bad : {"..\\\\evil.wav", "../evil.wav", "dir/a.wav", "C:\\\\a.wav", ".."})
|
||||
CHECK(!deserializeManifest(oneEntryJson(bad)).has_value());
|
||||
|
||||
// The rest of the rule set through decode — the direction that matters,
|
||||
// since a package can arrive from anywhere.
|
||||
CHECK(!deserializeManifest(oneEntryJson("CON.wav")).has_value()); // DOS device
|
||||
CHECK(!deserializeManifest(oneEntryJson("COM\xC2\xB2.wav")).has_value()); // COM²
|
||||
CHECK(!deserializeManifest(oneEntryJson("a.wav ")).has_value()); // trailing space
|
||||
CHECK(!deserializeManifest(oneEntryJson("a.wav.")).has_value()); // trailing dot
|
||||
CHECK(!deserializeManifest(oneEntryJson("a*b.wav")).has_value()); // reserved char
|
||||
// A NUL smuggled in as a JSON escape: the manifest text is legal, the
|
||||
// decoded name is not.
|
||||
CHECK(!deserializeManifest(oneEntryJson("a\\u0000b.wav")).has_value());
|
||||
// Ill-formed UTF-8 as raw bytes.
|
||||
CHECK(!deserializeManifest(oneEntryJson("a\xC3.wav")).has_value());
|
||||
// A lone surrogate never reaches the name rule — the JSON reader refuses
|
||||
// the unpaired \uD800 first. Pinned so that refusal cannot silently become
|
||||
// "decoded to U+FFFD and accepted".
|
||||
CHECK(!deserializeManifest(oneEntryJson("a\\ud800b.wav")).has_value());
|
||||
}
|
||||
|
||||
static void testDecodeRejectsTraversalInNestedPath() {
|
||||
// The one field in the format that CAN express a path. BankModel::add
|
||||
// catches the absolute forms only, so ".." arrives unless the package layer
|
||||
// refuses it.
|
||||
CHECK(!deserializeManifest(oneEntryJson("a.wav", "../../evil.wav")).has_value());
|
||||
CHECK(!deserializeManifest(oneEntryJson("a.wav", "bank/../evil.wav")).has_value());
|
||||
CHECK(!deserializeManifest(oneEntryJson("a.wav", "..")).has_value());
|
||||
// A ".." that is not a whole component still reads.
|
||||
CHECK(deserializeManifest(oneEntryJson("a.wav", "take..final/a.wav")).has_value());
|
||||
}
|
||||
|
||||
static void testEncodeRejectsTraversalInNestedPath() {
|
||||
PackageManifest m = fixture();
|
||||
m.entries[0].sample.relativePath = "../../evil.wav";
|
||||
CHECK(!serializeManifest(m).has_value());
|
||||
|
||||
PackageManifest m2 = fixture();
|
||||
m2.entries[0].sample.relativePath = "bank/../evil.wav";
|
||||
CHECK(!serializeManifest(m2).has_value());
|
||||
}
|
||||
|
||||
static void testDuplicateEntryNamesRejectedBothWays() {
|
||||
@@ -149,40 +191,92 @@ static void testDuplicateEntryNamesRejectedBothWays() {
|
||||
m.entries[1].fileName = m.entries[0].fileName;
|
||||
CHECK(!serializeManifest(m).has_value());
|
||||
|
||||
// Case-folded: two names one case-insensitive filesystem extracts onto a
|
||||
// single file are one name here too, in both directions.
|
||||
PackageManifest folded = fixture();
|
||||
folded.entries[1].fileName = "KICK.WAV"; // entries[0] is "kick.wav"
|
||||
CHECK(!serializeManifest(folded).has_value());
|
||||
|
||||
// Decode side, from a hand-built duplicate (a hostile package is not
|
||||
// limited to what encode emits).
|
||||
const std::string dup =
|
||||
"{\"entries\":["
|
||||
"{\"name\":\"a.wav\",\"length\":1,\"hash\":\"h\","
|
||||
"\"index\":{\"version\":1,\"samples\":[{\"id\":\"s1\",\"relativePath\":\"p\"}]}},"
|
||||
"{\"name\":\"a.wav\",\"length\":2,\"hash\":\"i\","
|
||||
"{\"name\":\"A.WAV\",\"length\":2,\"hash\":\"i\","
|
||||
"\"index\":{\"version\":1,\"samples\":[{\"id\":\"s2\",\"relativePath\":\"q\"}]}}]}";
|
||||
CHECK(!deserializeManifest(dup).has_value());
|
||||
|
||||
// Two names that differ outside the ASCII letters are still two names.
|
||||
const std::string distinct =
|
||||
"{\"entries\":["
|
||||
"{\"name\":\"a1.wav\",\"length\":1,\"hash\":\"h\","
|
||||
"\"index\":{\"version\":1,\"samples\":[{\"id\":\"s1\",\"relativePath\":\"p\"}]}},"
|
||||
"{\"name\":\"a2.wav\",\"length\":2,\"hash\":\"i\","
|
||||
"\"index\":{\"version\":1,\"samples\":[{\"id\":\"s2\",\"relativePath\":\"q\"}]}}]}";
|
||||
CHECK(deserializeManifest(distinct).has_value());
|
||||
}
|
||||
|
||||
static void testDuplicateEntriesKeyRejected() {
|
||||
// A repeated "entries" key must not accumulate into two arrays' worth of
|
||||
// entries — reject rather than silently union them.
|
||||
const std::string json =
|
||||
static void testRepeatedRootKeyRejected() {
|
||||
// A repeated "entries" must not accumulate into two arrays' worth of
|
||||
// entries; the other three assign rather than append, but "which duplicate
|
||||
// keys are legal" is one format answer, not four.
|
||||
const std::string entriesTwice =
|
||||
"{\"entries\":[{\"name\":\"a.wav\",\"length\":1,\"hash\":\"h\","
|
||||
"\"index\":{\"version\":1,\"samples\":[{\"id\":\"s1\",\"relativePath\":\"p\"}]}}],"
|
||||
"\"entries\":[{\"name\":\"b.wav\",\"length\":1,\"hash\":\"h\","
|
||||
"\"index\":{\"version\":1,\"samples\":[{\"id\":\"s2\",\"relativePath\":\"q\"}]}}]}";
|
||||
CHECK(!deserializeManifest(json).has_value());
|
||||
CHECK(!deserializeManifest(entriesTwice).has_value());
|
||||
|
||||
CHECK(!deserializeManifest("{\"bankName\":\"A\",\"bankName\":\"B\"}").has_value());
|
||||
CHECK(!deserializeManifest("{\"exported\":1,\"exported\":2}").has_value());
|
||||
CHECK(!deserializeManifest("{\"slots\":[],\"slots\":[]}").has_value());
|
||||
// Unknown keys stay repeatable: they are skipped, and a future format must
|
||||
// be free to add them.
|
||||
CHECK(deserializeManifest("{\"future\":1,\"future\":2}").has_value());
|
||||
|
||||
// The same answer one level down, so a hostile manifest cannot make two
|
||||
// readers disagree about which spelling of an entry field is the real one.
|
||||
CHECK(!deserializeManifest(
|
||||
"{\"entries\":[{\"name\":\"a.wav\",\"name\":\"b.wav\",\"length\":1,\"hash\":\"h\","
|
||||
"\"index\":{\"version\":1,\"samples\":[{\"id\":\"s1\",\"relativePath\":\"p\"}]}}]}")
|
||||
.has_value());
|
||||
CHECK(!deserializeManifest(
|
||||
"{\"entries\":[{\"name\":\"a.wav\",\"length\":1,\"length\":2,\"hash\":\"h\","
|
||||
"\"index\":{\"version\":1,\"samples\":[{\"id\":\"s1\",\"relativePath\":\"p\"}]}}]}")
|
||||
.has_value());
|
||||
CHECK(!deserializeManifest(
|
||||
"{\"entries\":[{\"name\":\"a.wav\",\"length\":1,\"hash\":\"h\",\"hash\":\"i\","
|
||||
"\"index\":{\"version\":1,\"samples\":[{\"id\":\"s1\",\"relativePath\":\"p\"}]}}]}")
|
||||
.has_value());
|
||||
CHECK(!deserializeManifest(
|
||||
"{\"entries\":[{\"name\":\"a.wav\",\"length\":1,\"hash\":\"h\","
|
||||
"\"index\":{\"version\":1,\"samples\":[{\"id\":\"s1\",\"relativePath\":\"p\"}]},"
|
||||
"\"index\":{\"version\":1,\"samples\":[{\"id\":\"s2\",\"relativePath\":\"q\"}]}}]}")
|
||||
.has_value());
|
||||
}
|
||||
|
||||
// --- rejection: structural ---------------------------------------------------
|
||||
|
||||
// A zero-length entry cannot round-trip through the shell's filesystem seam
|
||||
// (src/shell/package's appendPayload refuses an empty payload) — the format
|
||||
// layer must never produce one, so encode refuses it. Decode does not enforce
|
||||
// this (a hostile/older package declaring one is not this codec's concern).
|
||||
// See src/core/package/CLAUDE.md for the shell seam that forces this.
|
||||
static void testEncodeRejectsZeroLengthEntry() {
|
||||
PackageManifest m = fixture();
|
||||
m.entries[0].byteLength = 0;
|
||||
CHECK(!serializeManifest(m).has_value());
|
||||
}
|
||||
|
||||
// The asymmetry is deliberate: refusing a zero-length entry is an obligation on
|
||||
// what this layer WRITES, not a claim about what a package may declare. Pinned
|
||||
// so it is not "fixed" into a decode-side rejection.
|
||||
static void testDecodeAcceptsZeroLengthEntry() {
|
||||
auto m = deserializeManifest(
|
||||
"{\"entries\":[{\"name\":\"a.wav\",\"length\":0,\"hash\":\"h\","
|
||||
"\"index\":{\"version\":1,\"samples\":[{\"id\":\"s1\",\"relativePath\":\"p\"}]}}]}");
|
||||
CHECK(m.has_value());
|
||||
CHECK(m->entries.size() == 1);
|
||||
CHECK(m->entries[0].byteLength == 0);
|
||||
}
|
||||
|
||||
static void testEncodeRejectsUnrepresentableSample() {
|
||||
PackageManifest m = fixture();
|
||||
m.entries[0].sample.id.clear(); // BankModel::add rejects an empty id
|
||||
@@ -261,9 +355,12 @@ int main() {
|
||||
testUnknownKeysSkippedAtEveryLevel();
|
||||
testEncodeRejectsBadEntryName();
|
||||
testDecodeRejectsBadEntryName();
|
||||
testDecodeRejectsTraversalInNestedPath();
|
||||
testEncodeRejectsTraversalInNestedPath();
|
||||
testDuplicateEntryNamesRejectedBothWays();
|
||||
testDuplicateEntriesKeyRejected();
|
||||
testRepeatedRootKeyRejected();
|
||||
testEncodeRejectsZeroLengthEntry();
|
||||
testDecodeAcceptsZeroLengthEntry();
|
||||
testEncodeRejectsUnrepresentableSample();
|
||||
testDecodeRejectsMalformedShapes();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user