Merge remote-tracking branch 'origin/staging' into staging

frontend/src/app/main/data/workspace/modifiers.cljs

@@ -15,6 +15,7 @@
    [app.common.geom.point :as gpt]
    [app.common.geom.rect :as grc]
    [app.common.geom.shapes :as gsh]
+   [app.common.logging :as log]
    [app.common.math :as mth]
    [app.common.types.component :as ctk]
    [app.common.types.container :as ctn]
@@ -579,10 +580,21 @@
              modifiers (calculate-modifiers state ignore-constraints ignore-snap-pixel modif-tree page-id params)]
          (assoc state :workspace-modifiers modifiers))))))
 
+(defn- without-nil-ids
+  "Drop nil-keyed entries from a modif-tree. A nil shape id (possible in
+  production builds, where the upstream asserts are elided) would crash
+  the WASM heap write with `uuid/get-u32` being called on nil."
+  [modif-tree]
+  (if (contains? modif-tree nil)
+    (do (log/warn :hint "modif-tree contains a nil shape id; ignoring entry")
+        (dissoc modif-tree nil))
+    modif-tree))
+
 (defn- parse-structure-modifiers
   [modif-tree]
   (into
    []
+   (comp
     (mapcat
      (fn [[parent-id data]]
        (when (ctm/has-structure? (:modifiers data))
@@ -617,6 +629,8 @@
                      :index 0
                      :value (:value modifier)}]
                    nil)))))))
+    (filter (fn [{:keys [id parent]}]
+              (and (some? id) (some? parent)))))
    modif-tree))
 
 
@@ -624,7 +638,7 @@
   (let [default-transform (gmt/matrix)]
     (keep (fn [[id data]]
             (cond
-              (= id uuid/zero)
+              (or (nil? id) (= id uuid/zero))
               nil
 
               (ctm/has-geometry? (:modifiers data))
@@ -693,6 +707,7 @@
                         subtree-ids-by-id selection-rect-cache]
                  :or {ignore-constraints false ignore-snap-pixel false}
                  :as params}]
+  (let [modif-tree (without-nil-ids modif-tree)]
     (ptk/reify ::set-wasm-modifiers
       ptk/UpdateEvent
       (update [_ state]
@@ -751,7 +766,7 @@
                               (cached-translation-selrect ids (second (first modifiers)) selection-rect-cache)
                               (wasm.api/get-selection-rect ids)))]
               (rx/of (set-temporary-selrect selrect)
-                   (set-temporary-modifiers modifiers))))))))
+                     (set-temporary-modifiers modifiers)))))))))
 
 (defn propagate-structure-modifiers
   [modif-tree objects]
@@ -782,6 +797,7 @@
                         subtree-ids-by-id]
                  :or {ignore-constraints false ignore-snap-pixel false snap-ignore-axis nil undo-transation? true}
                  :as params}]
