Uploaded image for project: 'Struts 1'
  1. Struts 1
  2. STR-3130

Malformed or empty parameter converted to bignum field result in Exception being thrown on the floor

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Won't Fix
    • Affects Version/s: 1.3.5, 1.3.8
    • Fix Version/s: None
    • Component/s: Core
    • Labels:
      None

      Description

      If you make a field in ActionForm like
      BigDecimal myNum;
      with appropriate getter and setter, and then call action which uses that form with ?myNum= query string, you'll get an exception:
      org.apache.commons.beanutils.ConversionException
      at org.apache.commons.beanutils.converters.BigDecimalConverter.convert(BigDecimalConverter.java:117)
      at org.apache.commons.beanutils.ConvertUtilsBean.convert(ConvertUtilsBean.java:428)
      at org.apache.commons.beanutils.BeanUtilsBean.setProperty(BeanUtilsBean.java:1004)
      at org.apache.commons.beanutils.BeanUtilsBean.populate(BeanUtilsBean.java:811)
      at org.apache.commons.beanutils.BeanUtils.populate(BeanUtils.java:298)
      at org.apache.struts.util.RequestUtils.populate(RequestUtils.java:1252)
      at org.apache.struts.action.RequestProcessor.processPopulate(RequestProcessor.java:821)
      at ru.sbtc.shop.ShopRequestProcessor.processPopulate(ShopRequestProcessor.java:36)
      at org.apache.struts.action.RequestProcessor.process(RequestProcessor.java:254)
      at org.apache.struts.action.ActionServlet.process(ActionServlet.java:1482)
      at org.apache.struts.action.ActionServlet.doPost(ActionServlet.java:525)
      at javax.servlet.http.HttpServlet.service(HttpServlet.java:165)
      at javax.servlet.http.HttpServlet.service(HttpServlet.java:103)
      at com.caucho.server.http.FilterChainServlet.doFilter(FilterChainServlet.java:96)
      at ru.sbtc.shop.servlet.LoginFilter.doFilter(LoginFilter.java:132)
      at com.caucho.server.http.FilterChainFilter.doFilter(FilterChainFilter.java:88)
      at ru.sbtc.shop.servlet.TransactionFilter.doFilter(TransactionFilter.java:60)
      at com.caucho.server.http.FilterChainFilter.doFilter(FilterChainFilter.java:88)
      at ru.sbtc.shop.servlet.TransactionFilter.doFilter(TransactionFilter.java:60)
      at com.caucho.server.http.FilterChainFilter.doFilter(FilterChainFilter.java:88)
      at ru.sbtc.sitebuilder.servlet.SectionFilter.doFilter(SectionFilter.java:63)
      at com.caucho.server.http.FilterChainFilter.doFilter(FilterChainFilter.java:88)
      at ru.sbtc.sitebuilder.servlet.NewCharsetFilter.doFilter(NewCharsetFilter.java:112)
      at com.caucho.server.http.FilterChainFilter.doFilter(FilterChainFilter.java:88)
      at com.caucho.http.security.SecurityFilter.doFilter(SecurityFilter.java:115)
      at com.caucho.server.http.FilterChainFilter.doFilter(FilterChainFilter.java:88)
      at com.caucho.server.http.Invocation.service(Invocation.java:315)
      at com.caucho.server.http.HttpRequest.handleRequest(HttpRequest.java:253)
      at com.caucho.server.http.HttpRequest.handleConnection(HttpRequest.java:170)
      at com.caucho.server.TcpConnection.run(TcpConnection.java:139)
      at java.lang.Thread.run(Thread.java:595)
      Caused by: java.lang.NumberFormatException
      at java.math.BigDecimal.<init>(BigDecimal.java:457)
      at java.math.BigDecimal.<init>(BigDecimal.java:647)
      at org.apache.commons.beanutils.converters.BigDecimalConverter.convert(BigDecimalConverter.java:112)
      ... 30 more

      This is bad because:

      • Forms are for users to fill values into, you know, and user have the habit to write random garbage into fields or leave them empty.
      • I, as a developer of struts-based app, have no easy chances to work-around this, since exception is thrown straight from the ActionServlet

      This bug happens because BigDecimalConvertor reads:
      public BigDecimalConverter()

      { this.defaultValue = null; this.useDefault = false; }

      public Object convert(Class type, Object value) {

      if (value == null) {
      if (useDefault)

      { return (defaultValue); } else { throw new ConversionException("No value specified"); }
      }

      if (value instanceof BigDecimal) { return (value); }

      try { return (new BigDecimal(value.toString())); } catch (Exception e) {
      if (useDefault) { return (defaultValue); }

      else

      { throw new ConversionException(e); }

      }

      }

      AND ConvertUtilsBean reads:
      public void deregister() {
      ...
      converters.clear();
      register(BigDecimal.class, new BigDecimalConverter());
      register(BigInteger.class, new BigIntegerConverter());
      register(Boolean.TYPE, new BooleanConverter(defaultBoolean));
      register(Boolean.class, new BooleanConverter(defaultBoolean));

      You see? Primitive types get their defaults which let them survive nulls and mistypes, but no such service fir bignums - you'll get in a storm of exceptions if ever will try to use them.

      I guess that's beanutils authors to blame, but they will surely say "we don't see this as a problem, that's how it should work, re-register those types if you want". Struts don't have that privilege, because web-app which will respond with unhandled exceptions on form mistypes is spelled 'miserable'.

      Also, did noone ever really use bignums in those forms? I can't think of another way this bug persisted happily all those years to the 1.3.8.

      There is a workaround, however: convertNull parameter will force struts to re-register those:
      if (convertNull) {
      ConvertUtils.deregister();
      ConvertUtils.register(new BigDecimalConverter(null),
      BigDecimal.class);
      ConvertUtils.register(new BigIntegerConverter(null),
      BigInteger.class);
      ...
      Thus effectively solving this issue. But I think that, given all I wrote above, someone should just rewrite actionservlet to both re-registed bignum converters with sane defauls and handle possible conversion errors when parsing forms, assigning nulls (and possibly complaining to logs) when exception occurs in Converter instead of introducing user to all that stack trace.

        Attachments

          Activity

            People

            • Assignee:
              pbenedict Paul Benedict
              Reporter:
              ilyak Ilya Kasnacheev
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: