Hi Pierre,
Thanks, I was aware of:
<jsp:setProperty name="bean" property="prop" param="prop"/>
incorrectly allowing the set to get through.
I just sent a separate e-mail concerning this. I looked at
implementing a form of 3C. I asked someone here at SAS who
has done a lot of HTML and JSP. He seemed to indicate to me
that the sequence of parameters in the query string wasn't
reliable and the presence of intermixed empty values wasn't
significant. If this is true, then I'm not sure using the
parameter's position is important. You would get different
results depending how the browser arranged them.
My experimental patch simply set the indexed property with
a new array containing only the non-empty values and left
the property unchanged if there were no non-empty values.
It is possible I misinterpreted my HTML/JSP expert. Am I
incorrect about the sequence of parameters returned by
browers not being reliable?
Cheers,
Larry
> -----Original Message-----
> From: Pierre Delisle [mailto:[EMAIL PROTECTED]]
> Sent: Monday, November 20, 2000 9:32 AM
> To: [EMAIL PROTECTED]; [EMAIL PROTECTED]
> Subject: Re: Proper jsp:setProperty behavior for Tomcat 3.2
>
>
> Larry Isaacs wrote:
> >
> > When applying Gareth Morgan's bug fix for JspRuntimeLibrary
> in the Tomcat MAIN
> > branch, a difference between the tomcat_32 and MAIN
> versions revealed that
> > Tomcat 3.2 still has the bug discovered by Glenn Nielsen.
> This is where:
> >
> > <jsp:setProperty name="bean" property="prop" value=""/>
> >
> > leaves the property unchanged instead of setting it to an
> empty string. As Glenn
> > noted, the spec calls for ignoring an empty string when
> param is used, i.e.:
> >
> > <jsp:setProperty name="bean" property="prop" param="prop"/>
> >
> > with a query string of "?prop=". But doesn't say to ignore
> it for value. This
> > is easily fixed.
>
>
> 1) Please beware that the current fix to 3.3 did not quite do
> it properly
> (I ran more tests this weekend).
> It did fix the fact that an empty value is
> <jsp:setProperty> does go through
> properly (i.e. the property is set using the empty
> string), but it introduced
> the bug that an empty value for a request parameter also
> does go through
> (while it should not).
>
>
> > However, its not clear to me what the spec calls for with
> respect to empty
> > strings and indexed properties. Also, Tomcat 3.2's current
> behavior seems
> > inconsistent on this issue. I tested the following cases where the
> > property is a String array:
> >
> > 1) <jsp:setProperty name="bean" property="prop" param="prop"/>
> > 2) <jsp:setProperty name="bean" property="prop"/>
> > 3) <jsp:setProperty name="bean" property="*"/>
> >
> > For an empty query string, or a query string with mutlitple
> non-empty values,
> > you get the expected result. However, for the following
> query strings
> > which involve empty values, I get the following results for
> how the String
> > array is set.
> >
> > Query String Result1 Result2 Result3 Spec
> > ?prop= "" "" not changed
> not changed
> > ?prop=&prop= "","" "","" not changed ???
> > ?prop=&prop=text "","text" "","text" not changed ???
> >
> > I would expect the results to be the same instead of
> different. The spec
> > doesn't say that ignoring an empty value shouldn't apply to
> an indexed
> > property. But if that's true, what about "?prop=&prop="?
> Should this be
> > ignored too? And should "?prop=&prop=text" set the
> property to a single
> > element array containing "text"?
> >
> > Am I missing something in the spec? Can someone shed some
> light on what
> > proper behavior should be?
> >
>
>
> I discussed the issue with Eduardo (JSP spec lead)
> on Friday afternoon.
>
> He agreed that the spec is unclear and has filed a bug
> against the spec to trigger further discussions with the
> expert group.
>
> In the meantime, here is what we can do:
>
> -----
> 2) Bug fix: When using property="*", indexed properties are
> not properly handled when
> first property is null or empty
>
> Your test cases exposed this bug. Only the first parameter of an
> indexed property is considered when figuring out if it is
> null or empty.
>
> This is a simple fix. This will bring your test cases
> to all have a consistent behavior (i.e. do not ignore the
> request parameters for an indexed property as soon as first
> parameter value is empty).
>
> -----
> 3) Pick one behavior for 'indexed properties' in the case of
> empty values and document it properly.
>
> I see 3 types of behavior that could make sense:
>
> 3A) All request parameter values are passed 'intact' as an array
> to the indexed property setter method.
>
> This means that the following query strings would yield
> the following
> arrays being passed to the setter method:
>
> query string: ?prop=&prop=one&prop=&prop=three&prop=&prop=
> array: {"","one","","three","",""}
>
> query string: ?prop=&prop=&prop=&prop=&prop=&prop=
> array: {"","","","","",""}
>
> 3B) Same behavior as 3A), with the exception that the setter
> method with the array argument is called only if at least
> one parameter value is non-empty.
>
> query string: ?prop=&prop=one&prop=&prop=three&prop=&prop=
> array: {"","one","","three","",""}
>
> query string: ?prop=&prop=&prop=&prop=&prop=&prop=
> -> setter method not called; indexed property unchanged
>
> 3C) Only set the properties that have a non-empty value
>
> query string: ?prop=&prop=one&prop=&prop=three&prop=&prop=
> setIndexed(1, "one")
> setIndexed(3, "three")
>
> query string: ?prop=&prop=&prop=&prop=&prop=&prop=
> -> no setter called
>
> -----
> 3A is identical in behavior as what happens when an indexed property
> is set with the "value" attribute in <jsp:setProperty>
>
> <jsp:setProperty property="indexed" value=<%=new
> String[]{"","",""}%>/>
>
> 3C has the same behavior as for non-indexed properties; only non-empty
> values in request parameters trigger a call to the setter method
> for the specific index.
>
> 3B is half-way solution: it has the same behavior as for non-indexed
> properties only if all values of the indexed property are empty.
>
>
> > I'm +1 on fixing the <jsp:setProperty name="bean"
> property="prop" value=""/>
> > bug in Tomcat 3.2. If the desired behavior for indexed
> properties can be
> > determined, I can try to fix that too.
>
>
> I'd suggest the following:
>
> - We should fix 1), but with the 'new' fix discussed above
> (3.3. and 4.0 need that fix too)
>
> - We should fix 2)
>
> - Do 3A) because:
> - This is the current behavior (once we apply bug fix 2)
> - 3B does not seem a proper solution
> - 3C is not as trivial a fix as I'd like it to be
> (although I'd think the expert group may eventually
> favor this option
> to have consistent behavior between indexed and
> non-indexed properties)
> - not clear what the expert group will decide
> ... unless someone feels really strongly that 3C or should be
> implemented, and we could bite the bullet now to implement
> it (and influence
> the expert group to go that way)
>
> - Talk the CTS folks into writing test cases covering the
> null/empty values for both 'request parameters'
> and 'value' inputs (samples below)
>
> -- Pierre
>
> ==============================================================
> =============
> Sample Test cases (leveraging test cases already proposed by Larry)
>
> Assuming bean 'Bean' with the following properties:
>
> public String getProp();
> public void setProp(String val);
>
> public String[] getIndexed() // array getter
> public void setIndexed(String[] values); // array setter
> public void setIndexed(int index, String value); // indexed setter
> public String getIndexed(int index); // indexed getter
>
>
> 1) <jsp:setProperty name="bean" property="prop" value=""/>
>
> setProp("");
>
> 2) <jsp:setProperty name="bean" property="prop"/>
> 3) <jsp:setProperty name="bean" property="*"/>
> 4) <jsp:setProperty name="bean" property="prop" param="prop"/>
>
> query string result
> ------------ ------
> none no call made
> ?prop= no call made
>
> 5) <jsp:setProperty name="bean" property="indexed"/>
> 6) <jsp:setProperty name="bean" property="*"/>
> 7) <jsp:setProperty name="bean" property="indexed" param="indexed"/>
>
> If behavior 3A)
> query string result
> ------------ ------
> none no call made
> ?indexed= setIndexed({""})
> ?indexed=&indexed= setIndexed({"",""})
> ?indexed=&indexed=text setIndexed({"","text"})
>
> If behavior 3C)
> query string result
> ------------ ------
> none no call made
> ?indexed= no call made
> ?indexed=&indexed= no call made
> ?indexed=&indexed=text setIndexed(1,"text")
>
