Merge pull request #3323 from ikyasam18/master

Validate str type before tuple cast

luigi/parameter.py

@@ -1384,7 +1384,12 @@ def parse(self, x):
             # loop required to parse tuple of tuples
             return tuple(self._convert_iterable_to_tuple(x) for x in json.loads(x, object_pairs_hook=FrozenOrderedDict))
         except (ValueError, TypeError):
-            return tuple(literal_eval(x))  # if this causes an error, let that error be raised.
+            result = literal_eval(x)
+            # t_str = '("abcd")'
+            # Ensure that the result is not a string to avoid cases like ('a','b','c','d')
+            if isinstance(result, str):
+                raise ValueError("Parsed result cannot be a string")
+            return tuple(result)  # if this causes an error, let that error be raised.
 
     def _convert_iterable_to_tuple(self, x):
         if isinstance(x, str):

test/parameter_test.py

@@ -567,6 +567,18 @@ class Foo(luigi.Task):
         self.assertEqual(hash(Foo(args=('foo', 'bar')).args),
                          hash(p.normalize(p.parse('["foo", "bar"]'))))
 
+    def test_tuple_invalid_string(self):
+        param = luigi.TupleParameter()
+        self.assertRaises(ValueError, lambda: param.parse('("abcd")'))
+
+    def test_tuple_invalid_string_in_tuple(self):
+        param = luigi.TupleParameter()
+        self.assertRaises(ValueError, lambda: param.parse('(("abcd"))'))
+
+    def test_parse_invalid_format(self):
+        param = luigi.TupleParameter()
+        self.assertRaises(SyntaxError, lambda: param.parse('((1,2),(3,4'))
+
     def test_task(self):
         class Bar(luigi.Task):
             pass
@@ -1225,6 +1237,7 @@ class LocalParameters1304Test(LuigiTestCase):
 
     https://github.com/spotify/luigi/issues/1304#issuecomment-148402284
     """
+
     def test_local_params(self):
 
         class MyTask(RunOnceTask):