Author: sascharodekamp
Date: Mon Feb 28 13:43:23 2011 New Revision: 1075322 URL: http://svn.apache.org/viewvc?rev=1075322&view=rev Log: Improvement - if-regexp conditions in Minilang and Screens not thread safe (https://issues.apache.org/jira/browse/OFBIZ-4107). I added another getTestPattern Method to support case insensitive tests. The case insensitive testPattern is needed from the UtilHttp.checkURLforSpiders. Thanks Martin & Dimitri for your effort. Orig. Msg.: The Perl5Matcher Perl5Compiler are not thread safe, as mentioned in their documentation: http://jakarta.apache.org/oro/api/org/apache/oro/text/regex/Perl5Compiler.html A separate instance should be used per thread. The concrete issue occurred in our system in the createCommContentDataResource service. The service tries to save emails to the database after they have been sent. It failed because of the described issue. This caused the whole email service (sendMailFromScreen) to be rescheduled. So customers got emails twice. The Perl5Matcher and Perl5Compiler instances are assigned to static fields in the org.ofbiz.minilang.method.ifops.IfRegexp class. So every thread will use the same instance. No synchronization is done currently. Changing the fields to be non-static will not work either, as SimpleMethods are cached. So all calls to the same simple method use the same IfRegexp instance. We fixed the problem by moving the instantiation of the Perl5Matcher and Perl5Compiler to the exec() method in IfRegexp. Other classes are most likely affected, too: org.ofbiz.entity.condition.EntityComparisonOperator, org.ofbiz.minilang.method.conditional.RegexpCondition, org.ofbiz.minilang.operation.Regexp, org.ofbiz.widget.menu.ModelMenuCondition$IfRegexp, org.ofbiz.widget.screen.ModelScreenCondition$IfRegexp, org.ofbiz.widget.tree.ModelTreeCondition$IfRegexp Added: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/CompilerMatcher.java (with props) ofbiz/trunk/framework/minilang/script/ ofbiz/trunk/framework/minilang/script/org/ ofbiz/trunk/framework/minilang/script/org/ofbiz/ ofbiz/trunk/framework/minilang/script/org/ofbiz/minilang/ ofbiz/trunk/framework/minilang/script/org/ofbiz/minilang/method/ ofbiz/trunk/framework/minilang/script/org/ofbiz/minilang/method/ifops/ ofbiz/trunk/framework/minilang/script/org/ofbiz/minilang/method/ifops/IfRegexpTests.xml (with props) ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/ifops/test/ ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/ifops/test/IfRegexpTest.java (with props) ofbiz/trunk/framework/minilang/testdef/ ofbiz/trunk/framework/minilang/testdef/MinilangTests.xml (with props) Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilHttp.java ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/EntityComparisonOperator.java ofbiz/trunk/framework/minilang/build.xml ofbiz/trunk/framework/minilang/ofbiz-component.xml ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/conditional/RegexpCondition.java ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/ifops/IfRegexp.java ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/operation/Regexp.java ofbiz/trunk/framework/widget/src/org/ofbiz/widget/form/ModelFormField.java ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenuCondition.java ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/ModelScreenCondition.java ofbiz/trunk/framework/widget/src/org/ofbiz/widget/tree/ModelTreeCondition.java Added: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/CompilerMatcher.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/CompilerMatcher.java?rev=1075322&view=auto ============================================================================== --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/CompilerMatcher.java (added) +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/CompilerMatcher.java Mon Feb 28 13:43:23 2011 @@ -0,0 +1,145 @@ +/******************************************************************************* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + *******************************************************************************/ +package org.ofbiz.base.util; + +import org.apache.oro.text.perl.Perl5Util; +import org.apache.oro.text.regex.MalformedPatternException; +import org.apache.oro.text.regex.Pattern; +import org.apache.oro.text.regex.Perl5Compiler; +import org.apache.oro.text.regex.Perl5Matcher; +import org.ofbiz.base.util.cache.UtilCache; + +/** + * Performs regular expression matching and compiled regular expression + * pattern caching. + */ +public class CompilerMatcher { + + public static final String module = CompilerMatcher.class.getName(); + + public static UtilCache<String, Pattern> compiledPatterns = UtilCache.createUtilCache("regularExpression.compiledPatterns", false); + + private Perl5Compiler compiler = new Perl5Compiler(); + private Perl5Matcher matcher = new Perl5Matcher(); + private Perl5Util perl5Util = new Perl5Util(); + + /** + * This class is *not* thread-safe so it must be + * accessed from a java.lang.ThreadLocal instance for multi-thread usage. + * ThreadLocal causes slightly extra memory usage, but allows for faster + * thread-safe processing than synchronization would afford. + * + * @return + */ + public static ThreadLocal<CompilerMatcher> getThreadLocal() { + return new ThreadLocal<CompilerMatcher>() { + + protected CompilerMatcher initialValue() { + return new CompilerMatcher(); + } + }; + } + + /** + * Returns true if the compiled version of the patternString regular + * expression argument matches the aString argument. + * Case sensitive + * + * @param aString + * @param patternString + * @return + * @throws MalformedPatternException + */ + public boolean matches(String aString, String patternString) throws MalformedPatternException { + return this.matches(aString, patternString, true); + } + + /** + * Returns true if the compiled version of the patternString regular + * expression argument matches the aString argument. + * @param aString + * @param patternString + * @param caseSensitive + * @return + * @throws MalformedPatternException + */ + public boolean matches(String aString, String patternString, boolean caseSensitive) throws MalformedPatternException { + return this.matcher.matches(aString, this.getTestPattern(patternString, caseSensitive)); + } + + + /** + * Returns true if the compiled version of the patternString regular + * expression argument is contained in the aString argument. + * + * @param aString + * @param patternString + * @return + * @throws MalformedPatternException + */ + public boolean contains(String aString, String patternString) throws MalformedPatternException { + return this.matcher.contains(aString, this.getTestPattern(patternString)); + } + + /** + * Compiles and caches a case sensitive regexp pattern for the given string pattern. + * + * @param stringPattern + * @return + * @throws MalformedPatternException + */ + private Pattern getTestPattern(String stringPattern) throws MalformedPatternException { + return this.getTestPattern(stringPattern, true); + } + + /** + * Compiles and caches a regexp pattern for the given string pattern. + * + * @param stringPattern + * @param caseSensitive + * @return + * @throws MalformedPatternException + */ + private Pattern getTestPattern(String stringPattern, boolean caseSensitive) throws MalformedPatternException { + Pattern pattern = compiledPatterns.get(stringPattern); + if (pattern == null) { + if (caseSensitive) { + pattern = compiler.compile(stringPattern); + } else { + pattern = compiler.compile(stringPattern, Perl5Compiler.CASE_INSENSITIVE_MASK); + } + + compiledPatterns.put(stringPattern, pattern); + Debug.logVerbose("Compiled and cached a pattern: '" + stringPattern + "' - " + Thread.currentThread(), module); + } + return pattern; + } + + /** + * Perl5Util's substitute() function implements Perl's s/// operator. + * It takes two arguments: a substitution expression, and an input. + * + * @param stringPattern + * @param input + * @return + */ + public String substitute(String stringPattern, String input) { + return this.perl5Util.substitute(stringPattern, input); + } +} Propchange: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/CompilerMatcher.java ------------------------------------------------------------------------------ svn:mime-type = text/plain Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilHttp.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilHttp.java?rev=1075322&r1=1075321&r2=1075322&view=diff ============================================================================== --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilHttp.java (original) +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilHttp.java Mon Feb 28 13:43:23 2011 @@ -30,7 +30,6 @@ import java.nio.ByteBuffer; import java.sql.Timestamp; import java.util.ArrayList; import java.util.Arrays; -import com.ibm.icu.util.Calendar; import java.util.Collection; import java.util.Currency; import java.util.Enumeration; @@ -42,8 +41,6 @@ import java.util.Map; import java.util.Set; import java.util.StringTokenizer; import java.util.TimeZone; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import javax.servlet.ServletContext; import javax.servlet.http.HttpServletRequest; @@ -54,9 +51,12 @@ import javolution.util.FastList; import javolution.util.FastMap; import org.apache.commons.lang.RandomStringUtils; +import org.apache.oro.text.regex.MalformedPatternException; import org.owasp.esapi.errors.EncodingException; import org.owasp.esapi.errors.IntrusionException; +import com.ibm.icu.util.Calendar; + /** * HttpUtil - Misc HTTP Utility Functions */ @@ -1266,20 +1266,26 @@ public class UtilHttp { String initialUserAgent = request.getHeader("User-Agent") != null ? request.getHeader("User-Agent") : ""; List<String> spiderList = StringUtil.split(UtilProperties.getPropertyValue("url", "link.remove_lsessionid.user_agent_list"), ","); if (UtilValidate.isNotEmpty(spiderList)) { - for (String spiderNameElement: spiderList) { - Pattern p = Pattern.compile("^.*" + spiderNameElement + ".*$", Pattern.CASE_INSENSITIVE); - Matcher m = p.matcher(initialUserAgent); - if (m.find()) { - request.setAttribute("_REQUEST_FROM_SPIDER_", "Y"); - result = true; - break; + + CompilerMatcher compilerMatcher = new CompilerMatcher(); + for (String spiderNameElement : spiderList) { + try { + if (compilerMatcher.matches(initialUserAgent, "^.*" + spiderNameElement + ".*$", false)) { + request.setAttribute("_REQUEST_FROM_SPIDER_", "Y"); + result = true; + break; + } + } + catch (MalformedPatternException e) { + Debug.logError(e, module); } } } } - if (!result) + if (!result) { request.setAttribute("_REQUEST_FROM_SPIDER_", "N"); + } return result; } Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/EntityComparisonOperator.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/EntityComparisonOperator.java?rev=1075322&r1=1075321&r2=1075322&view=diff ============================================================================== --- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/EntityComparisonOperator.java (original) +++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/EntityComparisonOperator.java Mon Feb 28 13:43:23 2011 @@ -30,6 +30,7 @@ import org.apache.oro.text.regex.Pattern import org.apache.oro.text.regex.PatternMatcher; import org.apache.oro.text.regex.Perl5Compiler; import org.apache.oro.text.regex.Perl5Matcher; +import org.ofbiz.base.util.CompilerMatcher; import org.ofbiz.base.util.Debug; import org.ofbiz.base.util.UtilGenerics; import org.ofbiz.base.util.UtilValidate; @@ -47,26 +48,19 @@ public abstract class EntityComparisonOp public static final String module = EntityComparisonOperator.class.getName(); - protected static PatternMatcher matcher = new Perl5Matcher(); - protected static Perl5Util perl5Util = new Perl5Util(); - protected static PatternCompiler compiler = new Perl5Compiler(); + protected transient static ThreadLocal<CompilerMatcher> compilerMatcher = CompilerMatcher.getThreadLocal(); - public static Pattern makeOroPattern(String sqlLike) { + public static String makeOroPattern(String sqlLike) { try { - sqlLike = perl5Util.substitute("s/([$^.+*?])/\\\\$1/g", sqlLike); - sqlLike = perl5Util.substitute("s/%/.*/g", sqlLike); - sqlLike = perl5Util.substitute("s/_/./g", sqlLike); + sqlLike = compilerMatcher.get().substitute("s/([$^.+*?])/\\\\$1/g", sqlLike); + sqlLike = compilerMatcher.get().substitute("s/%/.*/g", sqlLike); + sqlLike = compilerMatcher.get().substitute("s/_/./g", sqlLike); } catch (Throwable t) { String errMsg = "Error in ORO pattern substitution for SQL like clause [" + sqlLike + "]: " + t.toString(); Debug.logError(t, errMsg, module); throw new IllegalArgumentException(errMsg); } - try { - return compiler.compile(sqlLike); - } catch (MalformedPatternException e) { - e.printStackTrace(); - } - return null; + return sqlLike; } @Override @@ -276,7 +270,13 @@ public abstract class EntityComparisonOp } } else if (lhs instanceof String && rhs instanceof String) { //see if the lhs value is like the rhs value, rhs will have the pattern characters in it... - return matcher.matches((String) lhs, makeOroPattern((String) rhs)); + try { + return compilerMatcher.get().matches((String) lhs, makeOroPattern((String) rhs)); + } + catch (MalformedPatternException e) { + Debug.logError(e, module); + return false; + } } return true; } Modified: ofbiz/trunk/framework/minilang/build.xml URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/build.xml?rev=1075322&r1=1075321&r2=1075322&view=diff ============================================================================== --- ofbiz/trunk/framework/minilang/build.xml (original) +++ ofbiz/trunk/framework/minilang/build.xml Mon Feb 28 13:43:23 2011 @@ -40,4 +40,9 @@ under the License. <fileset dir="../security/build/lib" includes="*.jar"/> <fileset dir="../service/build/lib" includes="*.jar"/> </path> + + <target name="jar" depends="classes"> + <main-jar/> + <test-jar/> + </target> </project> Modified: ofbiz/trunk/framework/minilang/ofbiz-component.xml URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/ofbiz-component.xml?rev=1075322&r1=1075321&r2=1075322&view=diff ============================================================================== --- ofbiz/trunk/framework/minilang/ofbiz-component.xml (original) +++ ofbiz/trunk/framework/minilang/ofbiz-component.xml Mon Feb 28 13:43:23 2011 @@ -25,4 +25,6 @@ under the License. <classpath type="dir" location="config"/> <classpath type="dir" location="dtd"/> <classpath type="jar" location="build/lib/*"/> + + <test-suite loader="main" location="testdef/MinilangTests.xml"/> </ofbiz-component> Added: ofbiz/trunk/framework/minilang/script/org/ofbiz/minilang/method/ifops/IfRegexpTests.xml URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/script/org/ofbiz/minilang/method/ifops/IfRegexpTests.xml?rev=1075322&view=auto ============================================================================== --- ofbiz/trunk/framework/minilang/script/org/ofbiz/minilang/method/ifops/IfRegexpTests.xml (added) +++ ofbiz/trunk/framework/minilang/script/org/ofbiz/minilang/method/ifops/IfRegexpTests.xml Mon Feb 28 13:43:23 2011 @@ -0,0 +1,32 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +--> + +<simple-methods xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" + xsi:noNamespaceSchemaLocation="http://ofbiz.apache.org/dtds/simple-methods.xsd"> + + <simple-method method-name="testIfRegexp" short-description="Test case for race condition in if-regexp minilang op." login-required="false"> + <set field="foo" value="bar"/> + <if-regexp field="foo" expr="text.*"> + <set field="foo" value="bar'"/> + </if-regexp> + <return response-code="success"/> + </simple-method> + +</simple-methods> Propchange: ofbiz/trunk/framework/minilang/script/org/ofbiz/minilang/method/ifops/IfRegexpTests.xml ------------------------------------------------------------------------------ svn:mime-type = text/plain Modified: ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/conditional/RegexpCondition.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/conditional/RegexpCondition.java?rev=1075322&r1=1075321&r2=1075322&view=diff ============================================================================== --- ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/conditional/RegexpCondition.java (original) +++ ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/conditional/RegexpCondition.java Mon Feb 28 13:43:23 2011 @@ -24,11 +24,7 @@ import java.util.Map; import javolution.util.FastList; import org.apache.oro.text.regex.MalformedPatternException; -import org.apache.oro.text.regex.Pattern; -import org.apache.oro.text.regex.PatternCompiler; -import org.apache.oro.text.regex.PatternMatcher; -import org.apache.oro.text.regex.Perl5Compiler; -import org.apache.oro.text.regex.Perl5Matcher; +import org.ofbiz.base.util.CompilerMatcher; import org.ofbiz.base.util.Debug; import org.ofbiz.base.util.GeneralException; import org.ofbiz.base.util.ObjectType; @@ -59,8 +55,7 @@ public class RegexpCondition implements SimpleMethod simpleMethod; - static PatternMatcher matcher = new Perl5Matcher(); - static PatternCompiler compiler = new Perl5Compiler(); + private transient static ThreadLocal<CompilerMatcher> compilerMatcher = CompilerMatcher.getThreadLocal(); List<?> subOps = FastList.newInstance(); List<?> elseSubOps = null; @@ -87,14 +82,14 @@ public class RegexpCondition implements public boolean checkCondition(MethodContext methodContext) { String fieldString = getFieldString(methodContext); - Pattern pattern = null; + boolean matches = false; try { - pattern = compiler.compile(methodContext.expandString(this.exprExdr)); + matches = compilerMatcher.get().matches(fieldString, methodContext.expandString(this.exprExdr)); } catch (MalformedPatternException e) { Debug.logError(e, "Regular Expression [" + this.exprExdr + "] is mal-formed: " + e.toString(), module); } - if (matcher.matches(fieldString, pattern)) { + if (matches) { //Debug.logInfo("The string [" + fieldString + "] matched the pattern expr [" + pattern.getPattern() + "]", module); return true; } else { Modified: ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/ifops/IfRegexp.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/ifops/IfRegexp.java?rev=1075322&r1=1075321&r2=1075322&view=diff ============================================================================== --- ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/ifops/IfRegexp.java (original) +++ ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/ifops/IfRegexp.java Mon Feb 28 13:43:23 2011 @@ -18,17 +18,23 @@ *******************************************************************************/ package org.ofbiz.minilang.method.ifops; -import java.util.*; +import java.util.List; +import java.util.Map; import javolution.util.FastList; -import org.apache.oro.text.regex.*; -import org.w3c.dom.*; - -import org.ofbiz.base.util.*; +import org.apache.oro.text.regex.MalformedPatternException; +import org.ofbiz.base.util.CompilerMatcher; +import org.ofbiz.base.util.Debug; +import org.ofbiz.base.util.GeneralException; +import org.ofbiz.base.util.ObjectType; +import org.ofbiz.base.util.UtilXml; import org.ofbiz.base.util.string.FlexibleStringExpander; -import org.ofbiz.minilang.*; -import org.ofbiz.minilang.method.*; +import org.ofbiz.minilang.SimpleMethod; +import org.ofbiz.minilang.method.ContextAccessor; +import org.ofbiz.minilang.method.MethodContext; +import org.ofbiz.minilang.method.MethodOperation; +import org.w3c.dom.Element; /** * Iff the specified field complies with the pattern specified by the regular expression, process sub-operations @@ -46,8 +52,7 @@ public class IfRegexp extends MethodOper public static final String module = IfRegexp.class.getName(); - static PatternMatcher matcher = new Perl5Matcher(); - static PatternCompiler compiler = new Perl5Compiler(); + private transient static ThreadLocal<CompilerMatcher> compilerMatcher = CompilerMatcher.getThreadLocal(); List<MethodOperation> subOps = FastList.newInstance(); List<MethodOperation> elseSubOps = null; @@ -105,14 +110,14 @@ public class IfRegexp extends MethodOper // always use an empty string by default if (fieldString == null) fieldString = ""; - Pattern pattern = null; + boolean matches = false; try { - pattern = compiler.compile(methodContext.expandString(this.exprExdr)); + matches = compilerMatcher.get().matches(fieldString, methodContext.expandString(this.exprExdr)); } catch (MalformedPatternException e) { Debug.logError(e, "Regular Expression [" + this.exprExdr + "] is mal-formed: " + e.toString(), module); } - if (matcher.matches(fieldString, pattern)) { + if (matches) { return SimpleMethod.runSubOps(subOps, methodContext); } else { if (elseSubOps != null) { Added: ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/ifops/test/IfRegexpTest.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/ifops/test/IfRegexpTest.java?rev=1075322&view=auto ============================================================================== --- ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/ifops/test/IfRegexpTest.java (added) +++ ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/ifops/test/IfRegexpTest.java Mon Feb 28 13:43:23 2011 @@ -0,0 +1,87 @@ +/******************************************************************************* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + *******************************************************************************/ +package org.ofbiz.minilang.method.ifops.test; + +import java.util.Locale; +import org.ofbiz.base.util.Debug; +import org.ofbiz.base.util.UtilMisc; +import org.ofbiz.minilang.MiniLangException; +import org.ofbiz.minilang.SimpleMethod; +import org.ofbiz.minilang.method.MethodContext; +import org.ofbiz.service.LocalDispatcher; +import org.ofbiz.service.testtools.OFBizTestCase; + +public class IfRegexpTest extends OFBizTestCase { + + private static final String module = IfRegexpTest.class.getName(); + + public IfRegexpTest(String name) { + super(name); + } + + public void testIfRegexpThreadSafety() throws Exception { + MyThread t1 = new MyThread(null, dispatcher); + MyThread t2 = new MyThread(t1, dispatcher); + + t1.start(); + Thread.sleep(1000); + t2.start(); + + try { + t1.join(15000); + } catch (InterruptedException e) { + // if within 15 secs no problem has occurred, assume success + } + + assertTrue(t1.success); + assertTrue(t2.success); + } + + private static class MyThread extends Thread { + private boolean stopNow = false; + private boolean success = true; + private final MyThread friend; + private final LocalDispatcher dispatcher; + + public MyThread(MyThread friend, LocalDispatcher dispatcher) { + this.friend = friend; + this.dispatcher = dispatcher; + } + + @Override + public void run() { + int tryCount = 0; + while (!stopNow) { + MethodContext methodContext = new MethodContext(dispatcher.getDispatchContext(), UtilMisc.toMap("locale", Locale.getDefault()), null); + try { + tryCount++; + String responseCode = SimpleMethod.runSimpleMethod("component://minilang/script/org/ofbiz/minilang/method/ifops/IfRegexpTests.xml", "testIfRegexp", methodContext); + if (!"success".equals(methodContext.getEnv("responseMessage"))) { + success = false; + Debug.logError("ResponseCode not success: [" + responseCode + "], tryCount: [" + tryCount + "], envMap: [" + methodContext.getEnvMap() + "]", module); + if (friend != null) friend.stopNow = true; + break; + } + } catch (MiniLangException e) { + throw new RuntimeException(e); + } + } + }; + } +} Propchange: ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/ifops/test/IfRegexpTest.java ------------------------------------------------------------------------------ svn:mime-type = text/plain Modified: ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/operation/Regexp.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/operation/Regexp.java?rev=1075322&r1=1075321&r2=1075322&view=diff ============================================================================== --- ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/operation/Regexp.java (original) +++ ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/operation/Regexp.java Mon Feb 28 13:43:23 2011 @@ -18,12 +18,16 @@ *******************************************************************************/ package org.ofbiz.minilang.operation; -import java.util.*; - -import org.apache.oro.text.regex.*; -import org.w3c.dom.*; - -import org.ofbiz.base.util.*; +import java.util.List; +import java.util.Locale; +import java.util.Map; + +import org.apache.oro.text.regex.MalformedPatternException; +import org.ofbiz.base.util.CompilerMatcher; +import org.ofbiz.base.util.Debug; +import org.ofbiz.base.util.GeneralException; +import org.ofbiz.base.util.ObjectType; +import org.w3c.dom.Element; /** * Validates the current field using a regular expression @@ -32,19 +36,13 @@ public class Regexp extends SimpleMapOpe public static final String module = Regexp.class.getName(); - static PatternMatcher matcher = new Perl5Matcher(); - static PatternCompiler compiler = new Perl5Compiler(); - Pattern pattern = null; + private transient static ThreadLocal<CompilerMatcher> compilerMatcher = CompilerMatcher.getThreadLocal(); + String expr; public Regexp(Element element, SimpleMapProcess simpleMapProcess) { super(element, simpleMapProcess); expr = element.getAttribute("expr"); - try { - pattern = compiler.compile(expr); - } catch (MalformedPatternException e) { - Debug.logError(e, module); - } } @Override @@ -60,12 +58,15 @@ public class Regexp extends SimpleMapOpe return; } - if (pattern == null) { - messages.add("Could not compile regular expression \"" + expr + "\" for validation"); + boolean matches = false; + try { + matches = compilerMatcher.get().matches(fieldValue, expr); + } catch (MalformedPatternException e) { + Debug.logError(e, "Regular Expression [" + this.expr + "] is mal-formed: " + e.toString(), module); return; } - if (!matcher.matches(fieldValue, pattern)) { + if (!matches) { addMessage(messages, loader, locale); } } Added: ofbiz/trunk/framework/minilang/testdef/MinilangTests.xml URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/testdef/MinilangTests.xml?rev=1075322&view=auto ============================================================================== --- ofbiz/trunk/framework/minilang/testdef/MinilangTests.xml (added) +++ ofbiz/trunk/framework/minilang/testdef/MinilangTests.xml Mon Feb 28 13:43:23 2011 @@ -0,0 +1,9 @@ +<test-suite suite-name="MinilangTests" + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" + xsi:noNamespaceSchemaLocation="http://ofbiz.apache.org/dtds/test-suite.xsd"> + + <test-case case-name="IfRegexpTest"> + <junit-test-suite class-name="org.ofbiz.minilang.method.ifops.test.IfRegexpTest"/> + </test-case> + +</test-suite> Propchange: ofbiz/trunk/framework/minilang/testdef/MinilangTests.xml ------------------------------------------------------------------------------ svn:mime-type = text/plain Modified: ofbiz/trunk/framework/widget/src/org/ofbiz/widget/form/ModelFormField.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/form/ModelFormField.java?rev=1075322&r1=1075321&r2=1075322&view=diff ============================================================================== --- ofbiz/trunk/framework/widget/src/org/ofbiz/widget/form/ModelFormField.java (original) +++ ofbiz/trunk/framework/widget/src/org/ofbiz/widget/form/ModelFormField.java Mon Feb 28 13:43:23 2011 @@ -20,8 +20,10 @@ package org.ofbiz.widget.form; import java.io.IOException; import java.math.BigDecimal; +import java.sql.Timestamp; import java.text.DateFormat; import java.text.NumberFormat; +import java.util.Date; import java.util.HashMap; import java.util.LinkedList; import java.util.List; @@ -33,6 +35,9 @@ import java.util.TimeZone; import javolution.util.FastList; import javolution.util.FastMap; +import org.ofbiz.base.conversion.ConversionException; +import org.ofbiz.base.conversion.DateTimeConverters; +import org.ofbiz.base.conversion.DateTimeConverters.StringToTimestamp; import org.ofbiz.base.util.BshUtil; import org.ofbiz.base.util.Debug; import org.ofbiz.base.util.GeneralException; @@ -2150,9 +2155,52 @@ public class ModelFormField { throw new IllegalArgumentException(errMsg); } } else if ("date".equals(this.type) && retVal.length() > 10) { - retVal = retVal.substring(0,10); + Locale locale = (Locale) context.get("locale"); + if (locale == null) { + locale = Locale.getDefault(); + } + + StringToTimestamp stringToTimestamp = new DateTimeConverters.StringToTimestamp(); + Timestamp timestamp = null; + try { + timestamp = stringToTimestamp.convert(retVal); + Date date = new Date(timestamp.getTime()); + + DateFormat dateFormatter = DateFormat.getDateInstance(DateFormat.SHORT, locale); + retVal = dateFormatter.format(date); + } + catch (ConversionException e) { + String errMsg = "Error formatting date using default instead [" + retVal + "]: " + e.toString(); + Debug.logError(e, errMsg, module); + // create default date value from timestamp string + retVal = retVal.substring(0,10); + } + } else if ("date-time".equals(this.type) && retVal.length() > 16) { - retVal = retVal.substring(0,16); + Locale locale = (Locale) context.get("locale"); + TimeZone timeZone = (TimeZone) context.get("timeZone"); + if (locale == null) { + locale = Locale.getDefault(); + } + if (timeZone == null) { + timeZone = TimeZone.getDefault(); + } + + StringToTimestamp stringToTimestamp = new DateTimeConverters.StringToTimestamp(); + Timestamp timestamp = null; + try { + timestamp = stringToTimestamp.convert(retVal); + Date date = new Date(timestamp.getTime()); + + DateFormat dateFormatter = UtilDateTime.toDateTimeFormat(null, timeZone, locale); + retVal = dateFormatter.format(date); + } + catch (ConversionException e) { + String errMsg = "Error formatting date/time using default instead [" + retVal + "]: " + e.toString(); + Debug.logError(e, errMsg, module); + // create default date/time value from timestamp string + retVal = retVal.substring(0,16); + } } else if ("accounting-number".equals(this.type)) { Locale locale = (Locale) context.get("locale"); if (locale == null) { Modified: ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenuCondition.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenuCondition.java?rev=1075322&r1=1075321&r2=1075322&view=diff ============================================================================== --- ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenuCondition.java (original) +++ ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenuCondition.java Mon Feb 28 13:43:23 2011 @@ -27,11 +27,7 @@ import java.util.TimeZone; import javolution.util.FastList; import org.apache.oro.text.regex.MalformedPatternException; -import org.apache.oro.text.regex.Pattern; -import org.apache.oro.text.regex.PatternCompiler; -import org.apache.oro.text.regex.PatternMatcher; -import org.apache.oro.text.regex.Perl5Compiler; -import org.apache.oro.text.regex.Perl5Matcher; +import org.ofbiz.base.util.CompilerMatcher; import org.ofbiz.base.util.Debug; import org.ofbiz.base.util.GeneralException; import org.ofbiz.base.util.ObjectType; @@ -497,8 +493,7 @@ public class ModelMenuCondition { } public static class IfRegexp extends MenuCondition { - static PatternMatcher matcher = new Perl5Matcher(); - static PatternCompiler compiler = new Perl5Compiler(); + private transient static ThreadLocal<CompilerMatcher> compilerMatcher = CompilerMatcher.getThreadLocal(); protected FlexibleMapAccessor<Object> fieldAcsr; protected FlexibleStringExpander exprExdr; @@ -513,15 +508,6 @@ public class ModelMenuCondition { @Override public boolean eval(Map<String, Object> context) { Object fieldVal = this.fieldAcsr.get(context); - String expr = this.exprExdr.expandString(context); - Pattern pattern = null; - try { - pattern = compiler.compile(expr); - } catch (MalformedPatternException e) { - String errMsg = "Error in evaluation in if-regexp in screen: " + e.toString(); - Debug.logError(e, errMsg, module); - throw new IllegalArgumentException(errMsg); - } String fieldString = null; try { @@ -532,7 +518,13 @@ public class ModelMenuCondition { // always use an empty string by default if (fieldString == null) fieldString = ""; - return matcher.matches(fieldString, pattern); + try { + return compilerMatcher.get().matches(fieldString, this.exprExdr.expandString(context)); + } catch (MalformedPatternException e) { + String errMsg = "Error in evaluation in if-regexp in screen: " + e.toString(); + Debug.logError(e, errMsg, module); + throw new IllegalArgumentException(errMsg); + } } } Modified: ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/ModelScreenCondition.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/ModelScreenCondition.java?rev=1075322&r1=1075321&r2=1075322&view=diff ============================================================================== --- ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/ModelScreenCondition.java (original) +++ ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/ModelScreenCondition.java Mon Feb 28 13:43:23 2011 @@ -28,11 +28,7 @@ import java.util.TimeZone; import javolution.util.FastList; import org.apache.oro.text.regex.MalformedPatternException; -import org.apache.oro.text.regex.Pattern; -import org.apache.oro.text.regex.PatternCompiler; -import org.apache.oro.text.regex.PatternMatcher; -import org.apache.oro.text.regex.Perl5Compiler; -import org.apache.oro.text.regex.Perl5Matcher; +import org.ofbiz.base.util.CompilerMatcher; import org.ofbiz.base.util.Debug; import org.ofbiz.base.util.GeneralException; import org.ofbiz.base.util.ObjectType; @@ -495,8 +491,7 @@ public class ModelScreenCondition implem } public static class IfRegexp extends ScreenCondition { - static PatternMatcher matcher = new Perl5Matcher(); - static PatternCompiler compiler = new Perl5Compiler(); + private transient static ThreadLocal<CompilerMatcher> compilerMatcher = CompilerMatcher.getThreadLocal(); protected FlexibleMapAccessor<Object> fieldAcsr; protected FlexibleStringExpander exprExdr; @@ -511,15 +506,6 @@ public class ModelScreenCondition implem @Override public boolean eval(Map<String, Object> context) { Object fieldVal = this.fieldAcsr.get(context); - String expr = this.exprExdr.expandString(context); - Pattern pattern; - try { - pattern = compiler.compile(expr); - } catch (MalformedPatternException e) { - String errMsg = "Error in evaluation in if-regexp in screen: " + e.toString(); - Debug.logError(e, errMsg, module); - throw new IllegalArgumentException(errMsg); - } String fieldString = null; try { @@ -530,7 +516,13 @@ public class ModelScreenCondition implem // always use an empty string by default if (fieldString == null) fieldString = ""; - return matcher.matches(fieldString, pattern); + try { + return compilerMatcher.get().matches(fieldString, this.exprExdr.expandString(context)); + } catch (MalformedPatternException e) { + String errMsg = "Error in evaluation in if-regexp in screen: " + e.toString(); + Debug.logError(e, errMsg, module); + throw new IllegalArgumentException(errMsg); + } } } Modified: ofbiz/trunk/framework/widget/src/org/ofbiz/widget/tree/ModelTreeCondition.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/tree/ModelTreeCondition.java?rev=1075322&r1=1075321&r2=1075322&view=diff ============================================================================== --- ofbiz/trunk/framework/widget/src/org/ofbiz/widget/tree/ModelTreeCondition.java (original) +++ ofbiz/trunk/framework/widget/src/org/ofbiz/widget/tree/ModelTreeCondition.java Mon Feb 28 13:43:23 2011 @@ -27,11 +27,7 @@ import java.util.TimeZone; import javolution.util.FastList; import org.apache.oro.text.regex.MalformedPatternException; -import org.apache.oro.text.regex.Pattern; -import org.apache.oro.text.regex.PatternCompiler; -import org.apache.oro.text.regex.PatternMatcher; -import org.apache.oro.text.regex.Perl5Compiler; -import org.apache.oro.text.regex.Perl5Matcher; +import org.ofbiz.base.util.CompilerMatcher; import org.ofbiz.base.util.Debug; import org.ofbiz.base.util.GeneralException; import org.ofbiz.base.util.ObjectType; @@ -398,8 +394,7 @@ public class ModelTreeCondition { } public static class IfRegexp extends TreeCondition { - static PatternMatcher matcher = new Perl5Matcher(); - static PatternCompiler compiler = new Perl5Compiler(); + private transient static ThreadLocal<CompilerMatcher> compilerMatcher = CompilerMatcher.getThreadLocal(); protected FlexibleMapAccessor<Object> fieldAcsr; protected FlexibleStringExpander exprExdr; @@ -414,15 +409,6 @@ public class ModelTreeCondition { @Override public boolean eval(Map<String, ? extends Object> context) { Object fieldVal = this.fieldAcsr.get(context); - String expr = this.exprExdr.expandString(context); - Pattern pattern = null; - try { - pattern = compiler.compile(expr); - } catch (MalformedPatternException e) { - String errMsg = "Error in evaluation in if-regexp in screen: " + e.toString(); - Debug.logError(e, errMsg, module); - throw new IllegalArgumentException(errMsg); - } String fieldString = null; try { @@ -433,7 +419,13 @@ public class ModelTreeCondition { // always use an empty string by default if (fieldString == null) fieldString = ""; - return matcher.matches(fieldString, pattern); + try { + return compilerMatcher.get().matches(fieldString, this.exprExdr.expandString(context)); + } catch (MalformedPatternException e) { + String errMsg = "Error in evaluation in if-regexp in screen: " + e.toString(); + Debug.logError(e, errMsg, module); + throw new IllegalArgumentException(errMsg); + } } } |
Free forum by Nabble | Edit this page |