<< Back to previous view

[CLJ-1168] Setting clojure.read.eval to unknown on JVM cmd line causes --eval option to fail Created: 19/Feb/13  Updated: 22/Feb/13  Resolved: 22/Feb/13

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File clj-1168-patch-v1.txt     Text File CLJ-1168-with-read-known.patch    
Patch: Code
Approval: Ok

 Description   

First discovered by Tim McCormack with Clojure 1.5.0-beta13 and -RC15:

$ java -Dclojure.read.eval=unknown -jar ~/.m2/repository/org/clojure/clojure/1.5.0-RC16/clojure-1.5.0-RC16.jar -e "(+ 1 2)"
Exception in thread "main" java.lang.RuntimeException: Reading disallowed - read-eval bound to :unknown

This would prevent a Leiningen user from putting the following line in their project.clj file in order to look for read or read-string calls that don't explicitly bind read-eval to true or false:

:jvm-opts ["-Dclojure.read.eval=unknown"]



 Comments   
Comment by Andy Fingerhut [ 19/Feb/13 7:42 PM ]

Patch clj-1168-patch-v1.txt dated Feb 19 2013 has been tested manually with these results:

% java -cp clojure.jar clojure.main

(ran some simple tests to ensure repl still works, and obeys -Dclojure.read.eval=unknown setting)

% java -Dclojure.read.eval=unknown -jar clojure.jar -e '(println (+ 1 2))'
3

Comment by Steve Miner [ 20/Feb/13 4:52 PM ]

The patch works for me, but I think it would be safer for the patch to treat *read-eval* :unknown as false for the purpose of -e. It's unlikely that anyone wants to -e '#=(boom)'. Also, there's no need for the let to capture the cur-read-eval. I suggest something like this for the patch (updated):

(defn- read-never-unknown [read-fn & args]
      (binding [*read-eval* (true? *read-eval*)]
         (apply read-fn args)))
Comment by Andy Fingerhut [ 21/Feb/13 6:10 AM ]

I have asked on the Leiningen email list whether treating read-eval :unknown as false for the purpose of -e would break Leiningen functionality, and will report back here when I find out.

Comment by Andy Fingerhut [ 21/Feb/13 3:02 PM ]

I don't have an answer from Leiningen folks yet, but I do have another thought related to Steve's comment:

The current behavior is to treat *read-eval* as true for the purposes of reading lines in the REPL, if -Dclojure.read.eval=unknown was specified on the command line. Why should expressions given after -e be any more restricted? Whoever is invoking the JVM has complete control.

If they really want *read-eval* to be false when reading the expressions after -e, just use -Dclojure.read.eval=false instead of =unknown.

Comment by Steve Miner [ 21/Feb/13 3:45 PM ]

You're right, the user has control. I was just thinking that the user might not appreciate the details and it would be safer to default to the false behavior. However, I failed to consider the REPL treatment – that's a good point.

In any case, the logic for the binding in the patch could be simplified. I think this would work for the intended result:

(defn- read-never-unknown [read-fn & args]
      (binding [*read-eval* (and *read-eval* true)]
         (apply read-fn args)))

Sorry, if my comments have descended into bike-shedding.

Comment by Stuart Halloway [ 22/Feb/13 9:00 AM ]

I implemented "with-read-known" patch after input from Rich. Helpers that establish bindings should take a closure or fn body, rather than carrying around explicit fns and arg lists as the original 19 Feb patch does.

Marking screened as of the Feb 22 with-read-known patch.

Comment by Tim McCormack [ 22/Feb/13 9:55 AM ]

Steve: More importantly, any time you call (eval (read ...)), there's no reason for *read-eval* to be anything other than true.

Comment by Tim McCormack [ 22/Feb/13 10:12 AM ]

Newer patch works. My verification procedure:

  • Patched master (should probably use an RC instead)
  • Compiled and installed (mvn install) with version = 1.5.0-clj1168 in pom.xml
  • Set up fresh Leiningen project including these options:
    :dependencies [[org.clojure/clojure "1.5.0-clj1168"]]
    :jvm-opts ["-Dclojure.read.eval=unknown"]
  • Verify: `lein repl` succeeds
    • Verify: => (read-string "foo") fails with "Reading disallowed - *read-eval* bound to :unknown"
    • Verify: => #=(+ 1 2) prints 3 (read is fully available to REPL)
  • Write -main fn in project as
    (defn -main [& args]
      (println *read-eval*)
      (println (read-string (first args))))
  • Verify: `lein run foo` prints :unknown and then fails with "Reading disallowed" error.
  • Bind *read-eval* to false in -main
  • Verify: `lein run foo` prints foo.
Generated at Sat Dec 20 16:20:30 CST 2014 using JIRA 4.4#649-r158309.