🐛 Fix crash when a nil shape id reaches WASM modifiers

A modif-tree containing a nil shape id crashed the WASM heap write
with "Cannot read properties of null (reading '__u32_buffer')" in
propagate-modifiers. Production builds elide the upstream asserts,
so a nil id (e.g. update-dimensions called with a missing shape's
parent id in the variants flow) reached the uuid heap write.

Drop nil-keyed modif-tree entries in apply-wasm-modifiers and
set-wasm-modifiers before they reach propagate-modifiers and
set-structure-modifiers, filter nil ids at the parse level, and
guard the variant reposition/creation events against missing shapes.

AI-assisted-by: hy3
This commit is contained in:
Andrey Antukh 2026-07-28 12:09:40 +00:00
parent 319a2185c9
commit 94a6ba4afb
4 changed files with 290 additions and 161 deletions

View File

@ -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,44 +580,57 @@
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
[]
(mapcat
(fn [[parent-id data]]
(when (ctm/has-structure? (:modifiers data))
(->> (concat
(get-in data [:modifiers :structure-parent])
(get-in data [:modifiers :structure-child]))
(mapcat
(fn [modifier]
(case (:type modifier)
:remove-children
(->> (:value modifier)
(map (fn [child-id]
{:type :remove-children
:parent parent-id
:id child-id
:index 0
:value 0})))
(comp
(mapcat
(fn [[parent-id data]]
(when (ctm/has-structure? (:modifiers data))
(->> (concat
(get-in data [:modifiers :structure-parent])
(get-in data [:modifiers :structure-child]))
(mapcat
(fn [modifier]
(case (:type modifier)
:remove-children
(->> (:value modifier)
(map (fn [child-id]
{:type :remove-children
:parent parent-id
:id child-id
:index 0
:value 0})))
:add-children
(->> (:value modifier)
(map (fn [child-id]
{:type :add-children
:parent parent-id
:id child-id
:index (:index modifier)
:value 0})))
:add-children
(->> (:value modifier)
(map (fn [child-id]
{:type :add-children
:parent parent-id
:id child-id
:index (:index modifier)
:value 0})))
:scale-content
[{:type :scale-content
:parent parent-id
:id parent-id
:index 0
:value (:value modifier)}]
nil)))))))
:scale-content
[{:type :scale-content
:parent parent-id
:id parent-id
: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,26 +707,27 @@
subtree-ids-by-id selection-rect-cache]
:or {ignore-constraints false ignore-snap-pixel false}
:as params}]
(ptk/reify ::set-wasm-modifiers
ptk/UpdateEvent
(update [_ state]
(let [property-changes (extract-property-changes modif-tree)]
(if (d/not-empty? property-changes)
(-> state
(assoc :prev-wasm-props (:wasm-props state))
(assoc :wasm-props property-changes))
state)))
(let [modif-tree (without-nil-ids modif-tree)]
(ptk/reify ::set-wasm-modifiers
ptk/UpdateEvent
(update [_ state]
(let [property-changes (extract-property-changes modif-tree)]
(if (d/not-empty? property-changes)
(-> state
(assoc :prev-wasm-props (:wasm-props state))
(assoc :wasm-props property-changes))
state)))
ptk/WatchEvent
(watch [_ state _]
;; Entering an interactive transform (drag/resize/rotate). Flip
;; the renderer into fast + atlas-backdrop mode so the live
;; preview is cheap, tiles never appear sequentially and the main
;; thread is not blocked. The pair is closed in
;; `clear-local-transform`.
(ensure-interactive-transform-start!)
(let [snap-pixel? (and (not ignore-snap-pixel) (contains? (:workspace-layout state) :snap-pixel-grid))
translation? (every? #(ctm/only-move? (:modifiers %)) (vals modif-tree))]
ptk/WatchEvent
(watch [_ state _]
;; Entering an interactive transform (drag/resize/rotate). Flip
;; the renderer into fast + atlas-backdrop mode so the live
;; preview is cheap, tiles never appear sequentially and the main
;; thread is not blocked. The pair is closed in
;; `clear-local-transform`.
(ensure-interactive-transform-start!)
(let [snap-pixel? (and (not ignore-snap-pixel) (contains? (:workspace-layout state) :snap-pixel-grid))
translation? (every? #(ctm/only-move? (:modifiers %)) (vals modif-tree))])
(if translation?
;; Pure translation: no structure changes needed. If structure
@ -782,43 +797,59 @@
subtree-ids-by-id]
:or {ignore-constraints false ignore-snap-pixel false snap-ignore-axis nil undo-transation? true}
:as params}]
(ptk/reify ::apply-wasm-modifiers
ptk/WatchEvent
(watch [_ state _]
(let [translation?
(every? #(ctm/only-move? (:modifiers %)) (vals modif-tree))]
(wasm.api/clean-modifiers)
(when-not translation?
(wasm.api/set-structure-modifiers (parse-structure-modifiers modif-tree)))
(let [modif-tree (without-nil-ids modif-tree)]
(ptk/reify ::apply-wasm-modifiers
ptk/WatchEvent
(watch [_ state _]
(let [translation?
(every? #(ctm/only-move? (:modifiers %)) (vals modif-tree))]
(wasm.api/clean-modifiers)
(when-not translation?
(wasm.api/set-structure-modifiers (parse-structure-modifiers modif-tree)))
;; Apply property changes (e.g. grow-type) to WASM shapes before
;; propagating geometry, so propagate_modifiers sees the updated state.
(doseq [[id {:keys [property value]}] (extract-property-changes modif-tree)]
(when (= property :grow-type)
(wasm.api/use-shape id)
(wasm.api/set-shape-grow-type value)))
;; Apply property changes (e.g. grow-type) to WASM shapes before
;; propagating geometry, so propagate_modifiers sees the updated state.
(doseq [[id {:keys [property value]}] (extract-property-changes modif-tree)]
(when (= property :grow-type)
(wasm.api/use-shape id)
(wasm.api/set-shape-grow-type value)))
(let [objects (dsh/lookup-page-objects state)
(let [objects (dsh/lookup-page-objects state)
geometry-entries
(parse-geometry-modifiers modif-tree)
geometry-entries
(parse-geometry-modifiers modif-tree)
snap-pixel?
(and (not ignore-snap-pixel) (contains? (:workspace-layout state) :snap-pixel-grid))
snap-pixel?
(and (not ignore-snap-pixel) (contains? (:workspace-layout state) :snap-pixel-grid))]
transforms
(cond
(and translation? (not snap-pixel?))
;; Mirror WASM `propagate_modifiers` in CLJS: splat the
;; translation matrix onto every descendant. Without
;; this step the commit would only touch the dragged
;; primaries and descendants would snap back to their
;; pre-drag positions on drop.
;;
;; Skipped when `snap-pixel?` is on: WASM applies
;; per-shape pixel correction (different scale/translate
;; per descendant) which we can't replicate cheaply on
;; the CLJS side.
transforms
(cond
(and translation? (not snap-pixel?))
;; Mirror WASM `propagate_modifiers` in CLJS: splat the
;; translation matrix onto every descendant. Without
;; this step the commit would only touch the dragged
;; primaries and descendants would snap back to their
;; pre-drag positions on drop.
;;
;; Skipped when `snap-pixel?` is on: WASM applies
;; per-shape pixel correction (different scale/translate
;; per descendant) which we can't replicate cheaply on
;; the CLJS side.
(reduce
(fn [acc [id data]]
(let [t (:transform data)
subtree-ids
(or (get subtree-ids-by-id id)
(cfh/get-children-ids-with-self objects id))]
(reduce (fn [a sid] (assoc a sid t)) acc subtree-ids)))
{}
geometry-entries)
;; Context lost / mid-reload: do not call into WASM. Use
;; root transforms (and splat translation onto descendants
;; when we can) so the commit still lands in file data.
(not (wasm.api/initialized?))
(if translation?
(reduce
(fn [acc [id data]]
(let [t (:transform data)
@ -828,27 +859,12 @@
(reduce (fn [a sid] (assoc a sid t)) acc subtree-ids)))
{}
geometry-entries)
(into {}
(map (fn [[id data]] [id (:transform data)]))
geometry-entries))
;; Context lost / mid-reload: do not call into WASM. Use
;; root transforms (and splat translation onto descendants
;; when we can) so the commit still lands in file data.
(not (wasm.api/initialized?))
(if translation?
(reduce
(fn [acc [id data]]
(let [t (:transform data)
subtree-ids
(or (get subtree-ids-by-id id)
(cfh/get-children-ids-with-self objects id))]
(reduce (fn [a sid] (assoc a sid t)) acc subtree-ids)))
{}
geometry-entries)
(into {}
(map (fn [[id data]] [id (:transform data)]))
geometry-entries))
:else
(into {} (wasm.api/propagate-modifiers geometry-entries snap-pixel?)))
:else
(into {} (wasm.api/propagate-modifiers geometry-entries snap-pixel?))
ignore-tree
(calculate-ignore-tree-wasm transforms objects)
@ -885,29 +901,29 @@
(map :id))
ids)
undo-id (js/Symbol)]
(rx/concat
(if undo-transation?
(rx/of (dwu/start-undo-transaction undo-id))
(rx/empty))
(rx/of
(clear-local-transform)
(ptk/event ::dwg/move-frame-guides {:ids ids :transforms transforms})
(ptk/event ::dwcm/move-frame-comment-threads transforms)
(dwsh/update-shapes ids update-shape options)
undo-id (js/Symbol))
(rx/concat
(if undo-transation?
(rx/of (dwu/start-undo-transaction undo-id))
(rx/empty))
(rx/of
(clear-local-transform)
(ptk/event ::dwg/move-frame-guides {:ids ids :transforms transforms})
(ptk/event ::dwcm/move-frame-comment-threads transforms)
(dwsh/update-shapes ids update-shape options)
;; The update to the bool path needs to be in a different operation because it
;; needs to have the updated children info.
;; `update-layout? false`: recalculating a bool path can never change
;; `:hidden`, and the layout check would recompute the whole boolean
;; path in WASM once per bool shape just to find that out.
(dwsh/update-shapes bool-ids path/update-bool-shape (assoc options
:with-objects? true
:update-layout? false)))
;; The update to the bool path needs to be in a different operation because it
;; needs to have the updated children info.
;; `update-layout? false`: recalculating a bool path can never change
;; `:hidden`, and the layout check would recompute the whole boolean
;; path in WASM once per bool shape just to find that out.
(dwsh/update-shapes bool-ids path/update-bool-shape (assoc options
:with-objects? true
:update-layout? false)))
(if undo-transation?
(rx/of (dwu/commit-undo-transaction undo-id))
(rx/empty))))))))
(if undo-transation?
(rx/of (dwu/commit-undo-transaction undo-id))
(rx/empty)))))))))
(def ^:private
xf-rotation-shape

View File

@ -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))
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}))))))
container (get objects (:parent-id shape))]
(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}))))))))
(defn add-new-variant
"Create a new variant and add it to the variant-container"
@ -359,39 +360,40 @@
shape (get objects shape-id)
shape (if (ctc/is-variant-container? shape)
(get objects (last (:shapes shape)))
shape)
component-id (:component-id shape)
component (ctkl/get-component data component-id)
shape)]
(when (some? shape)
(let [component-id (:component-id shape)
component (ctkl/get-component data component-id)
container-id (:parent-id shape)
variant-container (get objects container-id)
has-layout? (ctsl/any-layout? variant-container)
container-id (:parent-id shape)
variant-container (get objects container-id)
has-layout? (ctsl/any-layout? variant-container)
new-component-id (uuid/next)
new-shape-id (uuid/next)
new-component-id (uuid/next)
new-shape-id (uuid/next)
prop-num (dec (count (:variant-properties component)))
prop-num (dec (count (:variant-properties component)))
changes (-> (pcb/empty-changes it page-id)
(pcb/with-library-data data)
(pcb/with-objects objects)
(pcb/with-page-id page-id)
(clv/generate-add-new-variant shape (:variant-id component) new-component-id new-shape-id prop-num))
changes (-> (pcb/empty-changes it page-id)
(pcb/with-library-data data)
(pcb/with-objects objects)
(pcb/with-page-id page-id)
(clv/generate-add-new-variant shape (:variant-id component) new-component-id new-shape-id prop-num))
undo-id (js/Symbol)]
(rx/concat
(rx/of
(dwu/start-undo-transaction undo-id)
(dch/commit-changes changes)
(when-not has-layout?
(resposition-and-resize-variant new-shape-id))
(dwu/commit-undo-transaction undo-id)
(ptk/data-event :layout/update {:ids [(:parent-id shape)]})
(if multiselect?
(dws/shift-select-shapes new-shape-id)
(dws/select-shape new-shape-id)))
(->> (rx/of (focus-property (:id variant-container)))
(rx/delay 250))))))))
undo-id (js/Symbol)]
(rx/concat
(rx/of
(dwu/start-undo-transaction undo-id)
(dch/commit-changes changes)
(when-not has-layout?
(resposition-and-resize-variant new-shape-id))
(dwu/commit-undo-transaction undo-id)
(ptk/data-event :layout/update {:ids [(:parent-id shape)]})
(if multiselect?
(dws/shift-select-shapes new-shape-id)
(dws/select-shape new-shape-id)))
(->> (rx/of (focus-property (:id variant-container)))
(rx/delay 250))))))))))
(defn transform-in-variant
"Given the id of a main shape of a component, creates a variant structure for

View File

@ -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)))))))

View File

@ -34,6 +34,7 @@
[frontend-tests.logic.groups-test]
[frontend-tests.logic.nudge-selected-shapes-test]
[frontend-tests.logic.pasting-in-containers-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]
@ -126,6 +127,7 @@
'frontend-tests.logic.nudge-selected-shapes-test
'frontend-tests.logic.pasting-in-containers-test
'frontend-tests.main-errors-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