Bug 1893645 Comment 24 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to Calixte Denizet (:calixte) from comment #18)
> We still use `new Function` in:
> https://github.com/mozilla/pdf.js/blob/master/src/core/function.js#L428-L436 
> I audited this code few days ago and I didn't see anything wrong, the generated function is executed in the worker so there's no access to the DOM.
> `eval` is used here in order to improve performance of some postscript functions, so I'm not sure that sending the execution in a sandbox and adding a sync point would help to keep the same performance.

A gecko sandbox does not affect performance of the code running inside the sandbox at all - it ends up running the same JS but in a different scope / with a different level of privilege. The only thing that would potentially be affected would be the marshalling of complex things into and out of the sandbox (as they'd have to be `structuredClone`'d), but that seems less likely to be an issue here as it looks like these postscript functions deal with numbers only? I'm not a JS engine expert but I would expect those to be performant just fine. Can we file a follow-up bug to investigate and get someone on the JS team to weigh in? Maybe it ends up wontfix if we really think the trade-off isn't worth it, but it seems worth at least checking if it could work. :-)
(In reply to Calixte Denizet (:calixte) from comment #18)
> We still use `new Function` in:
> https://github.com/mozilla/pdf.js/blob/master/src/core/function.js#L428-L436 
> I audited this code few days ago and I didn't see anything wrong, the generated function is executed in the worker so there's no access to the DOM.
> `eval` is used here in order to improve performance of some postscript functions, so I'm not sure that sending the execution in a sandbox and adding a sync point would help to keep the same performance.

A gecko sandbox does not affect performance of the code running inside the sandbox at all - it ends up running the same JS but in a different scope / with a different level of privilege. The only thing that would potentially be affected would be the marshalling of complex things into and out of the sandbox (as they'd have to be `structuredClone`'d or use xpc wrappers), but that seems less likely to be an issue here as it looks like these postscript functions deal with numbers only? I'm not a JS engine expert but I would expect those to be performant just fine. Can we file a follow-up bug to investigate and get someone on the JS team to weigh in? Maybe it ends up wontfix if we really think the trade-off isn't worth it, but it seems worth at least checking if it could work. :-)

Back to Bug 1893645 Comment 24