http://ofbiz.116.s1.nabble.com/Re-svn-commit-r772699-in-ofbiz-trunk-framework-base-src-org-ofbiz-base-util-base-src-org-ofbiz-base--tp200657p200658.html
Thank you for your review and comments. There might be a more efficient
way to do this, but this is the best I can come up with. It's an
improvement over the repeated String replace alls.
>
[hidden email] wrote:
>> Author: adrianc
>> Date: Thu May 7 16:30:02 2009
>> New Revision: 772699
>>
>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/BshUtil.java
>> URL:
http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/BshUtil.java?rev=772699&r1=772698&r2=772699&view=diff>> ==============================================================================
>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/BshUtil.java (original)
>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/BshUtil.java Thu May 7 16:30:02 2009
>> @@ -62,19 +61,17 @@
>> Debug.logError("BSH Evaluation error. Empty expression", module);
>> return null;
>> }
>> -
>> - if (Debug.verboseOn())
>> + if (Debug.verboseOn()) {
>> Debug.logVerbose("Evaluating -- " + expression, module);
>> - if (Debug.verboseOn())
>> Debug.logVerbose("Using Context -- " + context, module);
>> -
>> + }
>> try {
>> Interpreter bsh = makeInterpreter(context);
>> // evaluate the expression
>> - o = bsh.eval(expression);
>> - if (Debug.verboseOn())
>> + o = bsh.eval(StringUtil.convertOperatorSubstitutions(expression));
>> + if (Debug.verboseOn()) {
>> Debug.logVerbose("Evaluated to -- " + o, module);
>> -
>> + }
>> // read back the context info
>> NameSpace ns = bsh.getNameSpace();
>> String[] varNames = ns.getVariableNames();
>
> Please try not do do whitespace/formatting changes when altering code
> paths.
>
>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/StringUtil.java
>> URL:
http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/StringUtil.java?rev=772699&r1=772698&r2=772699&view=diff>> ==============================================================================
>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/StringUtil.java (original)
>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/StringUtil.java Thu May 7 16:30:02 2009
>> @@ -56,6 +56,7 @@
>> public class StringUtil {
>>
>> public static final String module = StringUtil.class.getName();
>> + protected static final Map<String, Pattern> substitionPatternMap;
>
> substitution, you mispelled(sic) the word.
>
> Additionally, this is not really a map. It's more a list of
> pattern/replacement pairs.
>
>> /** OWASP ESAPI canonicalize strict flag; setting false so we only get warnings about double encoding, etc; can be set to true for exceptions and more security */
>> public static final boolean esapiCanonicalizeStrict = false;
>> @@ -66,6 +67,14 @@
>> List<Codec> codecList = Arrays.asList(new HTMLEntityCodec(), new PercentCodec());
>> defaultWebEncoder = new DefaultEncoder(codecList);
>> defaultWebValidator = new DefaultValidator();
>> + substitionPatternMap = FastMap.newInstance();
>> + substitionPatternMap.put("&&", Pattern.compile("@and", Pattern.LITERAL));
>> + substitionPatternMap.put("||", Pattern.compile("@or", Pattern.LITERAL));
>> + substitionPatternMap.put("<=", Pattern.compile("@lteq", Pattern.LITERAL));
>> + substitionPatternMap.put(">=", Pattern.compile("@gteq", Pattern.LITERAL));
>> + substitionPatternMap.put("<", Pattern.compile("@lt", Pattern.LITERAL));
>> + substitionPatternMap.put(">", Pattern.compile("@gt", Pattern.LITERAL));
>> + substitionPatternMap.put("\"", Pattern.compile("'", Pattern.LITERAL));
>> }
>>
>> public static final SimpleEncoder htmlEncoder = new HtmlEncoder();
>> @@ -474,6 +483,33 @@
>> return outStrBfr.toString();
>> }
>>
>> + public static String convertOperatorSubstitutions(String expression) {
>> + String result = expression;
>> + if (result != null && result.contains("@")) {
>> + Set<String> keys = substitionPatternMap.keySet();
>> + for (String replacement : keys) {
>
> for (Map.Entry<String, Pattern> entry: substitionPatternMap) {
>
> Don't do a loop, then a separate fetch. The above is more efficient.
>
>> + Pattern pattern = substitionPatternMap.get(replacement);
>> + result = pattern.matcher(result).replaceAll(replacement);
>
> There is probably a more efficient way to do this, instead of looping
> over the entire string for each listing pattern/replacement. Maybe
> combining all the patterns in a (foo|bar) arrangement, looping with
> appendReplacement, then looking up the matched value in the key. But
> I could be wrong on that.
>
>> + }
>> + }
>> + return result;
>> + }
>> +
>> /**
>> * Uses a black-list approach for necessary characters for HTML.
>> * Does not allow various characters (after canonicalization), including "<", ">", "&" (if not followed by a space), and "%" (if not followed by a space).
>>
>
>