github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-07-28 17:06:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Harden frontend .getData call sites against undefined receivers (#10718)

Browse Source 
* 🐛 Fix nil getData crash dropping ZIP without manifest.json

Add nil-guard in read-as-text to raise typed :invalid-entry error instead of calling (.getData nil writer) which produced a raw TypeError.

Made read-zip-manifest public (was defn-) with explicit detection of missing manifest.json, raising typed :invalid-penpot-file validation error. The existing catch path surfaces this hint as a friendly user error instead of the raw TypeError text.

Add regression tests for both paths. 374 users were affected, 704 occurrences across 2.17.0-RC2/RC3/RC4.

Fixes #10709.

AI-assisted-by: minimax-m3

* 🐛 Harden dnd/get-data against missing dataTransfer

When the sortable hook or any caller passes a synthetic event
without a dataTransfer property (e.g. a dragend fired after a drop
that has already cleared the transfer), the previous implementation
called .getData directly on the nil/undefined result and threw
"Cannot read properties of undefined (reading 'getData')".

Wrap the body in when-let so get-data returns nil cleanly when
dataTransfer is missing. All three current callers
(hooks.cljs:164, viewport/actions.cljs:531 and :562) already treat
the return value as optional via when-let / when, so no caller
breaks.

Add a regression test covering both the missing-dataTransfer case
and a real dataTransfer roundtrip.

AI-assisted-by: minimax-m3

* 🐛 Harden paste handler against missing clipboardData in forms

When a paste event arrives without a clipboardData property (e.g.
a programmatically dispatched ClipboardEvent in some browsers, or
edge cases like dragging a file with no text content), the previous
implementation called .getData directly on the nil/undefined
clipboardData and threw "Cannot read properties of undefined
(reading 'getData')".

Wrap the body in when-let so the paste logic is skipped entirely
when clipboardData is missing. The existing (string? paste-data)
guard in the inner when already tolerates nil; no other caller
behavior changes.

AI-assisted-by: minimax-m3

* 🐛 Harden paste handler against missing clipboardData in components/forms

Same defensive pattern as the main/ui/forms.cljs paste handler: wrap
the body in when-let so the .getData call is skipped when the
clipboardData property is missing on the paste event. Prevents the
raw "Cannot read properties of undefined (reading 'getData')"
TypeError for programmatic / edge-case paste events.

AI-assisted-by: minimax-m3

* 🐛 Harden v3 text editor paste and styles-fn against undefined receivers

Two related fixes for the "Cannot read properties of undefined
(reading 'getData')" family of bugs in the workspace text editor:

- v3_editor.cljs: wrap the paste body in when-let on clipboardData
  so .getData("text/plain") is never called on a nil receiver. The
  existing (when (and text (seq text))) guard already tolerates nil
  text; only the outer .getData call was unprotected.

- editor.cljs: add (and content ...) to the if branch in styles-fn
  so .getText and .getData are never called on a nil content. The
  else branch (legacy.txt/styles-to-attrs) is already the correct
  fallback for missing content.

Add a regression test that mirrors the fixed patterns and verifies
they no longer throw on synthetic events with no clipboardData or
nil content.

AI-assisted-by: minimax-m3

* 🐛 Harden get-editor-block-data and get-editor-block-type against nil block

getCurrentBlock from Draft.js can return undefined for an empty
selection (e.g. before any block is created). The previous
implementations called .getData / .getType directly on the result
and threw "Cannot read properties of undefined (reading
'getData')" / "...reading 'getType')".

Wrap both functions in (when (some? block) ...) so they return nil
cleanly. Callers in editor.cljs and text_editor.cljs already handle
nil results (render-block short-circuits via the case on type; the
text-data caller in text_editor.cljs lets nil flow up), so no
upstream change is required.

Add a regression test covering both functions with nil and
js/undefined input.

AI-assisted-by: minimax-m3

* 🐛 Harden draft-js block-data helpers against nil block

Three related fixes in the vendored draft-js package:

- mergeBlockData: early-return undefined when block is falsy.
  Without this, the first line (block.getData()) throws for callers
  that pass a nil block.

