Skip to:

  1. Skip to Jira Navigation
  2. Skip to Side Navigation
  3. Skip to Main Content

Change *read-eval* default value to false

Description

Discussion on the security implications of read-eval defaulting to true here: https://groups.google.com/forum/?fromgroups=#!topic/clojure/qUk-bM0JSGc

I'm not sure whether the unit test that needs read-eval true in order to pass is a sign of lots of other code that would break if read-eval defaulted to false.

Environment

None

Attachments

2

Activity

Show:

Rich Hickey February 9, 2013 at 3:17 PM

This ticket is an excellent example of a terrible ticket.

1) It does not lead with a problem statement. The title takes the form akin to "I want X". It proposes a tactic.

2) The description is woefully inadequate. Here too, no problem statement. Descriptions should point to any discussions, but discussions are long and rambling, and no substitute for a succinct problem statement in the description. Descriptions need to be maintained, with the current strategy and name of best patch.

3) No resolution strategy. Just patches attached. How is a reviewer supposed to assess your patch if you don't even bother stating what the problem is and what your approach will be in fixing it, how that approach does fix it, and what if any tradeoffs are being made?

4) The change being requested would be a breaking change. That should be made extremely clear in the description, and doubles the threshold for quality of motivation and implementation.

5) Any breaking change would have to carefully enumerate what would break and when, what the migration strategy would be etc.

6) The patch breaks the compiler. If you don't understand how the compiler works, and why features are there, you shouldn't submit patches that alter it. The only assessments made in comments are 'it works for me', which, while useful anecdotes, are insufficient.

While the ticket itself was bad, the uncritical rallying behind it was more troubling. This is not how Clojure was built, and will not be how it is maintained.

In the end, the ticket proposed a tactic, and that tactic has been rejected. Read safety has been addressed by other means.

Rich Hickey February 5, 2013 at 1:55 AM

Tim McCormack [personal] February 2, 2013 at 9:22 PM

I'm bumping the Priority field from its default value to "Major" to reflect recent events. (I personally feel "blocker" would be more appropriate, or at least "critical", but I don't want to step on any toes.)

Andy Fingerhut February 2, 2013 at 5:06 PM

Also removed my original didn't-fix-enough-things patch from Jan 30 2013.

Andy Fingerhut February 2, 2013 at 5:05 PM

clj-1153-patch-v2.txt dated Feb 2 2013 is functionally identical to Chas Emerick's CLJ-1185.patch dated Feb 1 2013, and no retesting is needed by anyone because of it.

The only change I have made to his patch is: the doc string for read-eval now says its default value is false.

Completed

Details

Assignee

Reporter

Patch

Code

Priority

Affects versions

Created January 30, 2013 at 4:49 PM
Updated September 14, 2017 at 1:04 PM
Resolved September 14, 2017 at 1:04 PM