mirror of
https://github.com/penpot/penpot.git
synced 2026-05-11 19:13:49 +00:00
🐛 Reject clipboard helpers gracefully on insecure origins (#9188)
* 🐛 Reject clipboard helpers gracefully on insecure origins Closes #6514. Resolves the user-visible crash originally reported in #4478. `app.util.clipboard/to-clipboard` and `to-clipboard-promise` called `(unchecked-get js/navigator "clipboard")` and then immediately invoked `.writeText` / `.write` on the result, with no guard for the case where `navigator.clipboard` is `undefined`. The W3C Clipboard API spec requires a "secure context" (HTTPS or localhost), so a Penpot instance served over plain HTTP - which the SSDP/LAN self-hosted setup in #4478 was - throws TypeError: Cannot read properties of undefined (reading 'writeText') synchronously the moment the user clicks any copy button. The error escapes the consuming function before any error-handling rx/of arm runs, so the whole UI ends up on the error screen instead of just the affected control showing a "could not copy" message. A third helper (`to-clipboard-multi`) already guards `clipboard` and `clipboard.write`, but if both are missing it silently returns nil which is also surprising for callers expecting a Promise. ## Fix Add a small `get-clipboard` accessor and an `unavailable-error` factory that returns `Promise.reject(Error(...))` with a clear message ("Clipboard API is unavailable. This usually happens when the page is served over plain HTTP; serve Penpot over HTTPS to enable copy-to-clipboard."). Wire all three helpers through the same defensive contract: - `to-clipboard` - return the rejected Promise when `navigator.clipboard.writeText` is missing. - `to-clipboard-promise` - return the rejected Promise when `navigator.clipboard.write` is missing. - `to-clipboard-multi` - convert the existing `if` into a `cond` with three branches: prefer `clipboard.write` for true multi-MIME output, fall through to `writeText` with the text/plain payload when only the legacy text path is available, and finally reject with the unavailable error when neither path exists. Previously the no-API case fell off the `when-let` and silently returned nil. The contract is now consistent: every helper either resolves or rejects a Promise, never throws synchronously, and never returns nil. Callers (which are already structured around rx streams that call `rx/from` on the helper's return value) can chain `.catch` / `rx/catch` to surface a status-bar message instead of crashing. The two stale `;; FIXME` comments on `to-clipboard` (rename to `write-text`) and `to-clipboard-promise` (this API is very confuse) are removed - the rename remains an open follow-up across 13+ call sites and is intentionally out of scope, but the API is no longer "confuse" once the contract is documented and uniform. CHANGES.md entry added under the 2.17.0 Bugs-fixed section describing the user-visible behaviour change. * 🐛 Reject paste-from-navigator gracefully on insecure origins Symmetric companion to the to-clipboard / to-clipboard-promise / to-clipboard-multi guards added earlier in this PR. The paste path went through fromNavigator (frontend/src/app/util/clipboard.js) which called `navigator.clipboard.read()` with no nil-check; on insecure origins (plain HTTP / non-localhost) this raised an opaque `TypeError: Cannot read properties of undefined (reading 'read')` and the workspace surfaced a generic 'Something wrong has happened' toast instead of the descriptive 'serve Penpot over HTTPS …' message users get for the copy direction. Mirror the get-clipboard pattern from clipboard.cljs: - Read `navigator.clipboard` once into a local. - If it's missing the `.read` method, throw a descriptive Error that matches the wording the copy direction already uses (only the verb swaps: 'paste-from-clipboard' instead of 'copy-to-clipboard'). - Otherwise, dispatch through the local handle. The existing app.util.clipboard/from-navigator (clipboard.cljs:32) already wraps impl/fromNavigator in rx/from, so a rejected Promise from the async function propagates as an rx error event. Existing callers that subscribe with .catch / on-error see the structured Error and surface the toast, identical to how to-clipboard's unavailable-error already flows. Repro (matches niwinz's reproduction in the PR comment): Object.defineProperty(navigator, 'clipboard', { value: undefined }); // … then attempt a paste action in the workspace … Before: TypeError in console + 'Something wrong has happened' toast. After: descriptive Error caught by the rx subscription and rendered through the existing unavailable-Clipboard-API surface. Refs #6514, #4478 * 🐛 Show user-facing toast when clipboard API is unavailable Niwinz's review on penpot#9188 caught that the rejected Promise from to-clipboard / to-clipboard-promise / to-clipboard-multi / fromNavigator now surfaces the correct error to the console, but the workspace UI still falls through to the generic "Something wrong has happened" toast because the on-clipboard-permission-error and the paste error-handler in paste-from-clipboard only branched on clipboard-permission-error?. Apply the patch he suggested in the review: - Add clipboard-unavailable-error? predicate that matches the Promise.reject(Error("Clipboard API is unavailable. ...")) thrown by the get-clipboard / unavailable-error helpers added earlier in this PR. Uses str/starts-with? on the message prefix so the predicate stays stable even if the trailing "serve Penpot over HTTPS ..." advice text is reworded later. - Convert on-clipboard-permission-error from `if` to `cond` and add a third arm that fires errors.clipboard-api-unavailable as a warning toast. - Add the same arm in the second cond block inside paste-from-clipboard, before the :not-implemented and :else arms. - Add the matching errors.clipboard-api-unavailable entry to frontend/translations/en.po with the wording niwinz suggested: "Clipboard API is unavailable. Serve Penpot over HTTPS to enable clipboard access". Refs penpot#9188 review. --------- Signed-off-by: Andrey Antukh <niwi@niwi.nz> Co-authored-by: Andrey Antukh <niwi@niwi.nz>
This commit is contained in:
Milos Milic
GitHub
parent
08bd53b6a1
commit
b54fa2f11c
@ -19,6 +19,7 @@
|
||||
- Fix `get-profile` masking transient DB errors as anonymous user (by @jack-stormentswe) [Github #9253](https://github.com/penpot/penpot/issues/9253)
|
||||
- Fix `Ctrl+'` "Show guides" shortcut on non-US keyboard layouts by matching the physical key location (by @RenzoMXD) [Github #8423](https://github.com/penpot/penpot/issues/8423)
|
||||
- Fix lost-update race on `team.features` during concurrent file creation (by @web-dev0521) [Github #9197](https://github.com/penpot/penpot/issues/9197)
|
||||
- Fix copy and paste actions crashing the workspace on insecure origins (plain HTTP / non-`localhost`) where the Clipboard API is unavailable (by @MilosM348) [Github #6514](https://github.com/penpot/penpot/issues/6514)
|
||||
|
||||
|
||||
## 2.16.0 (Unreleased)
|
||||
|
||||
@ -303,13 +303,30 @@
|
||||
(and (instance? js/DOMException cause)
|
||||
(= (.-name cause) "NotAllowedError")))
|
||||
|
||||
(defn- clipboard-unavailable-error?
|
||||
"Check if the given error is a clipboard API unavailable error
|
||||
(thrown when navigator.clipboard is undefined, e.g. on insecure
|
||||
origins per the W3C Secure Contexts spec)."
|
||||
[cause]
|
||||
(and (instance? js/Error cause)
|
||||
(str/starts-with? (.-message cause) "Clipboard API is unavailable.")))
|
||||
|
||||
(defn- on-clipboard-permission-error
|
||||
[cause]
|
||||
(if (clipboard-permission-error? cause)
|
||||
(cond
|
||||
(clipboard-permission-error? cause)
|
||||
(rx/of (ntf/show {:content (tr "errors.clipboard-permission-denied")
|
||||
:type :toast
|
||||
:level :warning
|
||||
:timeout 5000}))
|
||||
|
||||
(clipboard-unavailable-error? cause)
|
||||
(rx/of (ntf/show {:content (tr "errors.clipboard-api-unavailable")
|
||||
:type :toast
|
||||
:level :warning
|
||||
:timeout 5000}))
|
||||
|
||||
:else
|
||||
(rx/throw cause)))
|
||||
|
||||
(defn paste-from-clipboard
|
||||
@ -511,6 +528,12 @@
|
||||
:level :warning
|
||||
:timeout 5000}))
|
||||
|
||||
(clipboard-unavailable-error? cause)
|
||||
(rx/of (ntf/show {:content (tr "errors.clipboard-api-unavailable")
|
||||
:type :toast
|
||||
:level :warning
|
||||
:timeout 5000}))
|
||||
|
||||
(:not-implemented (ex-data cause))
|
||||
(rx/of (ntf/warn (tr "errors.clipboard-not-implemented")))
|
||||
|
||||
|
||||
@ -70,40 +70,85 @@
|
||||
([event options]
|
||||
(from-data-transfer (.-dataTransfer ^js event) options)))
|
||||
|
||||
;; FIXME: rename to `write-text`
|
||||
(def ^:private unavailable-error-message
|
||||
"Clipboard API is unavailable. This usually happens when the page is served over plain HTTP; serve Penpot over HTTPS to enable copy-to-clipboard.")
|
||||
|
||||
(defn- get-clipboard
|
||||
"Return the active `navigator.clipboard` object, or nil when the
|
||||
asynchronous Clipboard API is not exposed (e.g. on insecure origins
|
||||
per the W3C spec, which is what triggered #4478)."
|
||||
[]
|
||||
(unchecked-get js/navigator "clipboard"))
|
||||
|
||||
(defn- unavailable-error
|
||||
"Build the error wrapped in a rejected Promise so callers can chain
|
||||
`rx/from`/`.catch` and surface a meaningful message instead of the
|
||||
opaque `TypeError: Cannot read properties of undefined (reading
|
||||
'writeText')` that leaked out of the previous implementation."
|
||||
[]
|
||||
(js/Promise.reject (js/Error. unavailable-error-message)))
|
||||
|
||||
(defn to-clipboard
|
||||
"Write a plain-text string to the system clipboard.
|
||||
|
||||
Always returns a Promise. Resolves on success; rejects with a clear
|
||||
`Error` when `navigator.clipboard` is unavailable (insecure origin)
|
||||
so callers can chain error handling instead of crashing the UI on a
|
||||
synchronous `TypeError` like #4478."
|
||||
[data]
|
||||
(assert (string? data) "`data` should be string")
|
||||
(let [clipboard (unchecked-get js/navigator "clipboard")]
|
||||
(.writeText ^js clipboard data)))
|
||||
(let [clipboard (get-clipboard)]
|
||||
(if (and clipboard (unchecked-get clipboard "writeText"))
|
||||
(.writeText ^js clipboard data)
|
||||
(unavailable-error))))
|
||||
|
||||
(defn- create-clipboard-item
|
||||
[mimetype promise]
|
||||
(js/ClipboardItem.
|
||||
(js-obj mimetype promise)))
|
||||
|
||||
;; FIXME: this API is very confuse
|
||||
(defn to-clipboard-promise
|
||||
"Write a single asynchronous payload to the clipboard under the given
|
||||
MIME type. The `promise` is resolved by the browser when the
|
||||
ClipboardItem is committed.
|
||||
|
||||
Returns the Promise produced by `clipboard.write`, or a rejected
|
||||
Promise carrying an `Error` when the asynchronous Clipboard API is
|
||||
not available (insecure origin / unsupported browser). Mirrors
|
||||
`to-clipboard`'s defensive contract."
|
||||
[mimetype promise]
|
||||
(let [clipboard (unchecked-get js/navigator "clipboard")
|
||||
data (create-clipboard-item mimetype promise)]
|
||||
(.write ^js clipboard #js [data])))
|
||||
(let [clipboard (get-clipboard)]
|
||||
(if (and clipboard (unchecked-get clipboard "write"))
|
||||
(let [data (create-clipboard-item mimetype promise)]
|
||||
(.write ^js clipboard #js [data]))
|
||||
(unavailable-error))))
|
||||
|
||||
(defn to-clipboard-multi
|
||||
"Write multiple MIME representations as a single ClipboardItem.
|
||||
`items` is a map of mime-type (string) -> string payload.
|
||||
Falls back to `writeText` when the async Clipboard API is unavailable."
|
||||
`items` is a map of mime-type (string) -> string payload.
|
||||
|
||||
Falls back to `writeText` with the `text/plain` payload (or the
|
||||
first available payload) when the asynchronous `clipboard.write`
|
||||
API is unavailable. If neither path is reachable (e.g. insecure
|
||||
origin), returns a rejected Promise mirroring `to-clipboard`'s
|
||||
contract instead of throwing synchronously."
|
||||
[items]
|
||||
(let [clipboard (unchecked-get js/navigator "clipboard")]
|
||||
(if (and clipboard (unchecked-get clipboard "write"))
|
||||
(let [obj (reduce-kv
|
||||
(fn [acc mime payload]
|
||||
(let [blob (js/Blob. #js [payload] #js {:type mime})]
|
||||
(unchecked-set acc mime (js/Promise.resolve blob))
|
||||
acc))
|
||||
#js {} items)
|
||||
(let [clipboard (get-clipboard)]
|
||||
(cond
|
||||
(and clipboard (unchecked-get clipboard "write"))
|
||||
(let [obj (reduce-kv
|
||||
(fn [acc mime payload]
|
||||
(let [blob (js/Blob. #js [payload] #js {:type mime})]
|
||||
(unchecked-set acc mime (js/Promise.resolve blob))
|
||||
acc))
|
||||
#js {} items)
|
||||
item (js/ClipboardItem. obj)]
|
||||
(.write ^js clipboard #js [item]))
|
||||
|
||||
(and clipboard (unchecked-get clipboard "writeText"))
|
||||
(when-let [text (or (get items "text/plain")
|
||||
(first (vals items)))]
|
||||
(.writeText ^js clipboard text)))))
|
||||
(.writeText ^js clipboard text))
|
||||
|
||||
:else
|
||||
(unavailable-error))))
|
||||
|
||||
@ -145,13 +145,27 @@ function sortItems(a, b) {
|
||||
}
|
||||
|
||||
/**
|
||||
* Read the active system clipboard via the asynchronous Clipboard API.
|
||||
*
|
||||
* Throws a descriptive `Error` when `navigator.clipboard` is unavailable
|
||||
* (e.g. on insecure origins per the W3C Secure Contexts spec, mirroring
|
||||
* the failure mode that crashed the copy/write path in #4478 / #6514).
|
||||
* Without this guard, `navigator.clipboard.read()` raises an opaque
|
||||
* `TypeError: Cannot read properties of undefined (reading 'read')` and
|
||||
* the workspace surfaces a generic "Something wrong has happened" toast.
|
||||
*
|
||||
* @param {ClipboardSettings} [options]
|
||||
* @returns {Promise<Array<Blob>>}
|
||||
*/
|
||||
export async function fromNavigator(options) {
|
||||
options = options || {};
|
||||
const items = await navigator.clipboard.read();
|
||||
const clipboard = navigator.clipboard;
|
||||
if (!clipboard || typeof clipboard.read !== "function") {
|
||||
throw new Error(
|
||||
"Clipboard API is unavailable. This usually happens when the page is served over plain HTTP; serve Penpot over HTTPS to enable paste-from-clipboard."
|
||||
);
|
||||
}
|
||||
const items = await clipboard.read();
|
||||
const result = await Promise.all(
|
||||
Array.from(items).map(async (item) => {
|
||||
const itemAllowedTypes = Array.from(item.types)
|
||||
|
||||
@ -1413,6 +1413,10 @@ msgstr "Your browser cannot do this operation"
|
||||
msgid "errors.clipboard-permission-denied"
|
||||
msgstr "Clipboard access denied. Please allow clipboard permissions in your browser to paste content"
|
||||
|
||||
#: src/app/main/data/workspace/clipboard.cljs
|
||||
msgid "errors.clipboard-api-unavailable"
|
||||
msgstr "Clipboard API is unavailable. Serve Penpot over HTTPS to enable clipboard access"
|
||||
|
||||
#: src/app/main/errors.cljs:235
|
||||
msgid "errors.comment-error"
|
||||
msgstr "There was an error with the comment"
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user