tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Pierre Delisle <>
Subject Re: Proper jsp:setProperty behavior for Tomcat 3.2
Date Mon, 20 Nov 2000 14:31:34 GMT
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=""/>

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

View raw message