[GitHub] [jmeter] FSchumacher commented on a change in pull request #660: bz65108: escape/unescape jmeter variable references

[GitBox <gi...@apache.org>]

FSchumacher commented on a change in pull request #660:
URL: https://github.com/apache/jmeter/pull/660#discussion_r742991169



##########
File path: src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/util/GraphQLRequestParamUtils.java
##########
@@ -56,6 +55,12 @@
 
     private static final Pattern WHITESPACES_PATTERN = Pattern.compile("\\p{Space}+");
 
+    private static final Pattern JMETER_VARIABLE_REFERENCE_PATTERN = Pattern
+            .compile("(:\\s*)\\$\\{([^\\$\\{\\}]+)\\}([\\s,\\}])");

Review comment:
       No problem :)
   
   About the regex. In regex a `[` starts a character group, which means, that the order of the elements inside the brackets is irrelevant. 
   If you look at the following examples:
   ```
   abc: ${__log("Hello\, ${__machineIP()}")}
   abc: ${__log("Some $ and some { or even }")}
   abc: ${someFunction(abc)}${anotherFunction}
   abc: ${func2(a,b)}
   abc: ${func1(a)}
   abc: ${func0()}
   abc: ${more(abc,)}
   abc: Nothing to be found here
   ```
    `(:\s*)\$\{([^\$\{\}]+)\}([\s,\}])` would match the part after `abc` in lines
   ```
   abc: ${func2(a,b)}
   abc: ${func1(a)}
   abc: ${func0()}
   abc: ${more(abc,)}
   ```
   and a regex with negative lookahead like `:\s*\$\{(?!.*\$\{).*\)}` would match 
   ```
   abc: ${__log("Some $ and some { or even }")}
   abc: ${func2(a,b)}
   abc: ${func1(a)}
   abc: ${func0()}
   abc: ${more(abc,)}
   ```
   And those examples don't include a simple variable like `${var}`, which would yield wrong results with the lookahead variant.
   
   About the complex function setup. Considering the usage of the `__log` function, I think it is not that complex. The following expression is a valid one: `${__log("Hello\, ${__machineIP()}")}`.
   Another example is given in the help for [`__eval`](https://jmeter.apache.org/usermanual/functions.html#__eval)
   
   Given this, I don't think, that a simple regex approach would help all cases. It might be enough to help most users, though.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org