RealRaven2000 / SmartTemplates

Thunderbird Add-on: SmartTemplates
http://smarttemplates.quickfolders.org/
Other
25 stars 15 forks source link

Asyncify template scripting #266

Closed benibela closed 11 months ago

benibela commented 12 months ago

Something suddenly installed a thunderbird update on my system and then everything was broken

I updated to "smartTemplate-fx-4.0pre256", and then only the %{% scripting was broken

I made it async:

$  diff /tmp/chrome/content/smartTemplate-overlay.js  ~/opt/smartTemplate-fx-4.0pre256/chrome/content/smartTemplate-overlay.js 
3871a3872,3875
>         //alert("B:"+supportEval);
>         //var oldLog = util.logDebugOptional;
>       
>       //util.logDebugOptional = function(x,y) { alert(x+":: "+y); oldLog(x,y) }
3881c3885
<     function replaceJavascript(dmy, script) {
---
>     async function replaceJavascript(dmy, script) {
3905c3909
<         sandbox.variable = function(name, arg) {
---
>         sandbox.variable = async function(name, arg) {
3908c3912
<           let retVariable = replaceReservedWords("", name, arg || "");
---
>           let retVariable = await replaceReservedWords("", name, arg || "");
3932c3936
<             return function(arg){
---
>             return async function(arg){
3943c3947
<               let sbVal = replaceReservedWords("", aname, arg);
---
>               let sbVal = await replaceReservedWords("", aname, arg);
3958c3962
<         x = Cu.evalInSandbox("(" + script + ").toString()", sandbox); 
---
>         x = await Cu.evalInSandbox("(" + script + ")", sandbox); //todo: need to check if await is safe here
3979c3983
<       msg = msg.replace(/%\{%((.|\n|\r)*?)%\}%/gm, replaceJavascript); // also remove all newlines and unnecessary white spaces
---
>       msg = await SmartTemplate4.Util.replaceAsync(msg, /%\{%((.|\n|\r)*?)%\}%/gm, replaceJavascript); // also remove all newlines and unnecessary white spaces

smartTemplate-overlay.txt

Although that breaks the templates because they need to be made async as well.
One can still write %{% subject.toString() %}%, but now it is not a string, but a Promise

So something like %{% subject.toString() + "abc" %}% gets broken and would needed to be changed to %{% async function () { return await subject.toString() + "a" } () %}% by each user

RealRaven2000 commented 12 months ago

thanks, I was waiting for you to reach out. First and foremost I didn't know whether it was possible to call evalInSandbox asynchronously, couldn't find documentation on that. Obviously I had to make the whole regex parser asynchronously because I started to use API functions and you cannot call (await) async functions from a synchronous function.

I think asking the users to use async for any reserved word maybe an acceptable requirement. I guess we cannot replace functions with await %subject% internally and do the replacement in a first pass before running the script in the sandbox?

RealRaven2000 commented 12 months ago

If it's not possible (or too much hassle) I think asking users to asyncify access to variables would be good enough. Although in your example would it not be more correctly:

%{% async function () { 
  return (await subject).toString()  + "a";
}  () %}%
RealRaven2000 commented 12 months ago

I am currently working on 4.3 so there are some yet uncommited changes relating to #263 #264 #265, here is the latest patched version, including your code:

smartTemplate-fx-4.3pre30.zip

benibela commented 12 months ago

First and foremost I didn't know whether it was possible to call evalInSandbox asynchronously, couldn't find documentation on that.

Me neither. I have never used async/await before. I just tried it out

Obviously I had to make the whole regex parser asynchronously because I started to use API functions and you cannot call (await) async functions from a synchronous function.

Did you? It could have called all necessary API functions first, and then do the parsing afterwards

I guess we cannot replace functions with await %subject% internally and do the replacement in a first pass before running the script in the sandbox?

I do not know

At least, it could prepend async function () { return , so the user does not have to write that

%{% async function () { return (await subject).toString() + "a"; } () %}%

That does not work.

subject is a function. await needs a Promise.

The Function hack that allowed one to write subject+"" and subject("") + "" is confusing things.

Perhaps the valueOf line can be changed to make it a Promise

RealRaven2000 commented 12 months ago

I tried to make an old script work like this:

<p>Hello %from(name)%</p>
<p>
%{%  
async function() {
  switch(from('mail')) {
    case "thunderbirddaily@gmail.com":
      return "<p>Special text for thunderbirddaily.</p>"
        + "<p>Another paragraph!</p>";
    case "thunderbirddaily67@gmail.com":
      return "<p>Hello 67!</p>";
    case "axel@gmail.com":
      return file("W:\\_Tb Profiles\\abcdefg.default\\stationery\\QuickFolders - Links.html");
    default:
      return "<p>default case!</p>";
  }
} ()
%}%
</p>

but it only returns the raw script in the 2nd paragraph. ideas what I am doing wrong?

benibela commented 12 months ago

have you actually enabled it? both allowScripts and sandbox pref?

RealRaven2000 commented 12 months ago

have you actually enabled it? both allowScripts and sandbox pref?

I was missing extensions.smartTemplate4.sandbox .. can't remember why we have 2 settings.

RealRaven2000 commented 12 months ago

ok, this syntax works for me:

%{%  
(async function() {
  switch(await from("mail")) {
    case "thunderbirddaily@gmail.com":
      return "<p>Special text for thunderbirddaily.</p>"
        + "<p>Another paragraph!</p>";
    case "thunderbirddaily67@gmail.com":
      return "<p>Hello 67!</p>";
    case "axel@gmail.com":
      return file("W:\\_Tb Profiles\\abcdefg.default\\stationery\\QuickFolders - Links (English).html");
    default:
      return "<p>default case!</p>" + await from("mail");
  }
})()
%}%

So conversion of scripts is relatively straightforward

RealRaven2000 commented 12 months ago

I also double checked, interestingly the case return %file(..)% doesn't need to be queried with await, because we return it directly (even if it returns a promise) , it is resolved outside of the sandbox.

RealRaven2000 commented 11 months ago

Implemented in 4.3.1 - published 16/10/2023

benibela commented 11 months ago

I was missing extensions.smartTemplate4.sandbox .. can't remember why we have 2 settings.

Probably there used to be an eval without sandbox, and then it got removed

I also double checked, interestingly the case return %file(..)% doesn't need to be queried with await, because we return it directly (even if it returns a promise) , it is resolved outside of the sandbox.

although that might be unsafe ( if code could use it to run outside the sandbox. )