From 921a028a535f021f5662e42fc2741688e559e410 Mon Sep 17 00:00:00 2001 From: notevil Date: Thu, 23 Apr 2026 17:03:42 +0200 Subject: [PATCH] P3-03 + P3-09 review fixes : tighten RL check + correct thread-safety doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HIGH P3-03 : strict RL check laissait passer ':path', 'mod:', ':' silencieusement (tryParse les accepte en 1.20.1). Fix combo check (isEmpty || colonIdx<=0 || colonIdx>=length-1) rejette tous les cas edge. 3 nouveaux tests couvrent les malformed variants. MEDIUM P3-09 : javadoc de BondageStateHelpers affirmait ConcurrentHashMap alors que le backing V2EquipmentHelper utilise EnumMap via pattern MC main-thread + packet sync. Correction doc pour refléter la réalité. --- .../datadriven/DataDrivenItemParser.java | 21 ++++-- .../remake/v2/client/BondageStateHelpers.java | 9 ++- .../DataDrivenItemParserAnimationsTest.java | 75 ++++++++++++++++++- 3 files changed, 93 insertions(+), 12 deletions(-) diff --git a/src/main/java/com/tiedup/remake/v2/bondage/datadriven/DataDrivenItemParser.java b/src/main/java/com/tiedup/remake/v2/bondage/datadriven/DataDrivenItemParser.java index 721198b..df3df40 100644 --- a/src/main/java/com/tiedup/remake/v2/bondage/datadriven/DataDrivenItemParser.java +++ b/src/main/java/com/tiedup/remake/v2/bondage/datadriven/DataDrivenItemParser.java @@ -849,13 +849,19 @@ public final class DataDrivenItemParser { return null; } String s = elem.getAsString(); - // Strict RL parsing : animations MUST carry an explicit namespace - // (e.g. "mymod:foo"). ResourceLocation.tryParse defaults bare paths - // to the "minecraft" namespace, which silently masks modder typos — - // we reject strings without ':' here. - if (s.isEmpty() || s.indexOf(':') < 0) { + // Strict RL parsing : animations MUST carry an explicit namespace AND + // path (e.g. "mymod:foo"). ResourceLocation.tryParse would silently + // accept ":path" as "minecraft:path", "mod:" with an empty path, or + // ":" altogether — masking modder typos. Reject all of these here : + // - isEmpty() rejects "" + // - colonIdx <= 0 rejects "noNamespace" (-1) and ":path" (0) + // - colonIdx >= length - 1 rejects "mod:" (colon at end) + // The bare ":" is rejected by colonIdx == 0. + int colonIdx = s.indexOf(':'); + if (s.isEmpty() || colonIdx <= 0 || colonIdx >= s.length() - 1) { LOGGER.warn( - "[DataDrivenItemParser] Malformed ResourceLocation '{}' for {} in item {}, skipping.", + "[DataDrivenItemParser] Malformed ResourceLocation '{}' for {} in item {} " + + "(expected 'namespace:path'), skipping.", s, context, itemId @@ -865,7 +871,8 @@ public final class DataDrivenItemParser { ResourceLocation rl = ResourceLocation.tryParse(s); if (rl == null) { LOGGER.warn( - "[DataDrivenItemParser] Malformed ResourceLocation '{}' for {} in item {}, skipping.", + "[DataDrivenItemParser] Malformed ResourceLocation '{}' for {} in item {} " + + "(invalid characters), skipping.", s, context, itemId diff --git a/src/main/java/com/tiedup/remake/v2/client/BondageStateHelpers.java b/src/main/java/com/tiedup/remake/v2/client/BondageStateHelpers.java index 7e0e213..cb39406 100644 --- a/src/main/java/com/tiedup/remake/v2/client/BondageStateHelpers.java +++ b/src/main/java/com/tiedup/remake/v2/client/BondageStateHelpers.java @@ -39,10 +39,11 @@ import net.minecraft.world.item.ItemStack; * {@code PacketSyncStruggleState}). * * - *

Thread-safety : toutes les lectures backing sont - * déjà thread-safe (volatile / ConcurrentHashMap / EntityData). Ces - * helpers peuvent être appelés depuis les threads MC usuels (client - * tick, render thread) sans synchronisation additionnelle.

