Fix a couple of issues with handling multi-line strings

In some cases, in multi-line statements, particularly in the case
of multi-line strings, some lines would be missing from the line
range. It is now implemented to use the range of all lines between
the first and last lines.

Also the code in the string node visitor which was used to ignore
docstrings was incomplete, in that docstrings can be seen as ast
expressions with ast string values.  The string node visitor now
excludes these expressions as well.

Change-Id: I1cd9fb83351f9fb3ee0b60778518d331bf0f66a1
Closes-Bug: #1432807

bandit/core/node_visitor.py

@@ -132,7 +132,9 @@ class StatementBuffer():
         for n in ast.walk(node):
             if hasattr(n, 'lineno'):
                 lines.add(n.lineno)
-        return sorted(lines)
+        # we'll return a range here, because in some cases ast.walk skips over
+        # important parts, such as the middle lines in a multi-line string
+        return range(min(lines), max(lines) + 1)
 
     def get_skip_lines(self):
         return self.skip_lines
@@ -313,10 +315,21 @@ class BanditNodeVisitor(ast.NodeVisitor):
         self.logger.debug("visit_Str called (%s)" % ast.dump(node))
 
         # This check is to make sure we aren't running tests against
-        # docstrings (any statment that is just a string, nothing else)
-        if not isinstance(self.context['statement']['node'], ast.Str):
+        # docstrings (any statement that is just a string, nothing else)
+        node_object = self.context['statement']['node']
+
+        # docstrings can be represented as standalone ast.Str
+        is_str = isinstance(node_object, ast.Str)
+        # or ast.Expr with a value of type ast.Str
+        if (isinstance(node_object, ast.Expr) and
+                isinstance(node_object.value, ast.Str)):
+            is_standalone_expr = True
+        else:
+            is_standalone_expr = False
+        # if we don't have either one of those, run the test
+        if not (is_str or is_standalone_expr):
             self.update_score(self.tester.run_tests(self.context, 'Str'))
-            super(BanditNodeVisitor, self).generic_visit(node)
+        super(BanditNodeVisitor, self).generic_visit(node)
 
     def visit_Exec(self, node):
         self.context['str'] = 'exec'

examples/multiline-str.py

@@ -0,0 +1,19 @@
+# trigger warn for temp file
+x = """
+      /tmp
+"""
+
+# no finding
+"abc"
+
+# no finding, docstring
+def f():
+    """
+    /tmp
+    """
+# trigger warning for tempfile
+def f():
+    x='''
+    /tmp
+    '''
+

tests/test_functional.py

@@ -118,6 +118,10 @@ class FunctionalTests(unittest.TestCase):
         '''Test for dangerous imports.'''
         self.check_example('imports.py', info=2)
 
+    def test_multiline_str(self):
+        '''Test docstrings and multi-line strings are handled properly.'''
+        self.check_example('multiline-str.py', warn=2)
+
     def test_mktemp(self):
         '''Test for `tempfile.mktemp`.'''
         self.check_example('mktemp.py', warn=4)