Uploaded image for project: 'Struts 2'
  1. Struts 2
  2. WW-3171

"double" and "Double" are not validated with the same decimal séparator

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0.14
    • Fix Version/s: 2.5.12
    • Component/s: Core Actions
    • Labels:
      None
    • Environment:

      Windows or Linux - Running under Tomcat 6.0 - fr_FR

    • Flags:
      Important

      Description

      I had this strange behaviour.
      I have two double in my Action (extends ActionSupport):
      ---------------------------
      private double dbl1;
      private Double dbl2;
      ---------------------------
      With there respective getters and setters.

      But, when I call the action with these two doubles (from a classical HTML form), I must put a "dot" for the double, and a "comma" for the Double as a decimal separator,

      I'm surprised by this behaviour .... Nothing particular was done (HTML form calling an action). I can't imagine what is the reason of this behaviour, so I raise it as a bug.

      Regards

      Francillo

        Issue Links

          Activity

          Hide
          nether Jorge Fernandez added a comment - - edited

          Using the incorrect decimal separator with a Double value doesn't raise a validation error, instead it incorporates the fractional part of the number to the integral part of the number.

          For example, writing 10.0 will became 100 instead of raising the validation error "Invalid field value for field" as it does with double values.

          action:

          private Double doble=10.0;
              public Double getDoble(){
                  return doble;
              }
          
              public void setDoble(Double doble){
                  this.doble = doble;
              }
          

          jsp:

          <s:textfield key="doble" label="Double" />
          

          If the action is called from the jsp and return to the jsp the value of doble multiplies * 10 in each call.

          This happens with Struts 2.1.8, Glashfish v2.1 and Linux with the spanish language.

          Show
          nether Jorge Fernandez added a comment - - edited Using the incorrect decimal separator with a Double value doesn't raise a validation error, instead it incorporates the fractional part of the number to the integral part of the number. For example, writing 10.0 will became 100 instead of raising the validation error "Invalid field value for field" as it does with double values. action: private Double doble=10.0; public Double getDoble(){ return doble; } public void setDoble( Double doble){ this .doble = doble; } jsp: <s:textfield key= "doble" label= "Double" /> If the action is called from the jsp and return to the jsp the value of doble multiplies * 10 in each call. This happens with Struts 2.1.8, Glashfish v2.1 and Linux with the spanish language.
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          I think this issue is related to how XWork is converting primitives, I'm working on that
          http://jira.opensymphony.com/browse/XW-606

          Show
          lukaszlenart Lukasz Lenart added a comment - I think this issue is related to how XWork is converting primitives, I'm working on that http://jira.opensymphony.com/browse/XW-606
          Hide
          jlmagc Jose L Martinez-Avial added a comment -

          I just found this bug, which is causing some headaches for our application. Are there any news for this issue?

          Show
          jlmagc Jose L Martinez-Avial added a comment - I just found this bug, which is causing some headaches for our application. Are there any news for this issue?
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          Change is a bit obstructive so it's postponed to 2.5

          Show
          lukaszlenart Lukasz Lenart added a comment - Change is a bit obstructive so it's postponed to 2.5
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user lukaszlenart opened a pull request:

          https://github.com/apache/struts/pull/138

          WW-3171 WW-3357 WW-3650 WW-4581: Locale aware converters

          This PR introduces Locale aware conversion of `BigDecimal` and `Double` and `Double`

          WW-3171(https://issues.apache.org/jira/browse/WW-3171)
          WW-3357(https://issues.apache.org/jira/browse/WW-3357)
          WW-3650(https://issues.apache.org/jira/browse/WW-3650)
          WW-4581(https://issues.apache.org/jira/browse/WW-4581)

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/lukaszlenart/struts locale-aware-converters

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/struts/pull/138.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #138


          commit f874f9cde56f74c5161b17e645f779805c51a04b
          Author: Lukasz Lenart <lukaszlenart@apache.org>
          Date: 2017-05-12T10:19:48Z

          WW-4581 Uses proper logic to convert String to BigDecimal

          commit 266d78d32c786276f37ae701267f6719ea9f8a75
          Author: Lukasz Lenart <lukaszlenart@apache.org>
          Date: 2017-05-12T11:48:14Z

          WW-3650 Supports double conversion for different locale

          commit 20eced95008a9e08a20a59f287115ede00268897
          Author: Lukasz Lenart <lukaszlenart@apache.org>
          Date: 2017-05-12T12:40:20Z

          WW-3171 Converts numbers to strings using locale

          commit 229afea64e77c2dba9eec62b2c339e9fc92c9ec7
          Author: Lukasz Lenart <lukaszlenart@apache.org>
          Date: 2017-05-12T13:58:54Z

          WW-3171 Uses proper number of digits when formatting double


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user lukaszlenart opened a pull request: https://github.com/apache/struts/pull/138 WW-3171 WW-3357 WW-3650 WW-4581 : Locale aware converters This PR introduces Locale aware conversion of `BigDecimal` and `Double` and `Double` WW-3171 ( https://issues.apache.org/jira/browse/WW-3171 ) WW-3357 ( https://issues.apache.org/jira/browse/WW-3357 ) WW-3650 ( https://issues.apache.org/jira/browse/WW-3650 ) WW-4581 ( https://issues.apache.org/jira/browse/WW-4581 ) You can merge this pull request into a Git repository by running: $ git pull https://github.com/lukaszlenart/struts locale-aware-converters Alternatively you can review and apply these changes as the patch at: https://github.com/apache/struts/pull/138.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #138 commit f874f9cde56f74c5161b17e645f779805c51a04b Author: Lukasz Lenart <lukaszlenart@apache.org> Date: 2017-05-12T10:19:48Z WW-4581 Uses proper logic to convert String to BigDecimal commit 266d78d32c786276f37ae701267f6719ea9f8a75 Author: Lukasz Lenart <lukaszlenart@apache.org> Date: 2017-05-12T11:48:14Z WW-3650 Supports double conversion for different locale commit 20eced95008a9e08a20a59f287115ede00268897 Author: Lukasz Lenart <lukaszlenart@apache.org> Date: 2017-05-12T12:40:20Z WW-3171 Converts numbers to strings using locale commit 229afea64e77c2dba9eec62b2c339e9fc92c9ec7 Author: Lukasz Lenart <lukaszlenart@apache.org> Date: 2017-05-12T13:58:54Z WW-3171 Uses proper number of digits when formatting double
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user aleksandr-m commented on the issue:

          https://github.com/apache/struts/pull/138

          Will this break apps with locales which are using different decimal separator? E.g. in db `123.456` with locale which uses `,` as a decimal separator.

          Show
          githubbot ASF GitHub Bot added a comment - Github user aleksandr-m commented on the issue: https://github.com/apache/struts/pull/138 Will this break apps with locales which are using different decimal separator? E.g. in db `123.456` with locale which uses `,` as a decimal separator.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on the issue:

          https://github.com/apache/struts/pull/138

          It's all about `String -> BigDecimal/Double -> String` conversion, so if you have a `Double` in DB represented as `Double` it shouldn't be a problem. This solves posting numbers with proper locale and decimal separator and displaying those other way the same.

          See this example https://github.com/apache/struts-examples/blob/master/type-conversion/src/main/java/org/apache/struts/example/NumberAction.java#L13-L15

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on the issue: https://github.com/apache/struts/pull/138 It's all about `String -> BigDecimal/Double -> String` conversion, so if you have a `Double` in DB represented as `Double` it shouldn't be a problem. This solves posting numbers with proper locale and decimal separator and displaying those other way the same. See this example https://github.com/apache/struts-examples/blob/master/type-conversion/src/main/java/org/apache/struts/example/NumberAction.java#L13-L15
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user cnenning commented on the issue:

          https://github.com/apache/struts/pull/138

          In our apps we have often demand for locale aware values. But more often we need dates, not doubles. Mostly we do this by calling java methods in JSPs. But having converts which do this out of the box would be great.

          > Will this break apps with locales which are using different decimal separator?

          Yes, for some apps that would be a breaking change. But IMHO most apps use number formats which their users expect (-> locale aware number format). So for most it would be an improvement and they could remove custom code. But still, some apps might be broken.

          I always find it hard to get java `NumberFormat` and `ParsePositon` right. Not sure if there are subtle bugs in this PR. But there are lots of tests so hopefully all corner cases are covered 😉

          Show
          githubbot ASF GitHub Bot added a comment - Github user cnenning commented on the issue: https://github.com/apache/struts/pull/138 In our apps we have often demand for locale aware values. But more often we need dates, not doubles. Mostly we do this by calling java methods in JSPs. But having converts which do this out of the box would be great. > Will this break apps with locales which are using different decimal separator? Yes, for some apps that would be a breaking change. But IMHO most apps use number formats which their users expect (-> locale aware number format). So for most it would be an improvement and they could remove custom code. But still, some apps might be broken. I always find it hard to get java `NumberFormat` and `ParsePositon` right. Not sure if there are subtle bugs in this PR. But there are lots of tests so hopefully all corner cases are covered 😉
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on the issue:

          https://github.com/apache/struts/pull/138

          `ParsePosition` is basically used to check if the whole value was used, e.g. `1234abc` can be normally parsed into `1234` but comparing the `ParsePosition` with length of the value you can catch such problems.

          Also please notice that I have to change just two old tests because of how `double` was represented - I hope that this change is as less intrusive as possible

          @cnenning can you elaborate a bit more about parsing dates? I thought this is already supported.

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on the issue: https://github.com/apache/struts/pull/138 `ParsePosition` is basically used to check if the whole value was used, e.g. `1234abc` can be normally parsed into `1234` but comparing the `ParsePosition` with length of the value you can catch such problems. Also please notice that I have to change just two old tests because of how `double` was represented - I hope that this change is as less intrusive as possible @cnenning can you elaborate a bit more about parsing dates? I thought this is already supported.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user cnenning commented on the issue:

          https://github.com/apache/struts/pull/138

          > can you elaborate a bit more about parsing dates? I thought this is already supported.

          Now that you mention it, I see there is a locale aware `DateConverter`. Cannot remember why we rolled our own. Might be it was not using the format (`SHORT, MEDIUM, LONG`) we wanted. wdyt about making that configurable?

          Show
          githubbot ASF GitHub Bot added a comment - Github user cnenning commented on the issue: https://github.com/apache/struts/pull/138 > can you elaborate a bit more about parsing dates? I thought this is already supported. Now that you mention it, I see there is a locale aware `DateConverter`. Cannot remember why we rolled our own. Might be it was not using the format (`SHORT, MEDIUM, LONG`) we wanted. wdyt about making that configurable?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on the issue:

          https://github.com/apache/struts/pull/138

          Sure thing, please register an issue

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on the issue: https://github.com/apache/struts/pull/138 Sure thing, please register an issue
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user aleksandr-m commented on the issue:

          https://github.com/apache/struts/pull/138

          What about `Float` and `float`? I remember writing custom converter for it.

          Show
          githubbot ASF GitHub Bot added a comment - Github user aleksandr-m commented on the issue: https://github.com/apache/struts/pull/138 What about `Float` and `float`? I remember writing custom converter for it.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on the issue:

          https://github.com/apache/struts/pull/138

          @aleksandr-m good point, done

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on the issue: https://github.com/apache/struts/pull/138 @aleksandr-m good point, done
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on the issue:

          https://github.com/apache/struts/pull/138

          Any other objections? I would start with what we have here and improve

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on the issue: https://github.com/apache/struts/pull/138 Any other objections? I would start with what we have here and improve
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user cnenning commented on the issue:

          https://github.com/apache/struts/pull/138

          👍 for merging

          Show
          githubbot ASF GitHub Bot added a comment - Github user cnenning commented on the issue: https://github.com/apache/struts/pull/138 👍 for merging
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 20eced95008a9e08a20a59f287115ede00268897 in struts's branch refs/heads/master from Lukasz Lenart
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=20eced9 ]

          WW-3171 Converts numbers to strings using locale

          Show
          jira-bot ASF subversion and git services added a comment - Commit 20eced95008a9e08a20a59f287115ede00268897 in struts's branch refs/heads/master from Lukasz Lenart [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=20eced9 ] WW-3171 Converts numbers to strings using locale
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 229afea64e77c2dba9eec62b2c339e9fc92c9ec7 in struts's branch refs/heads/master from Lukasz Lenart
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=229afea ]

          WW-3171 Uses proper number of digits when formatting double

          Show
          jira-bot ASF subversion and git services added a comment - Commit 229afea64e77c2dba9eec62b2c339e9fc92c9ec7 in struts's branch refs/heads/master from Lukasz Lenart [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=229afea ] WW-3171 Uses proper number of digits when formatting double
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/struts/pull/138

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/struts/pull/138
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 2fb431d97e479881cace53d4dc387a961b52f575 in struts's branch refs/heads/master from Lukasz Lenart
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=2fb431d ]

          WW-3171 WW-3650 WW-4581 makes number converters locale aware

          Show
          jira-bot ASF subversion and git services added a comment - Commit 2fb431d97e479881cace53d4dc387a961b52f575 in struts's branch refs/heads/master from Lukasz Lenart [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=2fb431d ] WW-3171 WW-3650 WW-4581 makes number converters locale aware
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          PR got merged

          Show
          lukaszlenart Lukasz Lenart added a comment - PR got merged
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Struts-JDK7-master #633 (See https://builds.apache.org/job/Struts-JDK7-master/633/)
          WW-3171 Converts numbers to strings using locale (lukaszlenart: rev 20eced95008a9e08a20a59f287115ede00268897)

          • (edit) core/src/main/java/com/opensymphony/xwork2/conversion/impl/StringConverter.java
            WW-3171 Uses proper number of digits when formatting double (lukaszlenart: rev 229afea64e77c2dba9eec62b2c339e9fc92c9ec7)
          • (edit) core/src/test/resources/org/apache/struts2/views/jsp/ui/Select-3.txt
          • (edit) core/src/test/java/com/opensymphony/xwork2/validator/DoubleRangeFieldValidatorTest.java
          • (edit) core/src/main/java/com/opensymphony/xwork2/conversion/impl/StringConverter.java
          • (add) core/src/test/java/com/opensymphony/xwork2/conversion/impl/StringConverterTest.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Struts-JDK7-master #633 (See https://builds.apache.org/job/Struts-JDK7-master/633/ ) WW-3171 Converts numbers to strings using locale (lukaszlenart: rev 20eced95008a9e08a20a59f287115ede00268897) (edit) core/src/main/java/com/opensymphony/xwork2/conversion/impl/StringConverter.java WW-3171 Uses proper number of digits when formatting double (lukaszlenart: rev 229afea64e77c2dba9eec62b2c339e9fc92c9ec7) (edit) core/src/test/resources/org/apache/struts2/views/jsp/ui/Select-3.txt (edit) core/src/test/java/com/opensymphony/xwork2/validator/DoubleRangeFieldValidatorTest.java (edit) core/src/main/java/com/opensymphony/xwork2/conversion/impl/StringConverter.java (add) core/src/test/java/com/opensymphony/xwork2/conversion/impl/StringConverterTest.java
          Hide
          thrawnca Mitth'raw'nuruodo added a comment -

          It's a bit unfortunate that this was bundled with 2.5.12, which included important security fixes, despite being recognised as a potentially breaking change. A 2.5.10.2 release with just the security fixes would have been nice.

          Show
          thrawnca Mitth'raw'nuruodo added a comment - It's a bit unfortunate that this was bundled with 2.5.12, which included important security fixes, despite being recognised as a potentially breaking change. A 2.5.10.2 release with just the security fixes would have been nice.
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          Each of these vulnerabilities has an easy workaround to apply so that's why we decided to incorporate the fixes into a normal release.

          Show
          lukaszlenart Lukasz Lenart added a comment - Each of these vulnerabilities has an easy workaround to apply so that's why we decided to incorporate the fixes into a normal release.

            People

            • Assignee:
              lukaszlenart Lukasz Lenart
              Reporter:
              francillo françois-frédéric jean
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development