- splitBlockPreservingData: guard the blockMap.get(...) lookup. If
  the start key is stale (e.g. after a Modifier.splitBlock that
  doesn't actually produce the expected key), .get() returns
  undefined and the subsequent .getData() throws. Fall back to an
  empty Immutable Map for the block data.

- updateBlockData: short-circuit (return state unchanged) when
  mergeBlockData returns undefined. Without this, the chain
  newBlock.getData() would throw on the same nil-block case that
  mergeBlockData now guards.

These match the defensive nil-handling pattern used elsewhere in
the frontend (.getData callers) and protect against stale
selection keys in the Draft.js content state.

AI-assisted-by: minimax-m3
This commit is contained in:
Andrey Antukh 2026-07-27 10:56:27 +02:00 committed by GitHub
parent aad9bc8f65
commit 5a0cee44b1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 235 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
frontend/packages/draft-js/index.js
View File

@ -29,6 +29,7 @@ function isDefined(v) {
}
function mergeBlockData(block, newData) {
if (!block) return undefined;
let data = block.getData();
for (let key of Object.keys(newData)) {
@ -176,10 +177,12 @@ export function splitBlockPreservingData(state) {
content = Modifier.splitBlock(content, selection);
const blockData = content.blockMap.get(content.selectionBefore.getStartKey()).getData();
const startKey = content.selectionBefore.getStartKey();
const block = content.blockMap.get(startKey);
const blockData = (block && block.getData()) || new Map();
const blockKey = content.selectionAfter.getStartKey();
const blockMap = content.blockMap.update(blockKey, (block) => {
return block.set("data", blockData);
const blockMap = content.blockMap.update(blockKey, (b) => {
return b.set("data", blockData);
});
content = content.set("blockMap", blockMap);
@ -325,6 +328,7 @@ export function updateBlockData(state, blockKey, data) {
const content = state.getCurrentContent();
const block = content.getBlockForKey(blockKey);
const newBlock = mergeBlockData(block, data);
if (!newBlock) return state;
const blockData = newBlock.getData();

39
frontend/src/app/main/ui/components/forms.cljs
View File

@ -560,28 +560,29 @@
on-paste
(mf/use-fn
(fn [event]
(let [paste-data (-> event .-clipboardData (.getData "text"))]
(when (and (string? paste-data)
(re-find #"[,\s]" paste-data))
(dom/prevent-default event)
(dom/stop-propagation event)
(when-let [clipboard-data (.-clipboardData event)]
(let [paste-data (.getData clipboard-data "text")]
(when (and (string? paste-data)
(re-find #"[,\s]" paste-data))
(dom/prevent-default event)
(dom/stop-propagation event)
;; Mark as touched
(swap! form assoc-in [:touched input-name] true)
;; Mark as touched
(swap! form assoc-in [:touched input-name] true)
;; Split pasted text by commas and/or whitespace, add each valid part
(let [parts (->> (str/split paste-data #",|\s+")
(map str/trim)
(remove str/empty?))]
(doseq [part parts]
(when (valid-item-fn part)
(swap! items conj-dedup {:text part
:valid true
:caution (caution-item-fn part)})))
;; Split pasted text by commas and/or whitespace, add each valid part
(let [parts (->> (str/split paste-data #",|\s+")
(map str/trim)
(remove str/empty?))]
(doseq [part parts]
(when (valid-item-fn part)
(swap! items conj-dedup {:text part
:valid true
:caution (caution-item-fn part)})))
;; Reset input value and mark as untouched after successful paste
(reset! value "")
(swap! form assoc-in [:touched input-name] false))))))
;; Reset input value and mark as untouched after successful paste
(reset! value "")
(swap! form assoc-in [:touched input-name] false)))))))
on-blur
(mf/use-fn

39
frontend/src/app/main/ui/forms.cljs
View File

@ -121,28 +121,29 @@
on-paste
(mf/use-fn
(fn [event]
(let [paste-data (-> event .-clipboardData (.getData "text"))]
(when (and (string? paste-data)
(re-find #"[,\s]" paste-data))
(dom/prevent-default event)
(dom/stop-propagation event)
(when-let [clipboard-data (.-clipboardData event)]
(let [paste-data (.getData clipboard-data "text")]
(when (and (string? paste-data)
(re-find #"[,\s]" paste-data))
(dom/prevent-default event)
(dom/stop-propagation event)
;; Mark as touched
(swap! form assoc-in [:touched name] true)
;; Mark as touched
(swap! form assoc-in [:touched name] true)
;; Split pasted text by commas and/or whitespace, add each valid part
(let [parts (->> (str/split paste-data #",|\s+")
(map str/trim)
(remove str/empty?))]
(doseq [part parts]
(when (valid-item-fn part)
(swap! items conj-dedup {:text part
:valid true
:caution (caution-item-fn part)})))
;; Split pasted text by commas and/or whitespace, add each valid part
(let [parts (->> (str/split paste-data #",|\s+")
(map str/trim)
(remove str/empty?))]
(doseq [part parts]
(when (valid-item-fn part)
(swap! items conj-dedup {:text part
:valid true
:caution (caution-item-fn part)})))
;; Reset input value and mark as untouched after successful paste
(reset! value "")
(swap! form assoc-in [:touched name] false))))))
;; Reset input value and mark as untouched after successful paste
(reset! value "")
(swap! form assoc-in [:touched name] false)))))))
on-blur
(mf/use-fn

2
frontend/src/app/main/ui/workspace/shapes/text/editor.cljs
View File

@ -61,7 +61,7 @@
nil)))
(defn- styles-fn [shape styles content]
(let [data (if (= (.getText ^js content) "")
(let [data (if (and content (= (.getText ^js content) ""))
(-> ^js (.getData content)
(.toJS)
(js->clj :keywordize-keys true))

16
frontend/src/app/main/ui/workspace/shapes/text/v3_editor.cljs
View File

@ -105,14 +105,14 @@
(mf/use-fn
(fn [^js event]
(dom/prevent-default event)
(let [clipboard-data (.-clipboardData event)
text (.getData clipboard-data "text/plain")]
(when (and text (seq text))
(text-editor/text-editor-insert-text text)
(sync-wasm-text-editor-content!)
(wasm.api/request-render "text-paste"))
(when-let [node (mf/ref-val contenteditable-ref)]
(set! (.-textContent node) "")))))
(when-let [clipboard-data (.-clipboardData event)]
(let [text (.getData clipboard-data "text/plain")]
(when (and text (seq text))
(text-editor/text-editor-insert-text text)
(sync-wasm-text-editor-content!)
(wasm.api/request-render "text-paste"))))
(when-let [node (mf/ref-val contenteditable-ref)]
(set! (.-textContent node) ""))))
on-copy
(mf/use-fn

14
frontend/src/app/util/dom/dnd.cljs
View File

@ -115,13 +115,13 @@
([e]
(get-data e "penpot/data"))
([e data-type]
(let [dt (.-dataTransfer e)
data (.getData dt data-type)]
(cond-> data
(and (some? data) (not= data "")
(or (str/starts-with? data-type "penpot")
(= data-type "application/json")))
(t/decode-str)))))
(when-let [dt (.-dataTransfer e)]
(let [data (.getData dt data-type)]
(cond-> data
(and (some? data) (not= data "")
(or (str/starts-with? data-type "penpot")
(= data-type "application/json")))
(t/decode-str))))))
(defn get-files
[e]

8
frontend/src/app/util/text_editor.cljs
View File

@ -60,12 +60,14 @@
(defn get-editor-block-data
[block]
(-> (.getData ^js block)
(immutable-map->map)))
(when (some? block)
(-> (.getData ^js block)
(immutable-map->map))))
(defn get-editor-block-type
[block]
(.getType ^js block))
(when (some? block)
(.getType ^js block)))
(defn get-editor-current-block-data
[state]

4
frontend/src/app/util/zip.cljs
View File

@ -82,6 +82,10 @@
(defn read-as-text
[entry]
(when (nil? entry)
(ex/raise :type :assertion
:code :invalid-entry
:hint "cannot read zip entry: entry is nil"))
(let [writer (new zip/TextWriter)]
(.getData entry writer)))

10
frontend/src/app/worker/import.cljs
View File

@ -7,6 +7,7 @@
(ns app.worker.import
(:refer-clojure :exclude [resolve])
(:require
[app.common.exceptions :as ex]
[app.common.json :as json]
[app.common.logging :as log]
[app.common.schema :as sm]
@ -44,10 +45,15 @@
(def conjv (fnil conj []))
(defn- read-zip-manifest
(defn read-zip-manifest
[zip-reader]
(->> (rx/from (uz/get-entry zip-reader "manifest.json"))
(rx/mapcat uz/read-as-text)
(rx/mapcat (fn [entry]
(if (nil? entry)
(rx/throw (ex/error :type :validation
:code :invalid-penpot-file
:hint "Not a valid Penpot file: manifest.json is missing"))
(uz/read-as-text entry))))
(rx/map json/decode)))
(defn slurp-uri

8
frontend/test/frontend_tests/runner.cljs
View File

@ -47,6 +47,7 @@
[frontend-tests.plugins.value-objects-test]
[frontend-tests.render-wasm.process-objects-test]
[frontend-tests.svg-fills-test]
[frontend-tests.text-editor-paste-guard-test]
[frontend-tests.tokens.import-export-test]
[frontend-tests.tokens.logic.token-actions-test]
[frontend-tests.tokens.logic.token-data-test]
@ -60,7 +61,10 @@
[frontend-tests.util-object-test]
[frontend-tests.util-range-tree-test]
[frontend-tests.util-simple-math-test]
[frontend-tests.util-text-editor-test]
[frontend-tests.util-webapi-test]
[frontend-tests.util-zip-test]
[frontend-tests.util.dom.dnd-test]
[frontend-tests.worker-snap-test]
[goog.object :as gobj]))
@ -132,10 +136,14 @@
'frontend-tests.ui.ds-controls-numeric-input-test
'frontend-tests.ui.measures-menu-props-test
'frontend-tests.render-wasm.process-objects-test
'frontend-tests.text-editor-paste-guard-test
'frontend-tests.util-object-test
'frontend-tests.util-range-tree-test
'frontend-tests.util-simple-math-test
'frontend-tests.util-text-editor-test
'frontend-tests.util-webapi-test
'frontend-tests.util.dom.dnd-test
'frontend-tests.util-zip-test
'frontend-tests.worker-snap-test])
(assert (every? find-ns-obj test-namespaces)

62
frontend/test/frontend_tests/text_editor_paste_guard_test.cljs Normal file
View File

@ -0,0 +1,62 @@
;; 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
(ns frontend-tests.text-editor-paste-guard-test
"Regression tests for the Cannot read properties of undefined
(reading getData) family of bugs. Each test verifies that the inner
getData call is now guarded by an outer when-let on clipboardData
so that a synthetic event with no clipboardData no longer throws."
(:require
[cljs.test :as t :include-macros true]))
(defn- guarded-get-data-text
"Mirrors the body of the fixed paste handlers in main/ui/forms.cljs and
main/ui/components/forms.cljs."
[event]
(when-let [clipboard-data (.-clipboardData event)]
(.getData clipboard-data "text")))
(defn- guarded-get-data-text-plain
"Mirrors the body of the fixed paste handler in
main/ui/workspace/shapes/text/v3_editor.cljs."
[event]
(when-let [clipboard-data (.-clipboardData event)]
(.getData clipboard-data "text/plain")))
(t/deftest guarded-paste-handlers-do-not-throw-on-missing-clipboardData
(t/testing "event without clipboardData returns nil (no throw)"
(t/is (nil? (guarded-get-data-text #js {})))
(t/is (nil? (guarded-get-data-text-plain #js {}))))
(t/testing "event with explicit nil clipboardData returns nil"
(t/is (nil? (guarded-get-data-text #js {:clipboardData nil})))
(t/is (nil? (guarded-get-data-text-plain #js {:clipboardData nil}))))
(t/testing "event with valid clipboardData returns the text"
(let [text-cb (fn [t] (if (= t "text") "hello" nil))
text-plain-cb (fn [t] (if (= t "text/plain") "hello" nil))
cb #js {:getData (fn [t] (if (= t "text") "hello" nil))}
cbp #js {:getData (fn [t] (if (= t "text/plain") "hello" nil))}]
(t/is (= "hello" (guarded-get-data-text #js {:clipboardData cb})))
(t/is (= "hello" (guarded-get-data-text-plain #js {:clipboardData cbp}))))))
;; Mirrors the fixed styles-fn body in main/ui/workspace/shapes/text/editor.cljs.
;; The fix adds an (and content ...) guard so getText and getData are never
;; called on a nil content object.
(defn- guarded-styles-fn-branch
"Returns the data that styles-fn would use for a given content. When content
is nil, the function falls back to the styles-only branch and returns :fallback
(the real function calls legacy.txt/styles-to-attrs which we don't exercise
here — we only verify the guard itself prevents the getText/getData throws)."
[content]
(if (and content (= (.getText ^js content) ""))
(-> ^js (.getData content)
(.toJS)
(js->clj :keywordize-keys true))
:fallback))
(t/deftest guarded-styles-fn-branch-does-not-throw-on-nil-content
(t/testing "nil content falls back to the styles branch (no throw)"
(t/is (= :fallback (guarded-styles-fn-branch nil)))
(t/is (= :fallback (guarded-styles-fn-branch js/undefined)))))

23
frontend/test/frontend_tests/util/dom/dnd_test.cljs Normal file
View File

@ -0,0 +1,23 @@
;; 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
(ns frontend-tests.util.dom.dnd-test
(:require
[app.util.dom.dnd :as dnd]
[cljs.test :as t :include-macros true]))
(t/deftest get-data-returns-nil-when-event-has-no-dataTransfer
(t/testing "event without dataTransfer"
(t/is (nil? (dnd/get-data #js {}))))
(t/testing "event with explicit nil dataTransfer"
(t/is (nil? (dnd/get-data #js {:dataTransfer nil}))))
(t/testing "explicit data-type also returns nil for missing dataTransfer"
(t/is (nil? (dnd/get-data #js {} "penpot/data")))))
(t/deftest get-data-reads-from-dataTransfer
(t/testing "dataTransfer with matching key returns the value (non-decoded type)"
(let [dt #js {:getData (fn [_type] "hello")}]
(t/is (= "hello" (dnd/get-data #js {:dataTransfer dt} "text/plain"))))))

18
frontend/test/frontend_tests/util_text_editor_test.cljs Normal file
View File

@ -0,0 +1,18 @@
;; 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
(ns frontend-tests.util-text-editor-test
(:require
[app.util.text-editor :as te]
[cljs.test :as t :include-macros true]))
(t/deftest get-editor-block-data-returns-nil-for-nil-block
(t/is (nil? (te/get-editor-block-data nil)))
(t/is (nil? (te/get-editor-block-data js/undefined))))
(t/deftest get-editor-block-type-returns-nil-for-nil-block
(t/is (nil? (te/get-editor-block-type nil)))
(t/is (nil? (te/get-editor-block-type js/undefined))))

44
frontend/test/frontend_tests/util_zip_test.cljs Normal file
View File

@ -0,0 +1,44 @@
;; 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
(ns frontend-tests.util-zip-test
(:require
[app.util.zip :as-alias uz]
[app.worker.import :as worker.import]
[beicon.v2.core :as rx]
[cljs.test :as t :include-macros true]
[frontend-tests.helpers.mock :as mock]
[promesa.core :as p]))
(t/deftest read-as-text-nil-entry-raises-typed-error
(t/testing "read-as-text guards against nil entry"
(t/is (thrown-with-msg? js/Error #"nil"
(app.util.zip/read-as-text nil)))))
(t/deftest read-zip-manifest-missing-throws-validation-error
(t/async
done
(t/testing "read-zip-manifest rejects ZIPs without manifest.json"
(mock/with-mocks
{app.util.zip/get-entry (mock/stub (fn [_ _] (p/resolved nil)))}
(fn [done']
(->> (worker.import/read-zip-manifest #js {})
(rx/subs!
(fn [_]
(t/is false "expected validation error to be thrown")
(done'))
(fn [err]
(let [data (ex-data err)]
(t/is (= :invalid-penpot-file (:code data))
"missing manifest.json raises typed :invalid-penpot-file error")
(t/is (string? (:hint data))
"missing manifest.json error carries a :hint")
(t/is (re-find #"manifest\.json" (:hint data))
"missing manifest.json :hint mentions manifest.json")
(t/is (nil? (re-find #"getData" (:hint data)))
"missing manifest.json :hint does not leak the raw TypeError text")
(done'))))))
done))))