Stephen,

What means David is that UtilValidate.isDate(), and its variants isDateBeforeToday() and isDateAfterToday which call isDate(), are used in screens, forms and minilang (look for isDate in *.xml).

You can't rely only on compilation and make methods private

Jacques

From: "Stephen Rufle" <[email protected]>
I did see the defaultEmptyOK in each of the methods I changed, my patch fixes 
the issue in
org.ofbiz.base.util.UtilValidate.isDate(String, String, String) where the lines

        int intYear = Integer.parseInt(year);
        int intMonth = Integer.parseInt(month);
        int intDay = Integer.parseInt(day);

should only get parse-able values. Current OOTB does not allow the "catch invalid 
years" logic to correctly protect the parsing.
The three methods that are modified are only called from 
UtilValidate.isDate(String, String, String), so it seems like it should
be ok. I also tried an experiment locally I made those methods private and I do 
not get an compile errors, so I believe my change
is isolated to just fixing the issue I saw and has no ill side effects.

On 2/13/2011 10:24 AM, David E Jones wrote:

Please revert this change.

Changing things this way completely breaks the whole idea of the
"defaultEmptyOK" variable. If someone wants this to behave
differently then they can change it locally (patch or subclass). It
should NOT be changed in the project.

The whole idea with these validations is that you get one error if a
field is empty (if it is not allowed to be empty) and another
different error if the formatting is incorrect.

With this change it is now IMPOSSIBLE to have a field checked by
these methods that is allowed to be empty.

The current code is not broken. The problem is a lack of
understanding of how these validation methods are designed.

Jacques: please revert this and also please understand and test
before committing contributed patches. Don't trust the description
alone, look at the code and test it!

-David


On Feb 13, 2011, at 4:55 AM, [email protected] wrote:

Author: jleroux Date: Sun Feb 13 12:55:45 2011 New Revision:
1070230

URL: http://svn.apache.org/viewvc?rev=1070230&view=rev Log: A patch
from Stephen Rufle "Blank year in UtilValidate.isYear should return
false" (https://issues.apache.org/jira/browse/OFBIZ-4171) -
OFBIZ-4171

UtilValidate.isYear returns true for a blank year which the calling
function UtilValidate.isDate(String, String, String) tries to
parse. This causes an exception to be thrown ValidateMethod.java:96
:ERROR] [ValidateMethod.exec] Error in validation method
isDateAfterToday of class org.ofbiz.base.util.UtilValidate: null I
found this error when trying to add a new credit card in the
eCommerce checkout flow. 1. Add a product to the cart 2. login as
any user I used "admin" 3. Checkout Step "Shipping Address" (Step
1: Where shall we ship it?) * Click Next 4. Checkout Step "Shipping
Options" (Step 2: How shall we ship it?) * Choose "UPS Air" * Click
Next 5. Checkout Step "Payment Options" (Step 3: How shall you
pay?) 6. Create "Credit Card" * Fill Name * Card Type "Visa" * Card
Number "4111111111111111" * Expiration Date Month drop-down 01 *
Expiration Date Year drop-down leave blank * Choose billing
address * Click "Save" button

Should see "Error in validation method isDateAfterToday of class
org.ofbiz.base.util.UtilValidate: null"

My fix is to change isYear, isMonth, and isDay to return false when
a blank value is entered. After I make my change the message is
"The expiration date is before today"

Modified:
ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilValidate.java



Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilValidate.java
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilValidate.java?rev=1070230&r1=1070229&r2=1070230&view=diff


==============================================================================
---
ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilValidate.java
(original) +++
ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilValidate.java
Sun Feb 13 12:55:45 2011 @@ -746,7 +746,7 @@ public class
UtilValidate { *  to use 4-digit year numbers everywhere. */ public
static boolean isYear(String s) { -        if (isEmpty(s)) return
defaultEmptyOK; +        if (isEmpty(s)) return false;

if (!isNonnegativeInteger(s)) return false; return ((s.length() ==
2) || (s.length() == 4)); @@ -771,13 +771,13 @@ public class
UtilValidate {

/** isMonth returns true if string s is a valid month number
between 1 and 12. */ public static boolean isMonth(String s) { -
if (isEmpty(s)) return defaultEmptyOK; +        if (isEmpty(s))
return false; return isIntegerInRange(s, 1, 12); }

/** isDay returns true if string s is a valid day number between 1
and 31. */ public static boolean isDay(String s) { -        if
(isEmpty(s)) return defaultEmptyOK; +        if (isEmpty(s)) return
false; return isIntegerInRange(s, 1, 31); }







--
Stephen P Rufle
[email protected]
O:480-626-8022
Yahoo IM: stephen_rufle
AOL IM: stephen1rufle



Reply via email to