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

Ascending order - Click to sort in descending order
Hide
Permalink
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
Hide
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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.
Hide
Permalink
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
Permalink
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.

People

Vote (0)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4#649-r158309) | Report a problem
Powered by a free Atlassian JIRA open source license for Clojure. Try JIRA - bug tracking software for your team.