[Bug 1817595] [NEW] [SRU] urdfdom-headers and urdfdom should not use locale dependent parsing for floating point numbers

[Launchpad Bug Tracker]

You have been subscribed to a public bug by Jose Luis Rivero (j-rivero):

[Impact]

* The parsing done by the code of urdfdom truncate floating values on
systems that are not using the locales defined by default on English
systems. The users of this system are getting wrong values which are
specially sensible on variables with a range between 0 and 1.

* The URDF schema defines doubles in URDF as xs:double. Looking at the
XSD document it says that the mantissa for a double is represented as a
"decimal" number. And looking at the XSD document a decimal number is a
"finite-length sequence of decimal digits (#x30-#x39) separated by a
period as a decimal indicator". Thus, the locale should have no effect
on how the document is parsed, and using std::stod is incorrect. This
patch switches the Vector3 class to using the stream operator, which
seems to ignore locale.

* urdfdom-headers is a key part for the largest open source robotics
framework, ROS (Robot Operative System). So it potentially affects a big
part of the robotics open source community. We (the devs and maintainers
of ROS have been receiving many different issues or bugs coming from
this problem, as Simon wrote in the initial post of this bug.

[Test Case]

* We have prepared a test case that demonstrate the problem and the fix. Patched packages for both urdfdom and urdfdom_headers are in this ppa:
https://launchpad.net/~j-rivero/+archive/ubuntu/urdfdom-headers-sru2/+packages

* The dockerfile here https://github.com/j-rivero/sru_urdfdom/blob/master/Dockerfile tries to demonstrate the following:
 - Setup the test case, install packages
 - Import locale test from the latest development of urdfdom repo
 - Run tests, failed (demonstration of the bug)
 - Setup the PPA, install new version of packages
 - Run test, success (demonstration of the fix)

* This is the testing build of the dockerfile
https://build.osrfoundation.org/job/_urdfdom_sru/15/console

[Regression Potential]

* The change affected the result of parsed values from using stof/stod
functions to use stringstream. Although the patch is not small, the same
pattern is replied on every chunk of code changed. Standard use case of
users setting up digits in the form of integer or double is unlikely to
generate big regressions. Alternative use cases of users with weird
values in the input would trigger an exception from the code. This is no
different than the exceptions thrown by previous stof/stod funtions
(std::invalid_argument or std::out_of_range).

* The bug was reported in Nov 2017
(https://github.com/ros/urdfdom_headers/issues/41) the ROS community and
fixes landed in upstream repositories few weeks after. Many users from
different countries have been using new versions or these patches since
them, some of them are commenting/voting in this bug. Note that we were
also playing with upstream test suite to build the test case and no
regression was found in there.

[Other Info]

* Upstream releases new patched versions with the fix that I’ve included
into Debian Sid and have been synced by Ubuntu starting from Disco:
1.0.3-1

* debdiffs for versions from the PPA:
 - urdfdom debdiff:
   https://gist.github.com/j-rivero/9983dddc6dbbea99cd3c39774f1d0498

 - urdfdom-headers debdiff
   https://gist.github.com/j-rivero/bc563620802d78129d775b2d5aee69ac

https://github.com/ros/urdfdom/commit/3dc7ee812827cc69ffa457ef01fe7b9623096aed
https://github.com/ros/urdfdom_headers/commit/9d2b421f3fcbc2a32af40b99ebd9c2cb2d088fb9
https://github.com/ros/urdfdom_headers/pull/42
https://github.com/ros/urdfdom/pull/105
https://github.com/ros/urdfdom_headers/pull/47
https://github.com/ros/urdfdom/pull/115

See also the following upstream bug reports:
https://github.com/ros/urdfdom_headers/issues/45
https://github.com/ros/urdfdom/issues/119

and a few downstream bug reports to the ubuntu package:
https://github.com/ros-planning/moveit/issues/1333
https://github.com/ros/urdf/issues/21
https://github.com/ros-visualization/rviz/issues/1249
https://github.com/ros-visualization/rviz/issues/1151
https://github.com/ros-planning/moveit/issues/1050
https://github.com/ros-visualization/rviz/issues/1298

** Affects: urdfdom-headers
     Importance: Unknown
         Status: Unknown

** Affects: urdfdom (Ubuntu)
     Importance: Undecided
         Status: Fix Released

** Affects: urdfdom-headers (Ubuntu)
     Importance: Undecided
         Status: Fix Released

** Affects: urdfdom (Ubuntu Bionic)
     Importance: Undecided
         Status: New

** Affects: urdfdom-headers (Ubuntu Bionic)
     Importance: Undecided
         Status: New

-- 
[SRU] urdfdom-headers and urdfdom should not use locale dependent parsing for floating point numbers
https://bugs.launchpad.net/bugs/1817595
You received this bug notification because you are a member of Ubuntu Sponsors Team, which is subscribed to the bug report.