Clojure

Setting clojure.read.eval to unknown on JVM cmd line causes --eval option to fail

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: Release 1.5
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • 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"]

Activity

Hide
Andy Fingerhut added a comment -

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

Show
Andy Fingerhut added a comment - 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
Andy Fingerhut made changes -
Field Original Value New Value
Attachment clj-1168-patch-v1.txt [ 11864 ]
Hide
Steve Miner added a comment - - edited

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)))
Show
Steve Miner added a comment - - edited 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)))
Hide
Andy Fingerhut added a comment -

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.

Show
Andy Fingerhut added a comment - 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.
Hide
Andy Fingerhut added a comment -

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.

Show
Andy Fingerhut added a comment - 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.
Hide
Steve Miner added a comment -

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.

Show
Steve Miner added a comment - 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.
Hide
Stuart Halloway added a comment -

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.

Show
Stuart Halloway added a comment - 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.
Stuart Halloway made changes -
Patch Code [ 10001 ]
Approval Screened [ 10004 ]
Attachment CLJ-1168-with-read-known.patch [ 11873 ]
Rich Hickey made changes -
Approval Screened [ 10004 ] Ok [ 10007 ]
Hide
Tim McCormack added a comment -

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

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

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.
Show
Tim McCormack added a comment - 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.
Stuart Halloway made changes -
Resolution Completed [ 1 ]
Status Open [ 1 ] Closed [ 6 ]

People

Vote (0)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: