It depends on what required means here. I committed those because Matcher class is not thread safe.
I must say I did not cross any issues before doing these changes. They are more thread safe issues prevention.
So I believe it's safer to do it this way. Though I'm not quite sure pattern.matcher is atomic. So maybe using synchronized could be required to guarantee thread safe.
But contrary to the change I did in ProductEvents.java and changed again at r1541746, synchronized is not necessary here (nor in other changes I did in r1541641)
It's the same behaviour (compiled pattern is not reused) than before whith a chance that pattern.matcher is atomic, thus prevents possible issues.
> Are these changes required? Can we revert them?
>
> Jacopo
>
>
> On Nov 13, 2013, at 7:04 PM,
[hidden email] wrote:
>
>> Modified: ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java
>> URL:
>>
http://svn.apache.org/viewvc/ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java?rev=1541641&r1=1541640&r2=1541641&view=diff>> ============================================================================== ---
>> ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java (original) +++
>> ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java Wed Nov 13 18:04:14 2013 @@ -23,7 +23,6 @@ import
>> java.sql.Timestamp;
>> import java.util.List;
>> import java.util.Locale;
>> import java.util.Map;
>> -import java.util.regex.Matcher;
>> import java.util.regex.Pattern;
>>
>> import javax.transaction.Transaction;
>> @@ -957,9 +956,7 @@ public class LoginServices {
>> boolean usePasswordPattern = UtilProperties.getPropertyAsBoolean("security.properties",
>> "security.login.password.pattern.enable", true); if (usePasswordPattern) {
>> Pattern pattern = Pattern.compile(passwordPattern);
>> - Matcher matcher = pattern.matcher(newPassword);
>> - boolean matched = matcher.matches();
>> - if (!matched) {
>> + if (!pattern.matcher(newPassword).matches()) {
>> // This is a mix to handle the OOTB pattern which is only a fixed length
>> Map<String, String> messageMap = UtilMisc.toMap("minPasswordLength", Integer.toString(minPasswordLength));
>> String passwordPatternMessage = UtilProperties.getPropertyValue("security.properties",
>>
>> Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/LoginWorker.java
>> URL:
>>
http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/LoginWorker.java?rev=1541641&r1=1541640&r2=1541641&view=diff>> ============================================================================== ---
>> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/LoginWorker.java (original) +++
>> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/LoginWorker.java Wed Nov 13 18:04:14 2013 @@ -26,7 +26,6 @@ import
>> java.sql.Timestamp;
>> import java.util.List;
>> import java.util.Map;
>> import java.util.ServiceLoader;
>> -import java.util.regex.Matcher;
>> import java.util.regex.Pattern;
>>
>> import javax.servlet.ServletContext;
>> @@ -925,9 +924,8 @@ public class LoginWorker {
>> if (i == 0) {
>> String cn = x500Map.get("CN");
>> cn = cn.replaceAll("\\\\", "");
>> - Matcher m = pattern.matcher(cn);
>> - if (m.matches()) {
>> - userLoginId = m.group(1);
>> + if (pattern.matcher(cn).matches()) {
>> + userLoginId = pattern.matcher(cn).group(1);
>> } else {
>> Debug.logInfo("Client certificate CN does not match pattern: [" + cnPattern + "]", module);
>> }
Locked
