matomo-org / tag-manager

Free Open Source Matomo Tag Manager - A simple way to manage and maintain all of your (third-party) tags on your website.
https://matomo.org
GNU General Public License v3.0
169 stars 58 forks source link

[Bug] Calling a custom js variable within a custom js variable within a custom js variable leads to an error #869

Open SW-Vincent opened 2 weeks ago

SW-Vincent commented 2 weeks ago

Matomo Tag Manager recently added a feature to import variables within custom JS variables.

This feature is great overall : you can call a custom JS variable within a custom JS variable and while Matomo UI code will look like this :

function () { 
  var clickHostName = {{JS - Click hostname}};
  var targetHostName =  "dummy.example.com"
  if(targetHostName === targetHostName )){
    return true;
  }
  return false;
}

the code that is actually set by Matomo Tag Manager looks like this :

Templates['CustomJsFunctionVariable5630bd1c'] = (function () { return function (parameters, TagManager) { this.get = function () { 
  var clickHostName = TagManager._buildVariable({"name":"JS - Click hostname","type":"CustomJsFunction","lookUpTable":[],"defaultValue":"","parameters":{"jsFunction":"HERE-IS-A-STRING-FORMATTED-VERSION-OF-THE-FUNCTION"},"Variable":"CustomJsFunctionVariable4d0ada15"}, parameters.get('container')).get();
  var targetHostNames = "dummy.example.com" ;
  if(reservationsHostNames.includes(clickHostName)){
    return true;
  }
  return false;
}; } })(); 

Now we're getting to the bug : you noticed that the clickHostName variable is calling the customJSFunction as an object, with TagManager._buildVariable.parameters.jFunction being a string. If within this string there is an other custom JS function call, the " characters within the function reference will escape the string, leading to an error "Unexpected identifier 'name'" image

image

snake14 commented 2 weeks ago

Hi @SW-Vincent . Thank you for taking the time to create this issue. I'm not sure If I quite follow the issue. I tested nested Custom JS variables and they worked as expected. For example, I had variable 1 return a string value, variable 2 return the value of variable 1 concatenated with another string, and variable 3 return the value of variable 2 concatenated with a string. I didn't see any errors in debug mode or after publishing and variable 3 displayed the expected value.

SW-Vincent commented 2 weeks ago

Hi @snake14,

Considering the test you made, you understood the issue.

I reproduced your test with the following custom JS on 2 Matomo Cloud accounts :

function () { 
  return "abc"
}
function () { 
  //{{JS - const for debug - lvl 1}} function returns "abc"
  return {{JS - const for debug - lvl 1}} + "def"
}
function () { 
  //{{JS - const for debug - lvl 2}} concatenates {{JS - const for debug - lvl 1}} with "def"
  return {{JS - const for debug - lvl 2}} + "ghi"
}

The issue occured on one of the two Matomo Cloud accounts i ran the test on. The difference between these accounts is that content security policy prevents the error somehow when partially bloquing the preview script (the console cannot acces the code od container_containerId_preview.js so the error is not displayed and the preview works as expected).

Note that for the issue occurs even when the variable is never called.

I took the risk of publishing the container on the account that didn't show the issue (where CSP blocks the preview) and here is what i get :

Templates['CustomJsFunctionVariable452dc43a'] = (function() {
            return function(parameters, TagManager) {
                this.get = function() {
                    //TagManager._buildVariable({"name":"JS - const for debug - lvl 2","type":"CustomJsFunction","lookUpTable":[],"defaultValue":"","parameters":{"jsFunction":"function () { \n  \/\/{{JS - const for debug - lvl 1}} function returns \"abc\"\n  return {{JS - const for debug - lvl 1}} + \"def\"\n}"},"Variable":"CustomJsFunctionVariabled69366c8"}, parameters.get('container')).get() concatenates TagManager._buildVariable({"name":"JS - const for debug - lvl 1","type":"CustomJsFunction","lookUpTable":[],"defaultValue":"","parameters":{"jsFunction":"function () { \n  return \"abc\"\n}"},"Variable":"CustomJsFunctionVariable73c91c00"}, parameters.get('container')).get() with "def"
                    return TagManager._buildVariable({
                        "name": "JS - const for debug - lvl 2",
                        "type": "CustomJsFunction",
                        "lookUpTable": [],
                        "defaultValue": "",
                        "parameters": {
                            "jsFunction": "function () { \n  \/\/{{JS - const for debug - lvl 1}} function returns \"abc\"\n  return {{JS - const for debug - lvl 1}} + \"def\"\n}"
                        },
                        "Variable": "CustomJsFunctionVariabled69366c8"
                    }, parameters.get('container')).get() + "ghi"
                }
                ;
            }
        }

There is no issue once published (as you can see the 1st level variable isn't decompliled within the 3rd level variable), so it may be a preview issue only. This is still an issue though, as when it occurs preview doesn't work.

snake14 commented 2 weeks ago

@SW-Vincent That looks very similar to the test that I ran. However, I didn't see any errors in the JS console while in preview mode and the string output as expected. Maybe it's browser specific. I'm testing using the Brave browser. What browser are you testing with? Also, what's the rest of your environment (version of Matomo, ...) look like?

SW-Vincent commented 2 weeks ago

What browser are you testing with?

@snake14 I ran my previous tests on Chrome, with Matomo Helper Chrome extension active. I just did the same tests on Microsoft Edge and the results are the same.

What's the rest of your environment (version of Matomo, ...) look like

I used Matomo Cloud accounts for all the tests so i don't keep track of the version.

snake14 commented 2 weeks ago

@SW-Vincent I'm still unable to reproduce the issue. I tested on a Matomo Cloud instance (altamash.matomo.cloud), created variables nearly identical to what you described above, and I can see the variables outputting the expected values in preview mode. I tried Brave, Firefox, Chrome, and Opera; All of the browsers that I tried worked as expected.

Any suggestions @AltamashShaikh ?

AltamashShaikh commented 2 weeks ago

@SW-Vincent I tried creating 3 variables

  1. normal-nested function () { return "abc"; }
  2. normal-nested-lvl1 function () { return {{normal-nested}} + "def"; }
  3. normal-nested-lvl2 function () { return {{normal-nested-lvl1}} + "ghi"; }

I can see the following output in debug window

Screenshot from 2024-09-04 07-42-32

I published this changes and it seems to work fine for me too

Screenshot from 2024-09-04 07-43-13

Screenshot from 2024-09-04 07-45-20

SW-Vincent commented 2 weeks ago

A few specifications about my case :

Can you confirm to me that there is no CSP error in your console ? When there is a CSP error, preview still works as expected but the nested variables error doesn't occur (as its execution is bloqued by CSP) ;

AltamashShaikh commented 2 weeks ago

@SW-Vincent No CSP error and I tested in both preview and publish mode.

SW-Vincent commented 2 weeks ago

Then may i email you the matomo URL so we can check wether this is a Matomo Tag Manager issue or not ?

AltamashShaikh commented 2 weeks ago

Then may i email you the matomo URL so we can check wether this is a Matomo Tag Manager issue or not ?

@SW-Vincent sure :+1:

AltamashShaikh commented 4 hours ago

@SW-Vincent Can you check again ? You would have to create a new Variable and test it, since the fix was deployed but we cannot fix the existing ones