isomorphic-dompurify icon indicating copy to clipboard operation
isomorphic-dompurify copied to clipboard

Web Worker Support

Open wSedlacek opened this issue 2 years ago • 5 comments

Current Behavior

When importing into a worker environment the following runtime error occurs

Uncaught ReferenceError: window is not defined
    at node_modules/.pnpm/[email protected][email protected]/node_modules/isomorphic-dompurify/browser.js (isomorphic-dompurify.js?v=650fd638:949:22)

Change in behavior

The runtime error does not occur and DOMPurify is accessible both as a export and on self.DOMPurify/globalThis.DOMPurify in workers

Developer Notes

There are probably a dozen different ways to fix this issue, the one chosen was picked for being the one with the fewest number of changes while being backwards compatible all the way back to browser versions from 2012 (Firefox back to 2004)

See: https://developer.mozilla.org/en-US/docs/Web/API/Window/self#examples

Other approaches considered

  • Using typeof window !== "unefined"
    • This method of determining if the window exists and if this code is executing in a worker was considered however after this check was done the split in exuection after this was detected would have either needed to use self or pollyfil window on self. Since self always works it seemed more straight forward to just always use it instead of window

Example:

const globalObject = typeof window !== "undefined" ? window : self;
module.exports = globalObject.DOMPurify || (globalObject.DOMPurify = require('dompurify').default || require('dompurify'));
  • Using globalThis
    • This method would be better if trying to target the same code on server and browser however it is a much newer feature and not available on all platforms (ie, React Native, etc.)

Example:

module.exports = globalThis.DOMPurify || (globalThis.DOMPurify = require('dompurify').default || require('dompurify'));
  • Using typeof window !== "undefined" but not assigning to self.DOMPurify
    • This would simply remove the accessing of window in worker environments effectively killing the polyfil nature of this in worker environments but still allowing the import

Example:

const dompurify = require("dompurify")

if (typeof window !== "undefined") {
  window.DOMPurify = window.DOMPurify || dompurify.default || dompurify;
}

module.exports = typeof window !== "undefined" ? window.DOMPurify : dompurify.default || dompurify;
  • Using optional chaning
    • This would have had the same outcome of not having DOMPurify accessible on self/globalThis as the previous method however it would rely on a feature only aviable in newer JavaScript environments.

Example:

const dompurify = require("dompurify")

if (typeof window !== "undefined") {
  window.DOMPurify = window.DOMPurify || dompurify.default || dompurify;
}

module.exports = window?.DOMPurify || dompurify.default || dompurify;
  • Using null coalescing
    • Again this feature is very new and not supported in every environment
const dompurify = require("dompurify")

if (typeof window !== "undefined") {
  window.DOMPurify ??= dompurify.default || dompurify;
}

module.exports = window?.DOMPurify : dompurify.default || dompurify;

fixes: #240

wSedlacek avatar Dec 28 '23 16:12 wSedlacek

Thank you very much @wSedlacek for such a thoughtful analysis of solutions and choosing the best one for your PR! Please give me a couple of days to test the chosen solution.

kkomelin avatar Dec 30 '23 02:12 kkomelin

@wSedlacek I'm having hard times importing isomorphic-dompurify from inside of a Web Worker script. I don't have much experience with web workers. Could you please instruct me how to do it?

I'm using Vite vanilla template and importing this way import DOMPurify from "isomorphic-dompurify";, but unfortunately it gives me DOMPurify.sanitize is not a function.

In my package.json:

 "dependencies": {
    "isomorphic-dompurify": "github:wSedlacek/isomorphic-dompurify#feat/worker-support"
  }

kkomelin avatar Dec 30 '23 10:12 kkomelin

I’ll create an example today for testing and get back to you.

wSedlacek avatar Jan 02 '24 16:01 wSedlacek

https://github.com/cure53/DOMPurify/blob/d7498e0546c0f67cf95437ae430ff2035582a689/src/purify.js#L90-L96

So dompurify itself is also checking the window and early exiting. I am going to mark this as a draft for a moment while I evaluate the options.

Update: I was able to get it running with linkedom as noted in the comments of https://github.com/cure53/DOMPurify/issues/577

import createDOMPurify from "isomorphic-dompurify";
import { parseHTML } from "linkedom";

const { window } = parseHTML("<!DOCTYPE html><body>");
const DOMPurify = createDOMPurify(window);

self.onmessage = (event) => {
  const purified = DOMPurify.sanitize(event.data);
  console.log(`On Worker Thread: ${purified}`); // Should be <img src="x”> but is <img src=x onerror=alert(1)//>
};

The only issue is that while it does run, it doesn’t actually santaize anything at all. This all said, without the changing in this PR the example doesn’t run inside works at all (because it would crash when trying to use window.DOMPurify on import) I think it is better to have the issues with web workers be entirity with dompurify instead of the introp layer.

wSedlacek avatar Jan 02 '24 17:01 wSedlacek

Thank you @wSedlacek for your work on this issue! Really great job!

However, I think we cannot release this change without dompurify itself adding support for Web Worker, so I suggest us keeping this issue open in case anything changes in the future. For your particular case, you may consider using alternatives for now.

kkomelin avatar Jan 03 '24 01:01 kkomelin