[master 4/4] Fix a bug in reading a size specification with a radix in the numeric part.

mulkieran installerbot-noreply at redhat.com
Fri May 22 15:14:51 UTC 2015


From: mulhern <amulhern at redhat.com>

Since the | operator is not greedy, the match would return the first of the
three base alternatives that worked, and would split, e.g., "0.0" into
"0" and ".0" instead of "0.0" and nothing.

Now, base is a single alternative, that should capture numbers with a
decimal point correctly.

Fixes a bug introduced in commit 65703bfc851f6d919f8e66e1a635ace5835ea6c1.

Changes to the regular expression are:
* No longer allows spaces between optional sign and number. Decimal
constructor will raise exception on '- 2', so might as well not go to extra
trouble to accept it in regular expression.
* Uses verbose regular expression, to make it a little easier to read.
Uses separate compile before match call for the same reason.
* No longer allows trailing non-whitespace characters after a separating
space, e.g., "1 KiB or maybe 2". Still  allows trailing or leading
whitespace characters. Never allowed leading non-whitespace characters,
e.g., "nearly 1 PiB".
* The expression that matches the base is a lot simpler.

There is also a huge comment above the regular expression. Hopefully, it
will prevent people from making unnecessarily complicated and possibly
buggy regular expressions in future.

The only change to the behavior of the method is that the bug is fixed.

Adds a few more tests.

Signed-off-by: mulhern <amulhern at redhat.com>
---
 blivet/size.py     | 24 +++++++++++++++++++++++-
 tests/size_test.py | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/blivet/size.py b/blivet/size.py
index ff9e8cb..610b3ec 100644
--- a/blivet/size.py
+++ b/blivet/size.py
@@ -178,7 +178,29 @@ def parseSpec(spec):
     if radix != '.':
         spec = spec.replace(radix, '.')
 
-    m = re.match(r'(?P<numeric>(-|\+)?\s*(?P<base>([0-9]+)|([0-9]*\.[0-9]+)|([0-9]+\.[0-9]*))(?P<exp>(e|E)(-|\+)[0-9]+)?)\s*(?P<rest>[^\s]*)$', spec.strip())
+    # The purpose of this regular expression is to distinguish
+    # between the numeric part and the part that specifies the units.
+    # The regular expression that matches the numeric part of the spec
+    # should recognize all valid numbers and should not include any part
+    # of the unit specifier in the string that it recognizes. It may
+    # recognize strings that are not valid numbers as well, if it does
+    # some other part of the implementation is expected to recognize that
+    # the number is invalid and raise an exception.
+    # Specifically, the string "0.9.9 KiB" will be matched, and the numeric
+    # part will match "0.9.9". This is not a valid number, but that will
+    # be detected when an exception is raised during conversion of the numeric
+    # part to a numeric value.
+    spec_re = re.compile(
+       r"""(?P<numeric> # the numeric part consists of three parts, below
+           (-|\+)? # optional sign character
+           (?P<base>([0-9\.]+)) # the base
+           (?P<exp>(e|E)(-|\+)[0-9]+)?) # optional exponent
+           \s* # whitespace
+           (?P<rest>[^\s]*$) # the units specification
+        """,
+        re.VERBOSE
+    )
+    m = re.match(spec_re, spec.strip())
     if not m:
         raise ValueError("invalid size specification", spec)
 
diff --git a/tests/size_test.py b/tests/size_test.py
index 9416d71..7c12624 100644
--- a/tests/size_test.py
+++ b/tests/size_test.py
@@ -200,6 +200,57 @@ def testScientificNotation(self):
         self.assertEqual(size.parseSpec("1E-4KB"), Decimal("0.1"))
         self.assertEqual(Size("1E-10KB"), Size(0))
 
+        with self.assertRaises(ValueError):
+            # this is an exponent w/out a base
+            size.parseSpec("e+0")
+
+    def testFloatingPointStr(self):
+        self.assertEqual(size.parseSpec("1.5e+0 KiB"), Decimal(1536))
+        self.assertEqual(size.parseSpec("0.0"), Decimal(0))
+        self.assertEqual(size.parseSpec("0.9 KiB"), Decimal("921.6"))
+        self.assertEqual(size.parseSpec("1.5 KiB"), Decimal(1536))
+        self.assertEqual(size.parseSpec("0.5 KiB"), Decimal(512))
+        self.assertEqual(size.parseSpec(".5 KiB"), Decimal(512))
+        self.assertEqual(size.parseSpec("1. KiB"), Decimal(1024))
+        self.assertEqual(size.parseSpec("-1. KiB"), Decimal(-1024))
+        self.assertEqual(size.parseSpec("+1. KiB"), Decimal(1024))
+        self.assertEqual(size.parseSpec("+1.0000000e+0 KiB"), Decimal(1024))
+        self.assertEqual(size.parseSpec("+.5 KiB"), Decimal(512))
+
+        with self.assertRaises(ValueError):
+            # this is a fragment of an arithmetic expression
+            size.parseSpec("+ 1 KiB")
+
+        with self.assertRaises(ValueError):
+            # this is a fragment of an arithmetic expression
+            size.parseSpec("- 1 KiB")
+
+        with self.assertRaises(ValueError):
+            # this is a lonely .
+            size.parseSpec(". KiB")
+
+        with self.assertRaises(ValueError):
+            # this has a fragmentary exponent
+            size.parseSpec("1.0e+ KiB")
+
+        with self.assertRaises(ValueError):
+            # this is a version string, not a number
+            size.parseSpec("1.0.0")
+
+    def testWhiteSpace(self):
+        self.assertEqual(size.parseSpec("1 KiB "), Decimal(1024))
+        self.assertEqual(size.parseSpec(" 1 KiB"), Decimal(1024))
+        self.assertEqual(size.parseSpec(" 1KiB"), Decimal(1024))
+        self.assertEqual(size.parseSpec(" 1e+0KiB"), Decimal(1024))
+        with self.assertRaises(ValueError):
+            size.parseSpec("1 KiB just a lot of stray characters")
+        with self.assertRaises(ValueError):
+            size.parseSpec("just 1 KiB")
+
+    def testLeadingZero(self):
+        self.assertEqual(size.parseSpec("001 KiB"), Decimal(1024))
+        self.assertEqual(size.parseSpec("1e+01"), Decimal(10))
+
     def testPickling(self):
         s = Size("10 MiB")
         self.assertEqual(s, cPickle.loads(cPickle.dumps(s)))


-- 
To view this commit on github, visit https://github.com/rhinstaller/blivet/commit/9dc0115f1f927fac50a56d3e01184afaf749f8fe


More information about the anaconda-patches mailing list