+  (let [modif-tree (without-nil-ids modif-tree)]
     (ptk/reify ::apply-wasm-modifiers
       ptk/WatchEvent
       (watch [_ state _]
@@ -886,6 +902,7 @@
                       ids)
 
                 undo-id (js/Symbol)]
+
             (rx/concat
              (if undo-transation?
                (rx/of (dwu/start-undo-transaction undo-id))
@@ -907,7 +924,7 @@
 
              (if undo-transation?
                (rx/of (dwu/commit-undo-transaction undo-id))
-             (rx/empty))))))))
+               (rx/empty)))))))))
 
 (def ^:private
   xf-rotation-shape

frontend/src/app/main/data/workspace/variants.cljs

@@ -335,14 +335,15 @@
       (let [page-id   (:current-page-id state)
             objects   (dsh/lookup-page-objects state page-id)
             shape     (get objects shape-id)
-            container (get objects (:parent-id shape))
+            container (get objects (:parent-id shape))]
-            width     (+ (:width container) (:width shape) 20) ;; 20 is the default gap for variants
+        (when (and (some? shape) (some? container))
+          (let [width (+ (:width container) (:width shape) 20) ;; 20 is the default gap for variants
                 x     (- width (+ (:width shape) 30))]         ;; 30 is the default margin for variants
             (rx/of
              (dwt/update-dimensions [(:parent-id shape)] :width width)
              (dwt/update-position shape-id
                                   {:x x}
-                              {:absolute? false}))))))
+                                  {:absolute? false}))))))))
 
 (defn add-new-variant
   "Create a new variant and add it to the variant-container"
@@ -359,8 +360,9 @@
              shape               (get objects shape-id)
              shape               (if (ctc/is-variant-container? shape)
                                    (get objects (last (:shapes shape)))
-                                   shape)
+                                   shape)]
-             component-id        (:component-id shape)
+         (when (some? shape)
+           (let [component-id        (:component-id shape)
                  component           (ctkl/get-component data component-id)
 
                  container-id        (:parent-id shape)
@@ -391,7 +393,7 @@
                  (dws/shift-select-shapes new-shape-id)
                  (dws/select-shape new-shape-id)))
               (->> (rx/of (focus-property (:id variant-container)))
-               (rx/delay 250))))))))
+                   (rx/delay 250))))))))))
 
 (defn transform-in-variant
   "Given the id of a main shape of a component, creates a variant structure for

frontend/test/frontend_tests/logic/wasm_modifiers_nil_id_test.cljs

@@ -0,0 +1,109 @@
+;; This Source Code Form is subject to the terms of the Mozilla Public
+;; License, v. 2.0. If a copy of the MPL was not distributed with this
+;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
+;;
+;; Copyright (c) KALEIDOS INC Sucursal en España SL
+
+(ns frontend-tests.logic.wasm-modifiers-nil-id-test
+  "Reproduces the production crash \"Cannot read properties of null
+   (reading '__u32_buffer')\".
+
+   A modif-tree containing a nil shape id (production builds elide the
+   asserts that catch this upstream, e.g. `update-dimensions` called
+   with `[(:parent-id shape)]` when `shape` is missing) reached
+   `wasm.api/propagate-modifiers` / `wasm.api/set-structure-modifiers`,
+   and `mem.h32/write-uuid` crashed calling `uuid/get-u32` on nil while
+   writing to the WASM heap.
+
+   These tests assert that no nil id ever crosses the WASM boundary and
+   that valid shapes in the same modif-tree are still processed."
+  (:require
+   [app.common.geom.rect :as grc]
+   [app.common.math :as mth]
+   [app.common.test-helpers.compositions :as ctho]
+   [app.common.test-helpers.files :as cthf]
+   [app.common.test-helpers.ids-map :as cthi]
+   [app.common.test-helpers.shapes :as cths]
+   [app.common.types.modifiers :as ctm]
+   [app.common.uuid :as uuid]
+   [app.main.data.workspace.modifiers :as dwm]
+   [app.render-wasm.api :as wasm.api]
+   [cljs.test :as t :include-macros true]
+   [frontend-tests.helpers.state :as ths]
+   [frontend-tests.helpers.wasm :as thw]))
+
+(def ^:private captured-geometry-entries
+  "Entries passed to `wasm.api/propagate-modifiers` during a test."
+  (atom []))
+
+(def ^:private captured-structure-entries
+  "Entries passed to `wasm.api/set-structure-modifiers` during a test."
+  (atom []))
+
+(defn- install-capturing-spies!
+  "Replace the plain WASM mocks with variants that record their input.
+   Must run after `thw/setup-wasm-mocks!` so teardown still restores
+   the real implementations."
+  []
+  (set! wasm.api/propagate-modifiers
+        (fn [entries _pixel-precision]
+          (swap! captured-geometry-entries into entries)
+          (into []
+                (map (fn [[id data]] [id (:transform data)]))
+                entries)))
+  (set! wasm.api/set-structure-modifiers
+        (fn [entries]
+          (swap! captured-structure-entries into entries)
+          nil)))
+
+(t/use-fixtures :each
+  {:before (fn []
+             (cthi/reset-idmap!)
+             (reset! captured-geometry-entries [])
+             (reset! captured-structure-entries [])
+             (thw/setup-wasm-mocks!)
+             (install-capturing-spies!))
+   :after  (fn []
+             (thw/teardown-wasm-mocks!))})
+
+(t/deftest nil-id-does-not-reach-propagate-modifiers
+  ;; A nil-keyed entry must be dropped before the WASM heap write while
+  ;; the valid entry is still resized.
+  (t/async
+    done
+    (let [file       (-> (cthf/sample-file :file1)
+                         (ctho/add-rect :rect1 :x 10 :y 20 :width 100 :height 50))
+          store      (ths/setup-store file)
+          rect       (cths/get-shape file :rect1)
+          resize     (ctm/change-dimensions-modifiers rect :width 200)
+          modif-tree {nil {:modifiers resize}
+                      (:id rect) {:modifiers resize}}
+          events     [(dwm/apply-wasm-modifiers modif-tree {:ignore-snap-pixel true})]]
+      (ths/run-store
+       store done events
+       (fn [new-state]
+         (let [entry-ids (into #{} (map first) @captured-geometry-entries)
+               file'     (ths/get-file-from-state new-state)
+               rect'     (cths/get-shape file' :rect1)
+               width     (-> rect' :points grc/points->rect :width)]
+           (t/is (not (contains? entry-ids nil)))
+           (t/is (contains? entry-ids (:id rect)))
+           (t/is (mth/close? 200 width))))))))
+
+(t/deftest nil-id-does-not-reach-set-structure-modifiers
+  ;; A nil-keyed entry with structure modifiers must not produce
+  ;; structure entries with a nil :parent or :id.
+  (t/async
+    done
+    (let [file       (-> (cthf/sample-file :file1)
+                         (ctho/add-rect :rect1 :x 10 :y 20 :width 100 :height 50))
+          store      (ths/setup-store file)
+          rect       (cths/get-shape file :rect1)
+          modif-tree {nil {:modifiers (ctm/add-children nil [(uuid/next)] 0)}
+                      (:id rect) {:modifiers (ctm/change-dimensions-modifiers rect :width 200)}}
+          events     [(dwm/apply-wasm-modifiers modif-tree {:ignore-snap-pixel true})]]
+      (ths/run-store
+       store done events
+       (fn [_new-state]
+         (t/is (every? #(some? (:parent %)) @captured-structure-entries))
+         (t/is (every? #(some? (:id %)) @captured-structure-entries)))))))

frontend/test/frontend_tests/runner.cljs

@@ -39,6 +39,7 @@
    [frontend-tests.logic.pasting-in-containers-test]
    [frontend-tests.logic.sidebar-transform-coalescing-test]
    [frontend-tests.logic.update-position-test]
+   [frontend-tests.logic.wasm-modifiers-nil-id-test]
    [frontend-tests.main-errors-test]
    [frontend-tests.plugins.comments-test]
    [frontend-tests.plugins.context-shapes-test]
@@ -138,6 +139,7 @@
    'frontend-tests.main-errors-test
    'frontend-tests.logic.sidebar-transform-coalescing-test
    'frontend-tests.logic.update-position-test
+   'frontend-tests.logic.wasm-modifiers-nil-id-test
    'frontend-tests.plugins.comments-test
    'frontend-tests.plugins.context-shapes-test
    'frontend-tests.plugins.file-test