Uploaded image for project: 'Spring Framework'
  1. Spring Framework
  2. SPR-6871

Spring MVC regression: binding to indexed properties is broken

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Complete
    • Affects Version/s: 3.0.1
    • Fix Version/s: 3.0.2
    • Component/s: Web
    • Labels:
      None
    • Last commented by a User:
      true

      Description

      When you submit a form, with databinding on a command object with indexed properties, the indexed properties of the command object are not filled in.
      I think this bug is related to the correction that happened in SPR-6840.
      The characters [ and ] are not only removed from the id attrbute of the HTML elements, but also (unnessesary) from the name attribute of the HTML elements.

      You can see it happening via next example.
      When you type 10 and 20 in the input fields, the output to the console is:

      {Belgium=null, Switzerland=null}

      while the output should have been

      {Belgium=10, Switzerland=20}

      The class that acts as command object:
      --------------------------------------
      package org.example.entities;

      import java.util.LinkedHashMap;
      import java.util.Map;

      public class Continent {
      // key = name of country
      // Integer= number of inhabitants
      private Map<String, Integer> countries = new LinkedHashMap<String, Integer>();

      public Continent()

      { countries.put("Belgium", null); countries.put("Switzerland", null); }

      public void setCountries(Map<String, Integer> countries)

      { this.countries = countries; }

      public Map<String, Integer> getCountries()

      { return countries; }

      }

      The Controller class:
      ---------------------
      package org.example.web;

      import org.example.entities.Continent;
      import org.springframework.stereotype.Controller;
      import org.springframework.ui.Model;
      import org.springframework.validation.BindingResult;
      import org.springframework.web.bind.annotation.ModelAttribute;
      import org.springframework.web.bind.annotation.RequestMapping;
      import org.springframework.web.bind.annotation.RequestMethod;

      @Controller
      public class ContinentController {
      @RequestMapping(value = "/continent.htm", method = RequestMethod.GET)
      public String continentForm(Model model)

      { Continent continent = new Continent(); model.addAttribute(continent); return "continent.jsp"; }

      @RequestMapping(value = "/continent.htm", method = RequestMethod.POST)
      public String continentForm(@ModelAttribute Continent continent,
      BindingResult bindingResult)

      { System.out.println(continent.getCountries()); // Here you can see the bug return "continent.jsp"; }

      }

      continent.jsp
      -------------
      <?xml version="1.0" encoding="UTF-8"?>
      <%@page contentType="text/html" pageEncoding="UTF-8" session="false"%>
      <%@taglib prefix="c" uri="http://java.sun.com/jsp/jstl/core"%>
      <%@taglib prefix="form" uri="http://www.springframework.org/tags/form"%>
      <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
      "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
      <html xmlns="http://www.w3.org/1999/xhtml" xml:lang="nl" lang="nl">
      <head>
      <title>Continent example</title>
      <meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
      </head>
      <body>
      <form:form commandName="continent">
      <c:forEach items="$

      {continent.countries}

      " var="entry">
      <div>
      <form:label path="countries[$

      {entry.key}]">${entry.key}

      </form:label>
      <form:input path="countries[$

      {entry.key}]" />
      <form:errors path="countries[${entry.key}

      ]" />
      </div>
      </c:forEach>
      <div><input type="submit" /></div>
      </form:form>
      </body>
      </html>

        Issue Links

          Activity

          desmethans Hans Desmet created issue -
          Hide
          znbailey Zach Bailey added a comment -

          This is a rather grievous bug since it affects ALL form tags that contain these bracketed paths, which we use extensively throughout our application. For us, this would be considered a blocker-level bug since there is no reasonable workaround.

          Show
          znbailey Zach Bailey added a comment - This is a rather grievous bug since it affects ALL form tags that contain these bracketed paths, which we use extensively throughout our application. For us, this would be considered a blocker-level bug since there is no reasonable workaround.
          juergen.hoeller Juergen Hoeller made changes -
          Field Original Value New Value
          Assignee Juergen Hoeller [ juergen.hoeller ]
          Fix Version/s 3.0.2 [ 11332 ]
          juergen.hoeller Juergen Hoeller made changes -
          Link This issue is related to SPR-6840 [ SPR-6840 ]
          juergen.hoeller Juergen Hoeller made changes -
          Priority Major [ 3 ] Critical [ 2 ]
          Summary Spring MVC: binding to indexed properties is broken Spring MVC regression: binding to indexed properties is broken
          Hide
          juergen.hoeller Juergen Hoeller added a comment -

          Ouch, that's a pretty bad regression that we unfortunately neither had a unit test nor an integration test for. Sorry for that - an unfortunate last-minute accident.

          I've fixed this for 3.0.2 now. Should be available in tonight's 3.0.2 snapshot (build 591 and above).

          FYI, 3.0.2 is scheduled for release in early March already. Please use a snapshot for the time being and let us know whether it works for you.

          Juergen

          Show
          juergen.hoeller Juergen Hoeller added a comment - Ouch, that's a pretty bad regression that we unfortunately neither had a unit test nor an integration test for. Sorry for that - an unfortunate last-minute accident. I've fixed this for 3.0.2 now. Should be available in tonight's 3.0.2 snapshot (build 591 and above). FYI, 3.0.2 is scheduled for release in early March already. Please use a snapshot for the time being and let us know whether it works for you. Juergen
          juergen.hoeller Juergen Hoeller made changes -
          Resolution Complete [ 8 ]
          Status Open [ 1 ] Resolved [ 5 ]
          Hide
          znbailey Zach Bailey added a comment -

          Thanks for the quick turnaround, Juergen. I'll take a look at the snapshot and confirm the issue is fixed.

          Show
          znbailey Zach Bailey added a comment - Thanks for the quick turnaround, Juergen. I'll take a look at the snapshot and confirm the issue is fixed.
          Hide
          desmethans Hans Desmet added a comment -

          Tried it out with 3.0.2 snapshot (build 592). Problem solved.

          P.S. Problem in related SPR-6862 not yet solved

          Show
          desmethans Hans Desmet added a comment - Tried it out with 3.0.2 snapshot (build 592). Problem solved. P.S. Problem in related SPR-6862 not yet solved
          Hide
          grant.gochnauer Grant Gochnauer added a comment -

          Juergen,

          Where can I find the nightly snapshot?

          Thanks

          Show
          grant.gochnauer Grant Gochnauer added a comment - Juergen, Where can I find the nightly snapshot? Thanks
          Show
          grant.gochnauer Grant Gochnauer added a comment - Nevermind, I found it: http://static.springsource.org/downloads/nightly/snapshot-download.php?project=SPR
          Hide
          grant.gochnauer Grant Gochnauer added a comment -

          There appears to be another issue with this:

          https://fisheye.springsource.org/viewrep/spring-framework/trunk/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/tags/form/AbstractDataBoundFormElementTag.java?r1=2980&r2=3018

          It appears that indexed properties when used on the "id" attribute are still stripped.

          Thus we would get something like:
          <input id="promotionUser.formUserPropertyCodes'user_type'2" name="promotionUser.formUserPropertyCodes['user_type']" type="radio" value="543"/>

          Notice how "ID" has the stripped brackets but the name attribute does not.

          Show
          grant.gochnauer Grant Gochnauer added a comment - There appears to be another issue with this: https://fisheye.springsource.org/viewrep/spring-framework/trunk/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/tags/form/AbstractDataBoundFormElementTag.java?r1=2980&r2=3018 It appears that indexed properties when used on the "id" attribute are still stripped. Thus we would get something like: <input id="promotionUser.formUserPropertyCodes'user_type'2" name="promotionUser.formUserPropertyCodes ['user_type'] " type="radio" value="543"/> Notice how "ID" has the stripped brackets but the name attribute does not.
          Hide
          znbailey Zach Bailey added a comment -

          Grant,

          While I am not a core spring developer I am confident that is intended behavior, because if you look at the HTML/XHTML spec you will see that id attributes cannot contain bracket characters.

          Show
          znbailey Zach Bailey added a comment - Grant, While I am not a core spring developer I am confident that is intended behavior, because if you look at the HTML/XHTML spec you will see that id attributes cannot contain bracket characters.
          Hide
          grant.gochnauer Grant Gochnauer added a comment -

          Zach,

          I suppose that is true but it does break backward compatibility with 3.0.0 in our environment against 3.0.2's fix.

          Grant

          Show
          grant.gochnauer Grant Gochnauer added a comment - Zach, I suppose that is true but it does break backward compatibility with 3.0.0 in our environment against 3.0.2's fix. Grant
          Hide
          juergen.hoeller Juergen Hoeller added a comment -

          I'm afraid this is exactly what we desired to fix in 3.0.1 originally: the id attribute shouldn't contain any such brackets anymore, for no kind of input element. We were already doing this for all other form tags, just not for checkboxes and radiobuttons. It seems very much worth it to fix this remaining case once and for all in 3.0.x, with the name attribute regression properly addressed in 3.0.2, even we technically produce different ids for those then.

          Juergen

          Show
          juergen.hoeller Juergen Hoeller added a comment - I'm afraid this is exactly what we desired to fix in 3.0.1 originally: the id attribute shouldn't contain any such brackets anymore, for no kind of input element. We were already doing this for all other form tags, just not for checkboxes and radiobuttons. It seems very much worth it to fix this remaining case once and for all in 3.0.x, with the name attribute regression properly addressed in 3.0.2, even we technically produce different ids for those then. Juergen
          Hide
          grant.gochnauer Grant Gochnauer added a comment -

          Thanks for the quick response Juergen.

          Show
          grant.gochnauer Grant Gochnauer added a comment - Thanks for the quick response Juergen.
          juergen.hoeller Juergen Hoeller made changes -
          Link This issue is duplicated by SPR-6921 [ SPR-6921 ]
          juergen.hoeller Juergen Hoeller made changes -
          Link This issue is duplicated by SPR-7040 [ SPR-7040 ]
          tmarshall Trevor Marshall made changes -
          Workflow jira [ 30488 ] SPR Workflow [ 40463 ]
          juergen.hoeller Juergen Hoeller made changes -
          Link This issue is related to SPR-5382 [ SPR-5382 ]
          cbeams Chris Beams made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          tmarshall Trevor Marshall made changes -
          Workflow SPR Workflow [ 40463 ] New SPR Workflow [ 64014 ]
          tmarshall Trevor Marshall made changes -
          Workflow New SPR Workflow [ 64014 ] SPR Workflow [ 73458 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Resolved Resolved
          1d 5h 18m 1 Juergen Hoeller 20/Feb/10 1:57 AM
          Resolved Resolved Closed Closed
          850d 1h 45m 1 Chris Beams 19/Jun/12 3:42 AM

            People

            • Assignee:
              juergen.hoeller Juergen Hoeller
              Reporter:
              desmethans Hans Desmet
              Last updater:
              Trevor Marshall
            • Votes:
              3 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Days since last comment:
                8 years, 1 week, 3 days ago