TI-39: Bogus command line option causes segfault

- Return error message for invalid values in CmdChart
- Add tests (partially moved from cli.t)

src/commands/CmdChart.cpp

@@ -158,6 +158,10 @@ int renderChart (
                (rules.getBoolean ("reports." + type + ".weekday") ? 4 : 0);
 
   auto cell = rules.getInteger ("reports." + type + ".cell");
+
+  if (cell < 1)
+    throw std::string ("Invalid value for 'reports." + type + ".cell': '" + rules.get("reports." + type + ".cell") + "'");
+
   auto chars_per_hour = 60 / cell;
 
   // Each day is rendered separately.
@@ -169,6 +173,9 @@ int renderChart (
     if (rules.has ("reports." + type + ".lines"))
       num_lines = rules.getInteger ("reports." + type + ".lines", num_lines);
 
+    if (num_lines < 1)
+      throw std::string ("Invalid value for 'reports." + type + ".lines': '" + rules.get("reports." + type + ".lines") + "'");
+
     int spacing = 1;
     if (rules.has ("reports." + type + ".spacing"))
       spacing = rules.getInteger ("reports." + type + ".spacing");
@@ -292,6 +299,10 @@ static void renderAxis (
   int last_hour)
 {
   auto cell = rules.getInteger ("reports." + type + ".cell");
+
+  if (cell < 1)
+    throw std::string ("Invalid value for 'reports." + type + ".cell': '" + rules.get("reports." + type + ".cell") + "'");
+
   auto chars_per_hour = 60 / cell;
 
   auto spacing = rules.getInteger ("reports." + type + ".spacing");
@@ -408,6 +419,10 @@ static std::string renderSubTotal (
     int spacing = rules.getInteger ("reports." + type + ".spacing");
 
     auto cell = rules.getInteger ("reports." + type + ".cell");
+
+    if (cell < 1)
+      throw std::string ("Invalid value for 'reports." + type + ".cell': '" + rules.get("reports." + type + ".cell") + "'");
+
     auto chars_per_hour = 60 / cell;
 
     std::string pad (indent + ((last_hour - first_hour + 1) * (chars_per_hour + spacing)) + 1, ' ');
@@ -440,6 +455,10 @@ static void renderExclusionBlocks (
   const std::vector <Range>& excluded)
 {
   auto cell = rules.getInteger ("reports." + type + ".cell");
+
+  if (cell < 1)
+    throw std::string ("Invalid value for 'reports." + type + ".cell': '" + rules.get("reports." + type + ".cell") + "'");
+
   auto chars_per_hour = 60 / cell;
 
   auto spacing = rules.getInteger ("reports." + type + ".spacing");

test/chart.t

@@ -0,0 +1,101 @@
+#!/usr/bin/env python2.7
+# -*- coding: utf-8 -*-
+###############################################################################
+#
+# Copyright 2006 - 2017, Paul Beckingham, Federico Hernandez.
+#
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included
+# in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+# OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+# SOFTWARE.
+#
+# http://www.opensource.org/licenses/mit-license.php
+#
+###############################################################################
+
+import sys
+import os
+import unittest
+from datetime import datetime
+# Ensure python finds the local simpletap module
+sys.path.append(os.path.dirname(os.path.abspath(__file__)))
+
+from basetest import Timew, TestCase
+
+# Test methods available:
+#     self.assertEqual(a, b)
+#     self.assertNotEqual(a, b)
+#     self.assertTrue(x)
+#     self.assertFalse(x)
+#     self.assertIs(a, b)
+#     self.assertIsNot(substring, text)
+#     self.assertIsNone(x)
+#     self.assertIsNotNone(x)
+#     self.assertIn(substring, text)
+#     self.assertNotIn(substring, text
+#     self.assertRaises(e)
+#     self.assertRegexpMatches(text, pattern)
+#     self.assertNotRegexpMatches(text, pattern)
+#     self.tap("")
+
+
+class TestChart(TestCase):
+    def setUp(self):
+        """Executed before each test in the class"""
+        self.t = Timew()
+
+    def test_chart_day_with_invalid_config_for_lines(self):
+        """Chart should report error on invalid value for 'reports.day.lines'"""
+        code, out, err = self.t.runError("day rc.reports.day.lines=foobar")
+
+        self.assertIn("Invalid value for 'reports.day.lines': 'foobar'", err)
+
+    def test_chart_day_with_invalid_config_for_cell(self):
+        """Chart should report error on invalid value for 'reports.day.cell'"""
+        code, out, err = self.t.runError("day rc.reports.day.cell=foobar")
+
+        self.assertIn("Invalid value for 'reports.day.cell': 'foobar'", err)
+
+    def test_chart_week_with_invalid_config_for_lines(self):
+        """Chart should report error on invalid value for 'reports.week.lines'"""
+        code, out, err = self.t.runError("week rc.reports.week.lines=foobar")
+
+        self.assertIn("Invalid value for 'reports.week.lines': 'foobar'", err)
+
+    def test_chart_week_with_invalid_config_for_cell(self):
+        """Chart should report error on invalid value for 'reports.week.cell'"""
+        code, out, err = self.t.runError("week rc.reports.week.cell=foobar")
+
+        self.assertIn("Invalid value for 'reports.week.cell': 'foobar'", err)
+
+    def test_chart_month_with_invalid_config_for_lines(self):
+        """Chart should report error on invalid value for 'reports.month.lines'"""
+        code, out, err = self.t.runError("month rc.reports.month.lines=foobar")
+
+        self.assertIn("Invalid value for 'reports.month.lines': 'foobar'", err)
+
+    def test_chart_month_with_invalid_config_for_cell(self):
+        """Chart should report error on invalid value for 'reports.month.cell'"""
+        code, out, err = self.t.runError("month rc.reports.month.cell=foobar")
+
+        self.assertIn("Invalid value for 'reports.month.cell': 'foobar'", err)
+
+
+if __name__ == "__main__":
+    from simpletap import TAPTestRunner
+    unittest.main(testRunner=TAPTestRunner())
+
+# vim: ai sts=4 et sw=4 ft=python

test/cli.t

@@ -56,10 +56,6 @@ class TestCLI(TestCase):
         """Executed before each test in the class"""
         self.t = Timew()
 
-    def test_TI_39(self):
-        """TI-39: Bogus command line option causes segfault"""
-        self.t("week rc.reports.week.lines=foobar")
-
     def test_TimeWarrior_without_command_without_active_time_tracking(self):
         """Call 'timew' without active time tracking"""
         code, out, err = self.t()

test/rules.t.cpp

@@ -38,6 +38,10 @@ int main (int, char**)
   t.is (r.get ("string"),         "234", "Rules set string, get string");
   t.is (r.getInteger ("string"),  234,   "Rules set string, get string");
 
+  r.set ("undefined", "foo");
+  t.is (r.getInteger ("undefined", -1), 0, "Rules set non-int string, get 0");
+  t.is (r.getBoolean ("undefined"), false, "Rules set non-bool string, get false");
+
   r.set ("integer", 123);
   t.is (r.getInteger ("integer"), 123,   "Rules set integer, get integer");
   t.is (r.get        ("integer"), "123", "Rules set integer, get string");