The problem with that alone is that it will lead to the same problem further
down. For example, if formatKey is null, and value is a Number, the next
test will be to check formatStr against null. That could once again be
checking against a value that was left over from a previous instance of the
tag.

It seems to me that we should just make formatStr local to formatValue() and
be done with it. If you have a chance (it must be getting a bit late over
there now!), could you give that a quick try, and let me know the result? I
can check in the change.

--
Martin Cooper


----- Original Message -----
From: "Matt Read" <[EMAIL PROTECTED]>
To: "Struts Developers List" <[EMAIL PROTECTED]>
Sent: Wednesday, March 06, 2002 1:06 PM
Subject: RE: Re[2]: cvs commit:
jakarta-struts/src/share/org/apache/struts/taglib/bean WriteTag.java


> The fix I had made is as follows. Is there any reason why one would be
> better than the other? I'm new to editing struts source and would
appreciate
> pointers.
>
> Thanks,
> Matt.
>
> diff -r1.18 WriteTag.java
> 346c346
> <             if( ( formatStr==null ) && ( formatKey!=null ) ) {
> ---
> >             if( formatKey!=null ) {
>
> -----Original Message-----
> From: Martin Cooper [mailto:[EMAIL PROTECTED]]
> Sent: 06 March 2002 20:37
> To: Struts Developers List; Oleg V Alexeev
> Subject: Re: Re[2]: cvs commit:
> jakarta-struts/src/share/org/apache/struts/taglib/bean WriteTag.java
>
>
> The container is not required to call release() between uses of a pooled
tag
> handler. Resin, which Matt is using, doesn't call release() between uses,
> which is why I questioned the fix.
>
> From a quick look at the code, I believe the problem is that formatStr
> (which is not an attribute, although it is defined as if it is, with
getter
> and setter) is tested against null, and assigned within formatValue(). I
> don't know why it is tested for null, since there is no other method that
> manipulates this variable.
>
> I think the bug is that formatStr keeps its value from one use of the tag
to
> the next, and the first usage of that variable is a test against null.
After
> the first use, it will most likely be non-null in subsequent uses, thus
> causing the problem. The simplest fix is probably to null out formatStr at
> the end of formatValue(), so that the entry state is the same for the
first
> and subsequent uses of the tag. Let me know if you'd like me to make that
> change, Oleg.
>
> --
> Martin Cooper
>
>
> ----- Original Message -----
> From: "Oleg V Alexeev" <[EMAIL PROTECTED]>
> To: "Struts Developers List" <[EMAIL PROTECTED]>
> Sent: Wednesday, March 06, 2002 11:55 AM
> Subject: Re[2]: cvs commit:
> jakarta-struts/src/share/org/apache/struts/taglib/bean WriteTag.java
>
>
> > Hello Martin,
> >
> > I think that such error can be thrown in containers with tag pooling -
> > so prevously used tag instance will have default attribute value.
> >
> > Wednesday, March 06, 2002, 10:29:06 PM, you wrote:
> >
> > MC> Interesting. This should not make a difference unless there is
another
> bug
> > MC> involved (either in the tag or the container). What were the
symptoms
> > MC> involved here, and which container was being used?
> >
> > MC> --
> > MC> Martin Cooper
> >
> >
> > MC> ----- Original Message -----
> > MC> From: <[EMAIL PROTECTED]>
> > MC> To: <[EMAIL PROTECTED]>
> > MC> Sent: Wednesday, March 06, 2002 1:11 AM
> > MC> Subject: cvs commit:
> jakarta-struts/src/share/org/apache/struts/taglib/bean
> > MC> WriteTag.java
> >
> >
> > >> oalexeev    02/03/06 01:11:10
> > >>
> > >>   Modified:    src/share/org/apache/struts/taglib/bean WriteTag.java
> > >>   Log:
> > >>   Fix - formatKey class variable is not set to null in release()
> method.
> > >>   Bug submitted by "Matt Read" <[EMAIL PROTECTED]>
> > >>
> > >>   Revision  Changes    Path
> > >>   1.18      +5 -4
> > MC> jakarta-struts/src/share/org/apache/struts/taglib/bean/WriteTag.java
> > >>
> > >>   Index: WriteTag.java
> > >>   ===================================================================
> > >>   RCS file:
> > MC>
>
/home/cvs/jakarta-struts/src/share/org/apache/struts/taglib/bean/WriteTag.ja
> > MC> va,v
> > >>   retrieving revision 1.17
> > >>   retrieving revision 1.18
> > >>   diff -u -r1.17 -r1.18
> > >>   --- WriteTag.java 17 Jan 2002 18:20:16 -0000 1.17
> > >>   +++ WriteTag.java 6 Mar 2002 09:11:10 -0000 1.18
> > >>   @@ -1,7 +1,7 @@
> > >>    /*
> > >>   - * $Header:
> > MC>
>
/home/cvs/jakarta-struts/src/share/org/apache/struts/taglib/bean/WriteTag.ja
> > MC> va,v 1.17 2002/01/17 18:20:16 oalexeev Exp $
> > >>   - * $Revision: 1.17 $
> > >>   - * $Date: 2002/01/17 18:20:16 $
> > >>   + * $Header:
> > MC>
>
/home/cvs/jakarta-struts/src/share/org/apache/struts/taglib/bean/WriteTag.ja
> > MC> va,v 1.18 2002/03/06 09:11:10 oalexeev Exp $
> > >>   + * $Revision: 1.18 $
> > >>   + * $Date: 2002/03/06 09:11:10 $
> > >>     *
> > >>     *
> ====================================================================
> > >>     *
> > >>   @@ -89,7 +89,7 @@
> > >>     * output stream, optionally filtering characters that are
sensitive
> in
> > MC> HTML.
> > >>     *
> > >>     * @author Craig R. McClanahan
> > >>   - * @version $Revision: 1.17 $ $Date: 2002/01/17 18:20:16 $
> > >>   + * @version $Revision: 1.18 $ $Date: 2002/03/06 09:11:10 $
> > >>     */
> > >>
> > >>    public class WriteTag extends TagSupport {
> > >>   @@ -430,6 +430,7 @@
> > >>            property = null;
> > >>            scope = null;
> > >>            formatStr = null;
> > >>   +        formatKey = null;
> > >>            localeKey = null;
> > >>            bundle = null;
> > >>
> > >>
> > >>
> > >>
> > >>
> > >> --
> > >> To unsubscribe, e-mail:
> > MC> <mailto:[EMAIL PROTECTED]>
> > >> For additional commands, e-mail:
> > MC> <mailto:[EMAIL PROTECTED]>
> > >>
> >
> >
> > MC> --
> > MC> To unsubscribe, e-mail:
> <mailto:[EMAIL PROTECTED]>
> > MC> For additional commands, e-mail:
> <mailto:[EMAIL PROTECTED]>
> >
> >
> >
> > --
> > Best regards,
> >  Oleg                            mailto:[EMAIL PROTECTED]
> >
> >
> >
> > --
> > To unsubscribe, e-mail:
> <mailto:[EMAIL PROTECTED]>
> > For additional commands, e-mail:
> <mailto:[EMAIL PROTECTED]>
> >
>
>
> --
> To unsubscribe, e-mail:
<mailto:[EMAIL PROTECTED]>
> For additional commands, e-mail:
<mailto:[EMAIL PROTECTED]>
>
>
> --
> To unsubscribe, e-mail:
<mailto:[EMAIL PROTECTED]>
> For additional commands, e-mail:
<mailto:[EMAIL PROTECTED]>
>


--
To unsubscribe, e-mail:   <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>

Reply via email to