Return-Path: Delivered-To: apmail-struts-dev-archive@www.apache.org Received: (qmail 20411 invoked from network); 6 Jul 2007 14:59:41 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2) by minotaur.apache.org with SMTP; 6 Jul 2007 14:59:41 -0000 Received: (qmail 927 invoked by uid 500); 6 Jul 2007 03:46:23 -0000 Delivered-To: apmail-struts-dev-archive@struts.apache.org Received: (qmail 883 invoked by uid 500); 6 Jul 2007 03:46:23 -0000 Mailing-List: contact dev-help@struts.apache.org; run by ezmlm Precedence: bulk List-Unsubscribe: List-Help: List-Post: List-Id: "Struts Developers List" Reply-To: "Struts Developers List" Delivered-To: mailing list dev@struts.apache.org Received: (qmail 871 invoked by uid 99); 6 Jul 2007 03:46:23 -0000 Received: from herse.apache.org (HELO herse.apache.org) (140.211.11.133) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 05 Jul 2007 20:46:23 -0700 X-ASF-Spam-Status: No, hits=-0.0 required=10.0 tests=SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (herse.apache.org: domain of paulus.benedictus@gmail.com designates 64.233.166.183 as permitted sender) Received: from [64.233.166.183] (HELO py-out-1112.google.com) (64.233.166.183) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 05 Jul 2007 20:46:17 -0700 Received: by py-out-1112.google.com with SMTP id p76so174091pyb for ; Thu, 05 Jul 2007 20:45:56 -0700 (PDT) DKIM-Signature: a=rsa-sha1; c=relaxed/relaxed; d=gmail.com; s=beta; h=domainkey-signature:received:received:message-id:date:from:reply-to:user-agent:mime-version:to:subject:references:in-reply-to:content-type:content-transfer-encoding:sender; b=RJsDpm170z9wCrF5zNX7V4LzxSh8XAMez5Xk939R1PgfBI4So8DPKEOJHgXsm3u4bWIsxQKDO1FfD3EcBT6WGO0R+3txwK0t5s2Sb2npeB/CCx1TrsXcI4Z6k4wBgkbN32KW3/efrugSTTWmgNL1mmvZinNBniAgn9Ov0TA9kH8= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=beta; h=received:message-id:date:from:reply-to:user-agent:mime-version:to:subject:references:in-reply-to:content-type:content-transfer-encoding:sender; b=Y+fxLcNG8+2EnMjRpL9d05nWCTqW9wQPp+zAma4UAaR5Kwv8miEQYfUABWxxPH3+8ANhP2+/rEV3iTF62EStvdO+XNfmE3HsnlyyPDpVXUNF29ugsabsAKJDNpvgOhgvzMkNB+vnmqWwkxOjgTMKpW/Y02OSsktts7/3gUu0fMs= Received: by 10.35.131.13 with SMTP id i13mr544391pyn.1183693556508; Thu, 05 Jul 2007 20:45:56 -0700 (PDT) Received: from ?192.168.11.3? ( [65.30.71.168]) by mx.google.com with ESMTP id 37sm6055793nzf.2007.07.05.20.45.54 (version=TLSv1/SSLv3 cipher=RC4-MD5); Thu, 05 Jul 2007 20:45:55 -0700 (PDT) Message-ID: <468DBAF7.8020305@apache.org> Date: Thu, 05 Jul 2007 22:45:59 -0500 From: Paul Benedict Reply-To: pbenedict@apache.org User-Agent: Thunderbird 2.0.0.4 (Windows/20070604) MIME-Version: 1.0 To: Struts Developers List Subject: Re: svn commit: r552390 - in /struts/struts1/trunk/core/src/main/java/org/apache/struts/validator: FieldChecks.java LocalStrings.properties References: <20070702031923.D91E21A981A@eris.apache.org> <55afdc850707052022w11ad8c2v718fa492f523a43e@mail.gmail.com> In-Reply-To: <55afdc850707052022w11ad8c2v718fa492f523a43e@mail.gmail.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: Paul Benedict X-Virus-Checked: Checked by ClamAV on apache.org Good point. Niall Pemberton wrote: > STR-2611 just requested more info in the logged error message - but > this change goes further because in all cases except the required > validator validation will now fail if an exception is throw accessing > the property's value - whereas before it just skipped validation. For > anyone that has relied on that behaviour then its going to break their > application. > > Niall > > On 7/2/07, pbenedict@apache.org wrote: >> Author: pbenedict >> Date: Sun Jul 1 20:19:22 2007 >> New Revision: 552390 >> >> URL: http://svn.apache.org/viewvc?view=rev&rev=552390 >> Log: >> STR-2611: Log form name and field when accessing property fails >> >> Modified: >> >> struts/struts1/trunk/core/src/main/java/org/apache/struts/validator/FieldChecks.java >> >> >> struts/struts1/trunk/core/src/main/java/org/apache/struts/validator/LocalStrings.properties >> >> >> Modified: >> struts/struts1/trunk/core/src/main/java/org/apache/struts/validator/FieldChecks.java >> >> URL: >> http://svn.apache.org/viewvc/struts/struts1/trunk/core/src/main/java/org/apache/struts/validator/FieldChecks.java?view=diff&rev=552390&r1=552389&r2=552390 >> >> ============================================================================== >> >> --- >> struts/struts1/trunk/core/src/main/java/org/apache/struts/validator/FieldChecks.java >> (original) >> +++ >> struts/struts1/trunk/core/src/main/java/org/apache/struts/validator/FieldChecks.java >> Sun Jul 1 20:19:22 2007 >> @@ -20,6 +20,7 @@ >> */ >> package org.apache.struts.validator; >> >> +import org.apache.commons.beanutils.PropertyUtils; >> import org.apache.commons.logging.Log; >> import org.apache.commons.logging.LogFactory; >> import org.apache.commons.validator.Field; >> @@ -37,7 +38,7 @@ >> import javax.servlet.http.HttpServletRequest; >> >> import java.io.Serializable; >> - >> +import java.util.Collection; >> import java.util.Locale; >> import java.util.StringTokenizer; >> >> @@ -66,6 +67,43 @@ >> public static final String FIELD_TEST_EQUAL = "EQUAL"; >> >> /** >> + * Convenience method for getting a value from a bean property as a >> + * String. If the property is a >> String[] or >> + * Collection and it is empty, an empty >> String >> + * "" is returned. Otherwise, property.toString() is returned. >> This method >> + * may return null if there was an error retrieving >> the >> + * property. >> + *