+ *

Thread-safety : les APIs backing (V2EquipmentHelper, + * PlayerBindState) suivent le pattern MC standard — écriture serveur main + * thread, lecture client main thread après packet sync via + * {@code Context.enqueueWork}. Pas de lock externe requis tant que + * l'appelant est sur le main thread approprié.

* *

Note sur {@code @OnlyIn(Dist.CLIENT)} : ces helpers * fonctionnent aussi server-side par construction (ils lisent les mêmes diff --git a/src/test/java/com/tiedup/remake/v2/bondage/datadriven/DataDrivenItemParserAnimationsTest.java b/src/test/java/com/tiedup/remake/v2/bondage/datadriven/DataDrivenItemParserAnimationsTest.java index 9d7dcdb..583111b 100644 --- a/src/test/java/com/tiedup/remake/v2/bondage/datadriven/DataDrivenItemParserAnimationsTest.java +++ b/src/test/java/com/tiedup/remake/v2/bondage/datadriven/DataDrivenItemParserAnimationsTest.java @@ -32,7 +32,7 @@ import com.tiedup.remake.rig.anim.TiedUpLivingMotions; *

  • bloc vide → {@link AnimationBindings#EMPTY},
    • *
  • motions vanilla EF et custom TiedUp!,
    • *
  • tolerance typos (unknown motion) avec fuzzy-match suggestion,
    • - *
  • tolerance ResourceLocation malformee,
    • + *
  • tolerance ResourceLocation malformee (no-namespace, leading/trailing/bare colon),
    • *
  • tolerance values non-string,
    • *
  • one-shots on_equip / on_unequip,
    • *
  • Levenshtein distance (sanity check).
    • @@ -218,6 +218,79 @@ class DataDrivenItemParserAnimationsTest { assertNull(result.livingMotions().get(LivingMotions.WALK)); } + @Test + void parseAnimations_leadingColonRL_skipsEntry() { + // ":path" ne doit pas etre accepte silencieusement comme minecraft:path + // (ResourceLocation.tryParse le resoudrait sans le strict check). + String jsonStr = """ + { + "animations": { + "living_motions": { + "WALK": ":arms_cuffed_walk", + "IDLE": "tiedup:arms_cuffed_idle" + } + } + } + """; + + AnimationBindings result = + DataDrivenItemParser.parseAnimationBindings(json(jsonStr), ITEM_ID); + + assertNotNull(result); + assertEquals(1, result.livingMotions().size(), + "':path' rejete par strict check (colonIdx == 0)"); + assertNotNull(result.livingMotions().get(LivingMotions.IDLE)); + assertNull(result.livingMotions().get(LivingMotions.WALK)); + } + + @Test + void parseAnimations_trailingColonRL_skipsEntry() { + // "mod:" (empty path) doit etre rejete, tryParse l'accepterait. + String jsonStr = """ + { + "animations": { + "living_motions": { + "WALK": "tiedup:", + "IDLE": "tiedup:arms_cuffed_idle" + } + } + } + """; + + AnimationBindings result = + DataDrivenItemParser.parseAnimationBindings(json(jsonStr), ITEM_ID); + + assertNotNull(result); + assertEquals(1, result.livingMotions().size(), + "'mod:' rejete par strict check (colonIdx == length - 1)"); + assertNotNull(result.livingMotions().get(LivingMotions.IDLE)); + assertNull(result.livingMotions().get(LivingMotions.WALK)); + } + + @Test + void parseAnimations_bareColonRL_skipsEntry() { + // ":" seul doit etre rejete (colonIdx == 0 ET length - 1). + String jsonStr = """ + { + "animations": { + "living_motions": { + "WALK": ":", + "IDLE": "tiedup:arms_cuffed_idle" + } + } + } + """; + + AnimationBindings result = + DataDrivenItemParser.parseAnimationBindings(json(jsonStr), ITEM_ID); + + assertNotNull(result); + assertEquals(1, result.livingMotions().size(), + "':' rejete par strict check"); + assertNotNull(result.livingMotions().get(LivingMotions.IDLE)); + assertNull(result.livingMotions().get(LivingMotions.WALK)); + } + @Test void parseAnimations_nonStringValue_skipsEntry() { String jsonStr = """