🐛 Fix bad charcters allowed on variants properties

This commit is contained in:
Pablo Alba 2026-10-27 21:16:58 +01:00
parent 506923e2e9
commit cf59aabe6e
7 changed files with 367 additions and 70 deletions

View File

@ -16,22 +16,24 @@
(defn generate-update-property-name
[changes variant-id pos new-name]
(let [data (pcb/get-library-data changes)
objects (pcb/get-objects changes)
related-components (cfv/find-variant-components data objects variant-id)
(if-let [new-name (ctv/prepare-property-name new-name)]
(let [data (pcb/get-library-data changes)
objects (pcb/get-objects changes)
related-components (cfv/find-variant-components data objects variant-id)
props (-> related-components last :variant-properties)
prop-names (mapv :name props)
prop-names (concat (subvec prop-names 0 pos) (subvec prop-names (inc pos)))
new-name (ctv/update-number-in-repeated-item prop-names new-name)]
(reduce (fn [changes component]
(pcb/update-component
changes (:id component)
(fn [component]
(d/update-in-when component [:variant-properties pos] #(assoc % :name new-name)))
{:apply-changes-local-library? true}))
changes
related-components)))
props (-> related-components last :variant-properties)
prop-names (mapv :name props)
prop-names (concat (subvec prop-names 0 pos) (subvec prop-names (inc pos)))
new-name (ctv/update-number-in-repeated-item prop-names new-name)]
(reduce (fn [changes component]
(pcb/update-component
changes (:id component)
(fn [component]
(d/update-in-when component [:variant-properties pos] #(assoc % :name new-name)))
{:apply-changes-local-library? true}))
changes
related-components))
changes))
(defn generate-remove-property
@ -57,16 +59,18 @@
(defn generate-update-property-value
[changes component-id pos value]
(let [data (pcb/get-library-data changes)
component (ctcl/get-component data component-id true)
main-id (:main-instance-id component)
name (-> (:variant-properties component)
(update pos assoc :value value)
ctv/properties-to-name)]
(-> changes
(pcb/update-component component-id #(assoc-in % [:variant-properties pos :value] value)
{:apply-changes-local-library? true})
(pcb/update-shapes [main-id] #(assoc % :variant-name name)))))
(if-let [value (ctv/prepare-property-value value)]
(let [data (pcb/get-library-data changes)
component (ctcl/get-component data component-id true)
main-id (:main-instance-id component)
name (-> (:variant-properties component)
(update pos assoc :value value)
ctv/properties-to-name)]
(-> changes
(pcb/update-component component-id #(assoc-in % [:variant-properties pos :value] value)
{:apply-changes-local-library? true})
(pcb/update-shapes [main-id] #(assoc % :variant-name name))))
changes))
(defn generate-set-variant-error
@ -108,40 +112,46 @@
props (-> related-components last :variant-properties)
next-prop-num (ctv/next-property-number props)
property-name (or property-name (str ctv/property-prefix next-prop-num))
custom-name? (some? property-name)
property-name (if custom-name?
(ctv/prepare-property-name property-name)
(str ctv/property-prefix next-prop-num))]
(if (and custom-name? (nil? property-name))
changes
(let [prop-names (mapv :name props)
property-name (ctv/update-number-in-repeated-item prop-names property-name)
property-value (not-empty
(when property-value
(ctv/prepare-property-value property-value)))
mdata (if editing? {:editing? true} nil)
prop-names (mapv :name props)
property-name (ctv/update-number-in-repeated-item prop-names property-name)
[_ changes]
(reduce (fn [[num changes] component]
(let [main-id (:main-instance-id component)
mdata (if editing? {:editing? true} nil)
update-props #(-> (d/nilv % [])
(conj (with-meta {:name property-name
:value (cond fill-values? (str ctv/value-prefix num)
property-value property-value
:else "")}
mdata)))
[_ changes]
(reduce (fn [[num changes] component]
(let [main-id (:main-instance-id component)
update-props #(-> (d/nilv % [])
(conj (with-meta {:name property-name
:value (cond fill-values? (str ctv/value-prefix num)
property-value property-value
:else "")}
mdata)))
update-name #(cond fill-values? (if (str/empty? %)
(str ctv/value-prefix num)
(str % ", " ctv/value-prefix num))
property-value (if (str/empty? %)
property-value
(str % ", " property-value))
:else %)]
[(inc num)
(-> changes
(pcb/update-component (:id component)
#(update % :variant-properties update-props)
{:apply-changes-local-library? true})
(pcb/update-shapes [main-id] #(update % :variant-name update-name)))]))
[1 changes]
related-components)]
changes))
update-name #(cond fill-values? (if (str/empty? %)
(str ctv/value-prefix num)
(str % ", " ctv/value-prefix num))
property-value (if (str/empty? %)
property-value
(str % ", " property-value))
:else %)]
[(inc num)
(-> changes
(pcb/update-component (:id component)
#(update % :variant-properties update-props)
{:apply-changes-local-library? true})
(pcb/update-shapes [main-id] #(update % :variant-name update-name)))]))
[1 changes]
related-components)]
changes))))
(defn- generate-make-shape-no-variant
[changes shape]
@ -175,7 +185,7 @@
props (ctv/add-new-props props (repeat new-props ""))]
(if add-name?
(ctv/add-new-prop props (:name component))
(ctv/add-new-prop props (or (ctv/sanitize-property-text (:name component)) ""))
props)))
(defn- create-new-properties-from-non-variant

View File

@ -55,19 +55,47 @@
[value]
(some-> value str/trim))
(def ^:private forbidden-property-chars-re #"[,=]")
(defn sanitize-property-text
"Remove `,` and `=` from property name/value text, then trim.
Non-strings (including nil) return nil — never call str/replace on them."
[value]
(when (string? value)
(-> value
(str/replace forbidden-property-chars-re "")
normalize-property-text)))
(defn valid-property-name?
[value]
(let [value (normalize-property-text value)]
(and (string? value)
(not (str/blank? value))
(nil? (re-find forbidden-property-chars-re value))
(<= (count value) property-max-length))))
(defn valid-property-value?
[value]
(let [value (normalize-property-text value)]
(and (string? value)
(nil? (re-find forbidden-property-chars-re value))
(<= (count value) property-max-length))))
(defn prepare-property-name
"Sanitize then validate a property name. Returns the cleaned string or nil."
[value]
(let [value (sanitize-property-text value)]
(when (valid-property-name? value)
value)))
(defn prepare-property-value
"Sanitize then validate a property value. nil becomes \"\".
Other non-strings yield nil. Empty string remains valid."
[value]
(let [value (sanitize-property-text (if (nil? value) "" value))]
(when (valid-property-value? value)
value)))
(defn can-remove-property?
[properties]
(> (count properties) 1))
@ -112,7 +140,8 @@
([path properties]
(path-to-properties path properties 0))
([path properties min-props]
(let [cpath (cpn/split-path path)
(let [cpath (->> (cpn/split-path path)
(mapv #(or (sanitize-property-text %) "")))
total-props (max (count cpath) min-props)
assigned (mapv #(assoc % :value (nth cpath %2 "")) properties (range))
;; Add empty strings to the end of cpath to reach the minimum number of properties

View File

@ -16,7 +16,9 @@
[app.common.test-helpers.ids-map :as thi]
[app.common.test-helpers.shapes :as ths]
[app.common.test-helpers.variants :as thv]
[clojure.test :as t]))
[app.common.types.variant :as ctv]
[clojure.test :as t]
[cuerdas.core :as str]))
(t/use-fixtures :each thi/test-fixture)
@ -234,6 +236,176 @@
(t/is (= (-> comp01' :variant-properties first :value) "NewValue1"))
(t/is (= (-> comp02' :variant-properties first :value) "NewValue2"))))
(t/deftest test-update-property-value-sanitizes
(let [file (-> (thf/sample-file :file1)
(thv/add-variant :v01 :c01 :m01 :c02 :m02))
page (thf/current-page file)
comp01 (thc/get-component file :c01)
before (-> comp01 :variant-properties first :value)
changes (-> (pcb/empty-changes nil)
(pcb/with-page-id (:id page))
(pcb/with-library-data (:data file))
(pcb/with-objects (:objects page))
(clvp/generate-update-property-value (:id comp01) 0 "a=b,"))
file' (thf/apply-changes file changes)
comp01' (thc/get-component file' :c01)
main' (ths/get-shape file' :m01)
noop (-> (pcb/empty-changes nil)
(pcb/with-page-id (:id page))
(pcb/with-library-data (:data file))
(pcb/with-objects (:objects page))
(clvp/generate-update-property-value (:id comp01) 0 42))
file-noop (thf/apply-changes file noop)
comp-noop (thc/get-component file-noop :c01)
cleared (-> (pcb/empty-changes nil)
(pcb/with-page-id (:id page))
(pcb/with-library-data (:data file))
(pcb/with-objects (:objects page))
(clvp/generate-update-property-value (:id comp01) 0 "=,="))
file-cleared (thf/apply-changes file cleared)
comp-cleared (thc/get-component file-cleared :c01)
main-cleared (ths/get-shape file-cleared :m01)
nilled (-> (pcb/empty-changes nil)
(pcb/with-page-id (:id page))
(pcb/with-library-data (:data file))
(pcb/with-objects (:objects page))
(clvp/generate-update-property-value (:id comp01) 0 nil))
file-nilled (thf/apply-changes file nilled)
comp-nilled (thc/get-component file-nilled :c01)]
(t/is (= "ab" (-> comp01' :variant-properties first :value)))
(t/is (= "ab" (:variant-name main')))
(t/is (= before (-> comp-noop :variant-properties first :value)))
(t/is (= "" (-> comp-cleared :variant-properties first :value)))
(t/is (not (str/includes? (str (:variant-name main-cleared)) "=")))
(t/is (= "" (-> comp-nilled :variant-properties first :value)))))
(t/deftest test-update-property-name-sanitizes-and-noops
(let [file (-> (thf/sample-file :file1)
(thv/add-variant-two-properties :v01 :c01 :m01 :c02 :m02))
v-id (-> (ths/get-shape file :v01) :id)
page (thf/current-page file)
before (-> (thc/get-component file :c01) :variant-properties first :name)
changes (-> (pcb/empty-changes nil)
(pcb/with-page-id (:id page))
(pcb/with-library-data (:data file))
(pcb/with-objects (:objects page))
(clvp/generate-update-property-name v-id 0 "Size=Big"))
file' (thf/apply-changes file changes)
noop (-> (pcb/empty-changes nil)
(pcb/with-page-id (:id page))
(pcb/with-library-data (:data file))
(pcb/with-objects (:objects page))
(clvp/generate-update-property-name v-id 0 "=,="))
file-noop (thf/apply-changes file noop)]
(t/is (= "SizeBig" (-> (thc/get-component file' :c01) :variant-properties first :name)))
(t/is (= before (-> (thc/get-component file-noop :c01) :variant-properties first :name)))))
(t/deftest test-add-new-property-sanitizes
(let [file (-> (thf/sample-file :file1)
(thv/add-variant :v01 :c01 :m01 :c02 :m02))
v-id (-> (ths/get-shape file :v01) :id)
page (thf/current-page file)
before-count (-> (thc/get-component file :c01) :variant-properties count)
changes (-> (pcb/empty-changes nil)
(pcb/with-page-id (:id page))
(pcb/with-library-data (:data file))
(pcb/with-objects (:objects page))
(clvp/generate-add-new-property v-id {:property-name "Size=Big"
:property-value "a=b,"}))
file' (thf/apply-changes file changes)
comp01' (thc/get-component file' :c01)
noop (-> (pcb/empty-changes nil)
(pcb/with-page-id (:id page))
(pcb/with-library-data (:data file))
(pcb/with-objects (:objects page))
(clvp/generate-add-new-property v-id {:property-name "=,="}))
file-noop (thf/apply-changes file noop)]
(t/is (= "SizeBig" (-> comp01' :variant-properties last :name)))
(t/is (= "ab" (-> comp01' :variant-properties last :value)))
(t/is (= before-count
(-> (thc/get-component file-noop :c01) :variant-properties count)))))
(t/deftest test-add-new-property-blank-value-does-not-trail-separator
(let [file (-> (thf/sample-file :file1)
(thv/add-variant :v01 :c01 :m01 :c02 :m02))
v-id (-> (ths/get-shape file :v01) :id)
page (thf/current-page file)
main (ths/get-shape file :m01)
before (:variant-name main)
changes (-> (pcb/empty-changes nil)
(pcb/with-page-id (:id page))
(pcb/with-library-data (:data file))
(pcb/with-objects (:objects page))
(clvp/generate-add-new-property v-id {:property-name "Size"
:property-value "=,="}))
file' (thf/apply-changes file changes)
comp01' (thc/get-component file' :c01)
main' (ths/get-shape file' :m01)]
(t/is (= before (:variant-name main')))
(t/is (not (str/ends-with? (str (:variant-name main')) ", ")))
(t/is (= "" (-> comp01' :variant-properties last :value)))
(t/is (= "Size" (-> comp01' :variant-properties last :name)))))
(t/deftest test-update-property-value-from-formula-strips-equal-in-value
;; Option A / M2: formula parse may leave `=` inside a value; write path strips it.
(let [file (-> (thf/sample-file :file1)
(thv/add-variant :v01 :c01 :m01 :c02 :m02))
page (thf/current-page file)
comp01 (thc/get-component file :c01)
parsed (ctv/properties-formula->map "border=yes color=yes")
value (-> parsed first :value)
changes (-> (pcb/empty-changes nil)
(pcb/with-page-id (:id page))
(pcb/with-library-data (:data file))
(pcb/with-objects (:objects page))
(clvp/generate-update-property-value (:id comp01) 0 value))
file' (thf/apply-changes file changes)
comp01' (thc/get-component file' :c01)
main' (ths/get-shape file' :m01)]
(t/is (= "yes color=yes" value))
(t/is (= "yes coloryes" (-> comp01' :variant-properties first :value)))
(t/is (= "yes coloryes" (:variant-name main')))
(t/is (not= "yes color=yes" (-> comp01' :variant-properties first :value)))))
(t/deftest test-make-shapes-variant-sanitizes-component-name-as-value
;; create-new-properties-from-variant adds (:name component) as a value when
;; moving a variant into a differently named container.
(let [file (-> (thf/sample-file :file1)
(thv/add-variant :v01 :c01 :m01 :c02 :m02)
(thv/add-variant :v02 :c03 :m03 :c04 :m04)
(thc/update-component :c03 {:name "Big=red"}))
page (thf/current-page file)
container (ths/get-shape file :v01)
shape (ths/get-shape file :m03)
changes (-> (pcb/empty-changes nil)
(pcb/with-page-id (:id page))
(pcb/with-library-data (:data file))
(pcb/with-objects (:objects page))
(clvp/generate-make-shapes-variant [shape] container))
file' (thf/apply-changes file changes)
values (->> (thc/get-component file' :c03)
:variant-properties
(map :value)
set)]
(t/is (contains? values "Bigred"))))
(t/deftest test-duplicate-variant-container
(let [;; ==== Setup

View File

@ -171,3 +171,45 @@
(t/is (= (ctv/find-boolean-pair ["false" "true"]) {"true" true "false" false}))
(t/is (= (ctv/find-boolean-pair ["off" "on" "other"]) nil))
(t/is (= (ctv/find-boolean-pair ["hello" "bye"]) nil))))
(t/deftest sanitize-and-prepare-property-text
(t/testing "sanitize-property-text"
(t/is (= (ctv/sanitize-property-text "a=b") "ab"))
(t/is (= (ctv/sanitize-property-text "a,b") "ab"))
(t/is (= (ctv/sanitize-property-text " Big, red ") "Big red"))
(t/is (= (ctv/sanitize-property-text " a=b, ") "ab"))
(t/is (= (ctv/sanitize-property-text ",=") ""))
(t/is (nil? (ctv/sanitize-property-text nil)))
(t/is (nil? (ctv/sanitize-property-text 42))))
(t/testing "valid-property-name? rejects forbidden chars"
(t/is (true? (ctv/valid-property-name? "ok")))
(t/is (false? (ctv/valid-property-name? "a=b")))
(t/is (false? (ctv/valid-property-name? "a,b")))
(t/is (false? (ctv/valid-property-name? ""))))
(t/testing "valid-property-value? rejects forbidden chars"
(t/is (true? (ctv/valid-property-value? "ok")))
(t/is (true? (ctv/valid-property-value? "")))
(t/is (false? (ctv/valid-property-value? "a=b")))
(t/is (false? (ctv/valid-property-value? "a,b"))))
(t/testing "prepare-property-name"
(t/is (= (ctv/prepare-property-name "Size=Big") "SizeBig"))
(t/is (nil? (ctv/prepare-property-name "=,=")))
(t/is (= (ctv/prepare-property-name " ok ") "ok"))
(t/is (nil? (ctv/prepare-property-name 42))))
(t/testing "prepare-property-value"
(t/is (= (ctv/prepare-property-value "a=b,") "ab"))
(t/is (= (ctv/prepare-property-value "=,=") ""))
(t/is (= (ctv/prepare-property-value nil) ""))
(t/is (nil? (ctv/prepare-property-value 42)))))
(t/deftest path-to-properties-sanitizes-values
(let [props [{:name "Property 1" :value ""}]
result (ctv/path-to-properties "Button / Big, red" props)]
(t/is (= "Button" (-> result first :value)))
(t/is (= "Big red" (-> result second :value)))))

View File

@ -389,7 +389,7 @@
(mf/use-fn
(mf/deps component-ids)
(fn [pos value]
(let [value (ctv/normalize-property-text (d/nilv value ""))]
(when-let [value (ctv/prepare-property-value value)]
(doseq [id component-ids]
(st/emit!
(ev/event {::ev/name "variant-edit-property-value" ::ev/origin "workspace:combo-design-tab"})
@ -400,11 +400,11 @@
(mf/use-fn
(mf/deps variant-id)
(fn [event]
(let [value (ctv/normalize-property-text (dom/get-target-val event))
(let [value (ctv/prepare-property-name (dom/get-target-val event))
pos (-> (dom/get-current-target event)
(dom/get-data "position")
int)]
(when (ctv/valid-property-name? value)
(when value
(st/emit!
(dwv/update-property-name variant-id pos value {:trigger "workspace:design-tab-variant"}))))))
@ -1230,11 +1230,11 @@
(mf/use-fn
(mf/deps variant-id)
(fn [event]
(let [value (ctv/normalize-property-text (dom/get-target-val event))
(let [value (ctv/prepare-property-name (dom/get-target-val event))
pos (-> (dom/get-current-target event)
(dom/get-data "position")
int)]
(when (ctv/valid-property-name? value)
(when value
(st/emit!
(dwv/update-property-name variant-id pos value {:trigger "workspace:design-tab-component"}))))))

View File

@ -779,12 +779,13 @@
:renameProperty
(fn [pos name]
(let [nprops (->> (get-variant-components file-id id) first :variant-properties count)]
(let [nprops (->> (get-variant-components file-id id) first :variant-properties count)
prepared (ctv/prepare-property-name name)]
(cond
(or (not (nat-int? pos)) (>= pos nprops))
(u/not-valid plugin-id :pos pos)
(not (ctv/valid-property-name? name))
(nil? prepared)
(u/not-valid plugin-id :name name)
(not (r/check-permission plugin-id "library:write"))
@ -792,7 +793,7 @@
:else
(st/emit!
(dwv/update-property-name id pos (ctv/normalize-property-text name)
(dwv/update-property-name id pos prepared
{:trigger "plugin:rename-property"})))))))
(set! shape/variant-proxy variant-proxy)
@ -1090,12 +1091,13 @@
:setVariantProperty
(fn [pos value]
(let [nprops (-> (u/locate-library-component file-id id) :variant-properties count)]
(let [nprops (-> (u/locate-library-component file-id id) :variant-properties count)
prepared (ctv/prepare-property-value value)]
(cond
(or (not (nat-int? pos)) (>= pos nprops))
(u/not-valid plugin-id :pos (str pos))
(not (ctv/valid-property-value? value))
(nil? prepared)
(u/not-valid plugin-id :value value)
(not (r/check-permission plugin-id "library:write"))
@ -1104,7 +1106,7 @@
:else
(st/emit!
(se/event plugin-id "variant-edit-property-value")
(dwv/update-property-value id pos (ctv/normalize-property-text value))))))))
(dwv/update-property-value id pos prepared)))))))
(defn library-proxy? [p]
(obj/type-of? p "LibraryProxy"))

View File

@ -388,6 +388,48 @@
(t/is (= [plugin-id :setVariantProperty "Plugin doesn't have 'library:write' permission"]
(first @errors)))))))
(t/deftest variant-rename-property-sanitizes-name
(let [plugin-id "test-plugin"
file-id (uuid/next)
id (uuid/next)
renamed (atom nil)
errors (atom [])]
(with-redefs [r/check-permission (constantly true)
u/not-valid (mock/stub (fn [pid prop msg] (swap! errors conj [pid prop msg])))
library/get-variant-components (constantly [{:variant-properties [{:name "color" :value "red"}]}])
dwv/update-property-name (fn [vid pos name opts]
(reset! renamed {:id vid :pos pos :name name :opts opts})
::rename-event)
st/emit! mock/noop]
(let [proxy (library/variant-proxy plugin-id file-id id)]
(.renameProperty proxy 0 "a=b")
(t/is (empty? @errors))
(t/is (= "ab" (:name @renamed)))
(.renameProperty proxy 0 "=,=")
(t/is (= 1 (count @errors)))
(t/is (= :name (second (first @errors))))))))
(t/deftest component-set-variant-property-sanitizes-value
(let [plugin-id "test-plugin"
file-id (uuid/next)
id (uuid/next)
updated (atom nil)
errors (atom [])]
(with-redefs [r/check-permission (constantly true)
u/not-valid (mock/stub (fn [pid prop msg] (swap! errors conj [pid prop msg])))
u/locate-library-component (constantly {:id id :variant-properties [{:name "color"}]})
dwv/update-property-value (fn [cid pos value]
(reset! updated {:id cid :pos pos :value value})
::value-event)
st/emit! mock/noop]
(let [proxy (library/lib-component-proxy plugin-id file-id id)]
(.setVariantProperty proxy 0 "a=b")
(t/is (empty? @errors))
(t/is (= "ab" (:value @updated)))
(.setVariantProperty proxy 0 "=,=")
(t/is (empty? @errors))
(t/is (= "" (:value @updated)))))))
;; ---------------------------------------------------------------------------
;; connectLibrary
;; ---------------------------------------------------------------------------