>> + * NOTE: This method is a port from Commons Validator >> + * ValidatorUtils because the original version >> swallows >> + * exceptions and thus cannot indicate to the caller that the bean >> + * property was invalid. This version will throw an exception. >> + * >> + * @param bean The bean object. >> + * @param property The name of the property to access. >> + * @return The value of the property. >> + * @throws Exception if an error occurs retrieving the property >> + */ >> + private static String getValueAsString(Object bean, String >> property) >> + throws Exception { >> + >> + Object value = PropertyUtils.getProperty(bean, property); >> + if (value == null) { >> + return null; >> + } >> + >> + if (value instanceof String[]) { >> + return ((String[]) value).length > 0 ? value.toString() >> : ""; >> + >> + } else if (value instanceof Collection) { >> + return ((Collection) value).isEmpty() ? "" : >> value.toString(); >> + >> + } else { >> + return value.toString(); >> + } >> + } >> + >> + /** >> * Checks if the field isn't null and length of the field is >> greater than >> * zero not including whitespace. >> * >> @@ -86,7 +124,12 @@ >> HttpServletRequest request) { >> String value = null; >> >> - value = evaluateBean(bean, field); >> + try { >> + value = evaluateBean(bean, field); >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "required", e); >> + return false; >> + } >> >> if (GenericValidator.isBlankOrNull(value)) { >> errors.add(field.getKey(), >> @@ -121,7 +164,12 @@ >> String value = null; >> boolean required = false; >> >> - value = evaluateBean(bean, field); >> + try { >> + value = evaluateBean(bean, field); >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "requiredif", e); >> + return false; >> + } >> >> int i = 0; >> String fieldJoin = "AND"; >> @@ -224,9 +272,9 @@ >> HttpServletRequest request) { >> String value = null; >> >> - value = evaluateBean(bean, field); >> - >> try { >> + value = evaluateBean(bean, field); >> + >> String mask = >> Resources.getVarValue("mask", field, validator, >> request, true); >> >> @@ -267,7 +315,12 @@ >> Object result = null; >> String value = null; >> >> - value = evaluateBean(bean, field); >> + try { >> + value = evaluateBean(bean, field); >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "byte", e); >> + return Boolean.FALSE; >> + } >> >> if (GenericValidator.isBlankOrNull(value)) { >> return Boolean.TRUE; >> @@ -304,7 +357,12 @@ >> Object result = null; >> String value = null; >> >> - value = evaluateBean(bean, field); >> + try { >> + value = evaluateBean(bean, field); >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "byteLocale", e); >> + return Boolean.FALSE; >> + } >> >> if (GenericValidator.isBlankOrNull(value)) { >> return Boolean.TRUE; >> @@ -325,15 +383,16 @@ >> /** >> * @param bean >> * @param field >> - * @return >> + * @return the >> + * @throws Exception if an error occurs accessing the bean property >> */ >> - private static String evaluateBean(Object bean, Field field) { >> + private static String evaluateBean(Object bean, Field field) >> throws Exception { >> String value; >> >> if (isString(bean)) { >> value = (String) bean; >> } else { >> - value = ValidatorUtils.getValueAsString(bean, >> field.getProperty()); >> + value = getValueAsString(bean, field.getProperty()); >> } >> >> return value; >> @@ -360,7 +419,12 @@ >> Object result = null; >> String value = null; >> >> - value = evaluateBean(bean, field); >> + try { >> + value = evaluateBean(bean, field); >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "short", e); >> + return Boolean.FALSE; >> + } >> >> if (GenericValidator.isBlankOrNull(value)) { >> return Boolean.TRUE; >> @@ -397,7 +461,12 @@ >> Object result = null; >> String value = null; >> >> - value = evaluateBean(bean, field); >> + try { >> + value = evaluateBean(bean, field); >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "shortLocale", e); >> + return Boolean.FALSE; >> + } >> >> if (GenericValidator.isBlankOrNull(value)) { >> return Boolean.TRUE; >> @@ -436,7 +505,12 @@ >> Object result = null; >> String value = null; >> >> - value = evaluateBean(bean, field); >> + try { >> + value = evaluateBean(bean, field); >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "integer", e); >> + return Boolean.FALSE; >> + } >> >> if (GenericValidator.isBlankOrNull(value)) { >> return Boolean.TRUE; >> @@ -473,7 +547,12 @@ >> Object result = null; >> String value = null; >> >> - value = evaluateBean(bean, field); >> + try { >> + value = evaluateBean(bean, field); >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "integerLocale", e); >> + return Boolean.FALSE; >> + } >> >> if (GenericValidator.isBlankOrNull(value)) { >> return Boolean.TRUE; >> @@ -512,7 +591,12 @@ >> Object result = null; >> String value = null; >> >> - value = evaluateBean(bean, field); >> + try { >> + value = evaluateBean(bean, field); >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "long", e); >> + return Boolean.FALSE; >> + } >> >> if (GenericValidator.isBlankOrNull(value)) { >> return Boolean.TRUE; >> @@ -549,7 +633,12 @@ >> Object result = null; >> String value = null; >> >> - value = evaluateBean(bean, field); >> + try { >> + value = evaluateBean(bean, field); >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "longLocale", e); >> + return Boolean.FALSE; >> + } >> >> if (GenericValidator.isBlankOrNull(value)) { >> return Boolean.TRUE; >> @@ -588,7 +677,12 @@ >> Object result = null; >> String value = null; >> >> - value = evaluateBean(bean, field); >> + try { >> + value = evaluateBean(bean, field); >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "float", e); >> + return Boolean.FALSE; >> + } >> >> if (GenericValidator.isBlankOrNull(value)) { >> return Boolean.TRUE; >> @@ -625,7 +719,12 @@ >> Object result = null; >> String value = null; >> >> - value = evaluateBean(bean, field); >> + try { >> + value = evaluateBean(bean, field); >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "floatLocale", e); >> + return Boolean.FALSE; >> + } >> >> if (GenericValidator.isBlankOrNull(value)) { >> return Boolean.TRUE; >> @@ -664,7 +763,12 @@ >> Object result = null; >> String value = null; >> >> - value = evaluateBean(bean, field); >> + try { >> + value = evaluateBean(bean, field); >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "double", e); >> + return Boolean.FALSE; >> + } >> >> if (GenericValidator.isBlankOrNull(value)) { >> return Boolean.TRUE; >> @@ -701,7 +805,12 @@ >> Object result = null; >> String value = null; >> >> - value = evaluateBean(bean, field); >> + try { >> + value = evaluateBean(bean, field); >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "doubleLocale", e); >> + return Boolean.FALSE; >> + } >> >> if (GenericValidator.isBlankOrNull(value)) { >> return Boolean.TRUE; >> @@ -748,7 +857,12 @@ >> Object result = null; >> String value = null; >> >> - value = evaluateBean(bean, field); >> + try { >> + value = evaluateBean(bean, field); >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "date", e); >> + return Boolean.FALSE; >> + } >> >> boolean isStrict = false; >> String datePattern = >> @@ -811,10 +925,9 @@ >> HttpServletRequest request) { >> String value = null; >> >> - value = evaluateBean(bean, field); >> - >> - if (!GenericValidator.isBlankOrNull(value)) { >> - try { >> + try { >> + value = evaluateBean(bean, field); >> + if (!GenericValidator.isBlankOrNull(value)) { >> String minVar = >> Resources.getVarValue("min", field, validator, >> request, true); >> String maxVar = >> @@ -822,23 +935,23 @@ >> long longValue = Long.parseLong(value); >> long min = Long.parseLong(minVar); >> long max = Long.parseLong(maxVar); >> - >> + >> if (min > max) { >> throw new >> IllegalArgumentException(sysmsgs.getMessage( >> "invalid.range", minVar, maxVar)); >> } >> - >> + >> if (!GenericValidator.isInRange(longValue, min, max)) { >> errors.add(field.getKey(), >> Resources.getActionMessage(validator, >> request, va, field)); >> - >> + >> return false; >> } >> - } catch (Exception e) { >> - processFailure(errors, field, >> validator.getFormName(), "longRange", e); >> - >> - return false; >> } >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "longRange", e); >> + >> + return false; >> } >> >> return true; >> @@ -865,10 +978,9 @@ >> HttpServletRequest request) { >> String value = null; >> >> - value = evaluateBean(bean, field); >> - >> - if (!GenericValidator.isBlankOrNull(value)) { >> - try { >> + try { >> + value = evaluateBean(bean, field); >> + if (!GenericValidator.isBlankOrNull(value)) { >> String minVar = >> Resources.getVarValue("min", field, validator, >> request, true); >> String maxVar = >> @@ -888,11 +1000,11 @@ >> >> return false; >> } >> - } catch (Exception e) { >> - processFailure(errors, field, >> validator.getFormName(), "intRange", e); >> - >> - return false; >> } >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "intRange", e); >> + >> + return false; >> } >> >> return true; >> @@ -919,10 +1031,9 @@ >> HttpServletRequest request) { >> String value = null; >> >> - value = evaluateBean(bean, field); >> - >> - if (!GenericValidator.isBlankOrNull(value)) { >> - try { >> + try { >> + value = evaluateBean(bean, field); >> + if (!GenericValidator.isBlankOrNull(value)) { >> String minVar = >> Resources.getVarValue("min", field, validator, >> request, true); >> String maxVar = >> @@ -942,11 +1053,11 @@ >> >> return false; >> } >> - } catch (Exception e) { >> - processFailure(errors, field, >> validator.getFormName(), "doubleRange", e); >> - >> - return false; >> } >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "doubleRange", e); >> + >> + return false; >> } >> >> return true; >> @@ -973,10 +1084,9 @@ >> HttpServletRequest request) { >> String value = null; >> >> - value = evaluateBean(bean, field); >> - >> - if (!GenericValidator.isBlankOrNull(value)) { >> - try { >> + try { >> + value = evaluateBean(bean, field); >> + if (!GenericValidator.isBlankOrNull(value)) { >> String minVar = >> Resources.getVarValue("min", field, validator, >> request, true); >> String maxVar = >> @@ -984,23 +1094,23 @@ >> float floatValue = Float.parseFloat(value); >> float min = Float.parseFloat(minVar); >> float max = Float.parseFloat(maxVar); >> - >> + >> if (min > max) { >> throw new >> IllegalArgumentException(sysmsgs.getMessage( >> "invalid.range", minVar, maxVar)); >> } >> - >> + >> if (!GenericValidator.isInRange(floatValue, min, >> max)) { >> errors.add(field.getKey(), >> Resources.getActionMessage(validator, >> request, va, field)); >> - >> + >> return false; >> } >> - } catch (Exception e) { >> - processFailure(errors, field, >> validator.getFormName(), "floatRange", e); >> - >> - return false; >> } >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "floatRange", e); >> + >> + return false; >> } >> >> return true; >> @@ -1027,7 +1137,12 @@ >> Object result = null; >> String value = null; >> >> - value = evaluateBean(bean, field); >> + try { >> + value = evaluateBean(bean, field); >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "creditCard", e); >> + return Boolean.FALSE; >> + } >> >> if (GenericValidator.isBlankOrNull(value)) { >> return Boolean.TRUE; >> @@ -1063,7 +1178,12 @@ >> HttpServletRequest request) { >> String value = null; >> >> - value = evaluateBean(bean, field); >> + try { >> + value = evaluateBean(bean, field); >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "email", e); >> + return false; >> + } >> >> if (!GenericValidator.isBlankOrNull(value) >> && !GenericValidator.isEmail(value)) { >> @@ -1097,10 +1217,9 @@ >> HttpServletRequest request) { >> String value = null; >> >> - value = evaluateBean(bean, field); >> - >> - if (value != null) { >> - try { >> + try { >> + value = evaluateBean(bean, field); >> + if (value != null) { >> String maxVar = >> Resources.getVarValue("maxlength", field, >> validator, >> request, true); >> @@ -1122,11 +1241,11 @@ >> >> return false; >> } >> - } catch (Exception e) { >> - processFailure(errors, field, >> validator.getFormName(), "maxlength", e); >> - >> - return false; >> } >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "maxlength", e); >> + >> + return false; >> } >> >> return true; >> @@ -1153,10 +1272,9 @@ >> HttpServletRequest request) { >> String value = null; >> >> - value = evaluateBean(bean, field); >> - >> - if (!GenericValidator.isBlankOrNull(value)) { >> - try { >> + try { >> + value = evaluateBean(bean, field); >> + if (!GenericValidator.isBlankOrNull(value)) { >> String minVar = >> Resources.getVarValue("minlength", field, >> validator, >> request, true); >> @@ -1178,11 +1296,11 @@ >> >> return false; >> } >> - } catch (Exception e) { >> - processFailure(errors, field, >> validator.getFormName(), "minlength", e); >> - >> - return false; >> } >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "minlength", e); >> + >> + return false; >> } >> >> return true; >> @@ -1231,7 +1349,12 @@ >> HttpServletRequest request) { >> String value = null; >> >> - value = evaluateBean(bean, field); >> + try { >> + value = evaluateBean(bean, field); >> + } catch (Exception e) { >> + processFailure(errors, field, validator.getFormName(), >> "url", e); >> + return false; >> + } >> >> if (GenericValidator.isBlankOrNull(value)) { >> return true; >> @@ -1315,7 +1438,7 @@ >> sysmsgs.getMessage("validation.failed", validatorName, >> field.getProperty(), formName, t.toString()); >> >> - log.error(logErrorMsg); >> + log.error(logErrorMsg, t); >> >> // Add general "system error" message to show to the user >> String userErrorMsg = sysmsgs.getMessage("system.error"); >> >> Modified: >> struts/struts1/trunk/core/src/main/java/org/apache/struts/validator/LocalStrings.properties >> >> URL: >> http://svn.apache.org/viewvc/struts/struts1/trunk/core/src/main/java/org/apache/struts/validator/LocalStrings.properties?view=diff&rev=552390&r1=552389&r2=552390 >> >> ============================================================================== >> >> --- >> struts/struts1/trunk/core/src/main/java/org/apache/struts/validator/LocalStrings.properties >> (original) >> +++ >> struts/struts1/trunk/core/src/main/java/org/apache/struts/validator/LocalStrings.properties >> Sun Jul 1 20:19:22 2007 >> @@ -14,7 +14,7 @@ >> # limitations under the License. >> >> system.error=SYSTEM ERROR: Check logs for details. >> -validation.failed={0} validation failed for property {1} of form key >> {2}: {3} >> +validation.failed={0} validation failed for property \'{1}\' of form >> key \'{2}\': {3} >> variable.missing=Variable {0} is missing. >> variable.resource.notfound=Key {1} not found for Variable {0} in >> bundle {2}. >> invalid.range=Minimum value {0} is greater than maximum value {1} >> >> >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org > For additional commands, e-mail: dev-help@struts.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org For additional commands, e-mail: dev-help@struts.apache.org