From 391ba65c3975196b3051cd83c638b4d6263773ad Mon Sep 17 00:00:00 2001
From: Chris Perkins <chrisperkins99@gmail.com>
Date: Thu, 14 Apr 2016 09:04:00 -0600
Subject: [PATCH] Don't step in to clojure.core functions

The new step-in command makes it easy to step in (often accidentally) to
a function in clojure.core. But instrumenting clojure core functions is
dangerous, and can result in a stack overflow that crashes the nrepl
process. In fact, just instrumenting `clojure.core/symbol?` (without
even debugging it) causes a stack overflow. I'm not entirely sure yet
how this happens, but it seems safest to just prevent stepping in to all
core functions by default.
---
 src/cider/nrepl/middleware/debug.clj          |  7 +++++
 .../middleware/debug_integration_test.clj     | 27 ++++++++++---------
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/src/cider/nrepl/middleware/debug.clj b/src/cider/nrepl/middleware/debug.clj
index ef5af593a..0451b7972 100644
--- a/src/cider/nrepl/middleware/debug.clj
+++ b/src/cider/nrepl/middleware/debug.clj
@@ -438,6 +438,12 @@
             (eval form)
             (alter-meta! v assoc ::instrumented instrumented)))))))
 
+(defn safe-to-debug?
+  "Some namespaces are not safe to debug, because doing so can cause a stack
+  overflow that crashes the nrepl process."
+  [ns]
+  (not (#{'clojure.core} (ns-name ns))))
+
 (defn step-in?
   "Return true if we can and should step in to the function in the var `v`.
   The \"should\" part is determined by the value in `step-in-to-next?`, which
@@ -464,6 +470,7 @@
       ;; We do not go so far as to actually try to read the code of the function
       ;; at this point, which is at macroexpansion time.
       (and v
+           (safe-to-debug? (:ns m))
            (not (:macro m))
            (not (:inline m))))))
 
diff --git a/test/clj/cider/nrepl/middleware/debug_integration_test.clj b/test/clj/cider/nrepl/middleware/debug_integration_test.clj
index 642aec603..f0560d9c5 100644
--- a/test/clj/cider/nrepl/middleware/debug_integration_test.clj
+++ b/test/clj/cider/nrepl/middleware/debug_integration_test.clj
@@ -543,30 +543,33 @@
     (<-- {:status ["done"]})))
 
 (deftest step-in-to-function-in-jar
-  ;; Step into clojure.core/shuffle. To do this, we need to find and instrument
-  ;; the source, which is in a jar file.
-  (--> :eval "(ns user.test.step-in)")
+  ;; Step into clojure.tools.nrepl.server/handle*. To do this, we need to find
+  ;; and instrument the source, which is in a jar file. Note that this function
+  ;; is used because it is not marked as a :source-dep, so we can rely on the
+  ;; namespace remaining unmunged, which is important when these tests run on
+  ;; travis CI.
+  (--> :eval "(ns user.test.step-in
+                (:require [clojure.tools.nrepl.server :as server]))")
   (<-- {:ns "user.test.step-in"})
   (<-- {:status ["done"]})
 
   (--> :eval
        "#dbg
-        (defn foo [c]
-          (shuffle c))")
+        (defn foo [m]
+          (server/handle* m identity 23))")
   (<-- {:value "#'user.test.step-in/foo"})
   (<-- {:status ["done"]})
 
-  (--> :eval "(foo (range 2))")
-  (<-- {:debug-value "(0 1)" :coor [3 1]})
+  (--> :eval "(foo {})")
+  (<-- {:debug-value "{}" :coor [3 1]})
   (--> :in)
 
-  (let [msg  (<-- {:debug-value "(0 1)"
-                   :coor [5 1 1 1]})
+  (let [msg  (<-- {:debug-value "{}"
+                   :coor [3 1 1 1]})
         file (:file msg)]
     (.startsWith file "jar:file:")
-    (.endsWith file "/clojure/core.clj"))
+    (.endsWith file "/clojure/tools/nrepl/server.clj"))
 
   (--> :continue)
-  (let [result (read-string (:value (<-- {})))]
-    (is (#{[0 1] [1 0]} result)))
+  (<-- {:value "{:transport 23}"})
   (<-- {:status ["done"]}))