From a4d36ba0d22f858657fe40068867f099d79d6551 Mon Sep 17 00:00:00 2001
From: Vidar Holen <spam@vidarholen.net>
Date: Fri, 29 Nov 2013 23:05:41 -0800
Subject: [PATCH] Warn about while read f; do ssh "$f"; done

---
 ShellCheck/Analytics.hs | 36 ++++++++++++++++++++++++++++++++++++
 ShellCheck/Parser.hs    |  2 +-
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/ShellCheck/Analytics.hs b/ShellCheck/Analytics.hs
index 9acf67b..864cc74 100644
--- a/ShellCheck/Analytics.hs
+++ b/ShellCheck/Analytics.hs
@@ -134,6 +134,7 @@ basicChecks = [
     ,checkSshHereDoc
     ,checkSshCommandString
     ,checkGlobsAsOptions
+    ,checkWhileReadPitfalls
     ]
 treeChecks = [
     checkUnquotedExpansions
@@ -1781,3 +1782,38 @@ checkGlobsAsOptions (T_SimpleCommand _ _ args) =
             _ -> False
 
 checkGlobsAsOptions _ = return ()
+
+
+prop_checkWhileReadPitfalls1 = verify checkWhileReadPitfalls "while read foo; do ssh $foo uptime; done < file"
+prop_checkWhileReadPitfalls2 = verifyNot checkWhileReadPitfalls "while read -u 3 foo; do ssh $foo uptime; done 3< file"
+prop_checkWhileReadPitfalls3 = verifyNot checkWhileReadPitfalls "while true; do ssh host uptime; done"
+prop_checkWhileReadPitfalls4 = verifyNot checkWhileReadPitfalls "while read foo; do ssh $foo hostname < /dev/null; done"
+prop_checkWhileReadPitfalls5 = verifyNot checkWhileReadPitfalls "while read foo; do echo ls | ssh $foo; done"
+prop_checkWhileReadPitfalls6 = verifyNot checkWhileReadPitfalls "while read foo <&3; do ssh $foo; done 3< foo"
+
+checkWhileReadPitfalls (T_WhileExpression id [command] contents)
+        | isStdinReadCommand command = do
+    mapM_ checkMuncher contents
+  where
+    munchers = [ "ssh", "ffmpeg", "mplayer" ]
+
+    isStdinReadCommand (T_Pipeline _ [T_Redirecting id redirs cmd]) =
+        let plaintext = deadSimple cmd
+        in head (plaintext ++ [""]) == "read"
+            && (not $ "-u" `elem` plaintext)
+            && all (not . stdinRedirect) redirs
+    isStdinReadCommand _ = False
+
+    checkMuncher (T_Pipeline _ ((T_Redirecting _ redirs cmd):_)) = do
+        let name = fromMaybe "" $ getCommandBasename cmd
+        when ((not . any stdinRedirect $ redirs) && (name `elem` munchers)) $ do
+            info id 2095 $
+                name ++ " may swallow stdin, preventing this loop from working properly."
+            warn (getId cmd) 2095 $
+                "Add < /dev/null to prevent " ++ name ++ " from swallowing stdin."
+    checkMuncher _ = return ()
+
+    stdinRedirect (T_FdRedirect _ fd _)
+        | fd == "" || fd == "0" = True
+    stdinRedirect _ = False
+checkWhileReadPitfalls _ = return ()
diff --git a/ShellCheck/Parser.hs b/ShellCheck/Parser.hs
index 7013547..26fb4d2 100644
--- a/ShellCheck/Parser.hs
+++ b/ShellCheck/Parser.hs
@@ -308,7 +308,7 @@ readConditionContents single = do
         id <- getNextId
         x <- try (string "||" <|> string "-o")
         when (single && x == "||") $ addNoteFor id $ Note ErrorC 1024 "You can't use || inside [..]. Use [[..]] instead."
-        when (not single && x == "-o") $ addNoteFor id $ Note ErrorC 1025 "In [[..]], use && instead of -o."
+        when (not single && x == "-o") $ addNoteFor id $ Note ErrorC 1025 "In [[..]], use || instead of -o."
         softCondSpacing
         return $ TC_Or id typ x