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")
