Cesium3DTileStyle string variables substitution sometimes fails

[kiselev-dv]

What happened?

Substitution of variables in Cesium Scene Expression _evaluateVariableString not always substitute all the templates

cesium/packages/engine/Source/Scene/Expression.js

Lines 1217 to 1231 in 4e6dde5

	Node.prototype._evaluateVariableString = function (feature) {
	let result = this._value;
	let match = variableRegex.exec(result);
	while (match !== null) {
	const placeholder = match[0];
	const variableName = match[1];
	let property = getFeatureProperty(feature, variableName);
	if (!defined(property)) {
	property = "";
	}
	result = result.replace(placeholder, property);
	match = variableRegex.exec(result);
	}
	return result;
	};

Current version uses regex stored in a global variable. In JavaScript regular expression objects are statefull and _evaluateVariableString changes the regexp input string not accounting for regexp object state.

variableRegex keeps lastIndex that lead to an error in following scenario:

1. Have two variable placeholders in a string
2. Match first one (at this step lastIndex would point 1 position after variable template)
3. Rewrite the string with variable value,
4. For rewritten string, if value is shorter than variable name + 3 lastIndex will point further in rewritten string
5. If for rewritten string, lastIndex now points after $ sign for second variable template, match will fail and value won't be substituted.

One way to fix it would be to account for result string change in length after the replacement.

variableRegex.lastIndex += (property.length - placeholder.length);

Here is a patch (for whatever reason it fails to upload as a separate file)

diff --git a/packages/engine/Source/Scene/Expression.js b/packages/engine/Source/Scene/Expression.js
index a87be1b094..13b40725cc 100644
--- a/packages/engine/Source/Scene/Expression.js
+++ b/packages/engine/Source/Scene/Expression.js
@@ -1225,6 +1225,7 @@ Node.prototype._evaluateVariableString = function (feature) {
       property = "";
     }
     result = result.replace(placeholder, property);
+    variableRegex.lastIndex += (property.length - placeholder.length);
     match = variableRegex.exec(result);
   }
   return result;

There is a stack overflow question describing how state in JavaScript regexp works https://stackoverflow.com/questions/11477415/why-does-javascripts-regex-exec-not-always-return-the-same-value

Reproduction steps

...

Sandcastle example

No response

Environment

Browser: Firefox
CesiumJS Version: 1.124 1.125
Operating System: Linux Mint

[ggetz]

Good catch @kiselev-dv! Would you be able to open a PR with the fix?

[kiselev-dv]

Yep, and I'd really appreciate if you tell me how to run a specific test spec. I've tried npx gulp test --specs Specs/Scene/ExpressionSpec.js but it still runs all of the test, which takes quite a while.

UPD: found it nwm

[kiselev-dv]

Here is a PR #12466

I have signed Contributor License Agreement but it was somewhat like 4 years ago or mb more, I'm not sure if I have to update it.

I'm in CONTRIBUTORS.md but company have changed the name and domain name, and I'm not sure should I update it or not.

[ggetz]

Awesome, thank you!

I have signed Contributor License Agreement but it was somewhat like 4 years ago or mb more, I'm not sure if I have to update it.

You should still be covered!

I'm in CONTRIBUTORS.md but company have changed the name and domain name, and I'm not sure should I update it or not.

If it is the same organization, please feel free to update their information in CONTRIBUTORS.md. Otherwise, you can add yourself under a new organization as well.

Yep, and I'd really appreciate if you tell me how to run a specific test spec. I've tried npx gulp test --specs Specs/Scene/ExpressionSpec.js but it still runs all of the test, which takes quite a while.

If anyone else needs to know, it's with fit.