fix(asset-bridge): an empty catalog is an absent one on every family, not just the tree
Phase 7 found this on the tree family and fixed it there. It was inline in THREE
places: the body catalogue (phase 3), statics and land (phase 5), and the tree.
`expected != null` treats "" as a real fingerprint, so a caller that serialises a
missing value as an empty string has EVERY fetch refused -- with a sentence that
names no catalog at all ("catalog is now 8159778b"), which reads as a shard
fault rather than a caller one.
All three now go through one BridgeAssets.CatalogMismatch. Three copies of a
comparison are three chances for the next family to get it wrong in a way only a
differently-written client would ever reveal.
BridgeLeases keeps its own `expected != null` and is deliberately untouched:
there the value is a world property, where an empty string is a legitimate thing
to expect.
Verified against a live shard on a stock ServUO install, every family asked three
ways -- with a real catalog, with the field absent, and with an empty string:
cliloc.table walk 67,496 rows, 12 pages
body manifest / fetch 1,095 rows; ok all three ways
static + land fetch ok all three ways
static/land carry their OWN catalog art 66a112c1 vs body 323f284f
a cross-family catalog refused 422
tree manifest / fetch 141 files incl. BOTH empty ones, all three ways
empty files carry a VALID gzip member 2 rows gunzip to 0 bytes
a STALE catalog still refused on body, static and tree
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wDDVXWMDz82WqE1i969r4
This commit is contained in:
@@ -175,7 +175,7 @@ namespace Server.Custom.Bridge
|
|||||||
|
|
||||||
string id = SourceId();
|
string id = SourceId();
|
||||||
|
|
||||||
if (expected != null && expected != id)
|
if (BridgeAssets.CatalogMismatch(expected, id))
|
||||||
{
|
{
|
||||||
BridgeAssets.Fail(reqId, "UNREADABLE",
|
BridgeAssets.Fail(reqId, "UNREADABLE",
|
||||||
"the shard's client files changed since that catalogue was read (catalog "
|
"the shard's client files changed since that catalogue was read (catalog "
|
||||||
|
|||||||
@@ -670,6 +670,30 @@ namespace Server.Custom.Bridge
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Does a caller's asserted catalog id disagree with what this shard holds?
|
||||||
|
///
|
||||||
|
/// **An absent fingerprint and an empty one mean the same thing**, and that is the
|
||||||
|
/// whole reason this is a function rather than an inline `expected != null`. A caller
|
||||||
|
/// with nothing to assert sends the field absent or empty depending on how its own
|
||||||
|
/// client serialises a missing value, and treating `""` as a real id refuses **every**
|
||||||
|
/// fetch it makes — with a sentence naming no catalog at all ("catalog is now
|
||||||
|
/// 8159778b"), which reads as a shard fault rather than a caller one.
|
||||||
|
///
|
||||||
|
/// Phase 7 found this on the tree family, where a probe passed an empty string by
|
||||||
|
/// accident. It was inline in three places by then; it is one function now, because
|
||||||
|
/// three copies of a comparison are three chances for the next family to get it wrong
|
||||||
|
/// in a way only a differently-written client would ever reveal.
|
||||||
|
///
|
||||||
|
/// Note this is deliberately NOT the shape `BridgeLeases` uses for its own `expected`:
|
||||||
|
/// there the value is a world property, where an empty string is a legitimate thing to
|
||||||
|
/// expect and `!= null` is correct.
|
||||||
|
/// </summary>
|
||||||
|
internal static bool CatalogMismatch(string expected, string actual)
|
||||||
|
{
|
||||||
|
return !String.IsNullOrEmpty(expected) && !String.Equals(expected, actual, StringComparison.Ordinal);
|
||||||
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// SHA-256, lowercase hex. Shared because the hash in a manifest row, the hash in a
|
/// SHA-256, lowercase hex. Shared because the hash in a manifest row, the hash in a
|
||||||
/// fetch row and the hash the website stores must be one function.
|
/// fetch row and the hash the website stores must be one function.
|
||||||
|
|||||||
@@ -357,7 +357,7 @@ namespace Server.Custom.Bridge
|
|||||||
|
|
||||||
string id = SourceId();
|
string id = SourceId();
|
||||||
|
|
||||||
if (expected != null && expected != id)
|
if (BridgeAssets.CatalogMismatch(expected, id))
|
||||||
{
|
{
|
||||||
// The client files moved between the manifest and this fetch. Refusing is the only
|
// The client files moved between the manifest and this fetch. Refusing is the only
|
||||||
// honest answer: the keys were chosen against a catalogue that no longer describes
|
// honest answer: the keys were chosen against a catalogue that no longer describes
|
||||||
|
|||||||
@@ -421,12 +421,10 @@ namespace Server.Custom.Bridge
|
|||||||
List<TreeFile> files = Enumerate();
|
List<TreeFile> files = Enumerate();
|
||||||
string fingerprint = FingerprintOf(files);
|
string fingerprint = FingerprintOf(files);
|
||||||
|
|
||||||
// `IsNullOrEmpty`, not `!= null`. A caller that has no fingerprint to assert sends
|
// Shared with every other family on this plane, because an absent fingerprint and an
|
||||||
// the field absent OR empty depending on how its own client serialises a missing
|
// empty one have to mean the same thing here and there — see
|
||||||
// value, and the two must mean the same thing — an empty string compared against a
|
// `BridgeAssets.CatalogMismatch` for what treating them differently costs.
|
||||||
// real id refuses every fetch, with a sentence that names no catalog at all
|
if (BridgeAssets.CatalogMismatch(expected, fingerprint))
|
||||||
// ("catalog is now 8159778b"). Found by a probe that passed one.
|
|
||||||
if (!String.IsNullOrEmpty(expected) && expected != fingerprint)
|
|
||||||
{
|
{
|
||||||
// The tree moved between the manifest and this fetch. The same refusal the
|
// The tree moved between the manifest and this fetch. The same refusal the
|
||||||
// catalogue makes for a patched client, and for the same reason: these keys were
|
// catalogue makes for a patched client, and for the same reason: these keys were
|
||||||
|
|||||||
Reference in New Issue
Block a user