From 35fdaecb8341cf56f3cdb8456ce7c29a6aa4add9 Mon Sep 17 00:00:00 2001
From: Ben Hadley-Evans <ben@ebip.co.uk>
Date: Fri, 7 Oct 2016 21:36:49 +0100
Subject: [PATCH] [Fix #3521] Disable autocorrect on `Security/JSONLoad` by
 default.

The autocorrect on this cop is dangerous for at least two reasons depending on the value being passed to the `JSON` class method, so this disabled it by default. One known reason is where we pass in a stream such as `JSON.load(open('file'))`, this cannot be swapped out for `JSON.parse` without calling `#read` on the stream. Another known reason is where the JSON string is a single value, rather than a full JSON object, such as `JSON.load('false')`, this cannot be swapped out for `JSON.parse` without adding the `quirks_mode: true` option.

Also slightly improve the description in the cop and `enabled.yml`.

Also fix the offence message.
---
 CHANGELOG.md                                |  1 +
 config/enabled.yml                          |  8 +++++++-
 lib/rubocop/cop/security/json_load.rb       | 13 +++++++++++--
 spec/rubocop/cop/security/json_load_spec.rb |  2 +-
 4 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e469bf7a897b..5a2b078a865f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -35,6 +35,7 @@
 * [#3510](https://github.com/bbatsov/rubocop/issues/3510): Fix some issues with `Style/SafeNavigation`. Fix auto-correct of multiline if expressions, and do not register an offense for scenarios using `||` and ternary expression. ([@rrosenblum][])
 * [#3503](https://github.com/bbatsov/rubocop/issues/3503): Change misleading message of `Style/EmptyLinesAroundAccessModifier`. ([@bquorning][])
 * [#3407](https://github.com/bbatsov/rubocop/issues/3407): Turn off autocorrect for unsafe rules by default. ([@ptarjan][])
+* [#3521](https://github.com/bbatsov/rubocop/issues/3521): Turn off autocorrect for `Security/JSONLoad` by default. ([@savef][])
 
 ## 0.43.0 (2016-09-19)
 
diff --git a/config/enabled.yml b/config/enabled.yml
index 6131cf488599..e7afa303c988 100644
--- a/config/enabled.yml
+++ b/config/enabled.yml
@@ -1514,5 +1514,11 @@ Rails/Validation:
   Enabled: true
 
 Security/JSONLoad:
-  Description : 'Prefer usage of JSON.parse'
+  Description: >-
+                 Prefer usage of `JSON.parse` over `JSON.load` due to potential
+                 security issues. See reference for more information.
+  Reference: 'http://ruby-doc.org/stdlib-2.3.0/libdoc/json/rdoc/JSON.html#method-i-load'
   Enabled: true
+  # Autocorrect here will change to a method that may cause crashes depending
+  # on the value of the argument.
+  AutoCorrect: false
diff --git a/lib/rubocop/cop/security/json_load.rb b/lib/rubocop/cop/security/json_load.rb
index 5e11964870e5..ccd3393f8749 100644
--- a/lib/rubocop/cop/security/json_load.rb
+++ b/lib/rubocop/cop/security/json_load.rb
@@ -3,7 +3,16 @@
 module RuboCop
   module Cop
     module Security
-      # This cop checks for the use of unsecure JSON methods.
+      # This cop checks for the use of JSON class methods which have potential
+      # security issues.
+      #
+      # Autocorrect is disabled by default because it's potentially dangerous.
+      # If using a stream, like `JSON.load(open('file'))`, it will need to call
+      # `#read` manually, like `JSON.parse(open('file').read)`.
+      # If reading single values (rather than proper JSON objects), like
+      # `JSON.load('false')`, it will need to pass the `quirks_mode: true`
+      # option, like `JSON.parse('false', quirks_mode: true)`.
+      # Other similar issues may apply.
       #
       # @example
       #   # always offense
@@ -14,7 +23,7 @@ module Security
       #   JSON.parse("{}")
       #
       class JSONLoad < Cop
-        MSG = 'Prefer `JSON.parse` instead of `JSON#%s`.'.freeze
+        MSG = 'Prefer `JSON.parse` over `JSON.%s`.'.freeze
 
         def_node_matcher :json_load, <<-END
           (send (const nil :JSON) ${:load :restore} ...)
diff --git a/spec/rubocop/cop/security/json_load_spec.rb b/spec/rubocop/cop/security/json_load_spec.rb
index 3add926d0e88..11c476cc9f1f 100644
--- a/spec/rubocop/cop/security/json_load_spec.rb
+++ b/spec/rubocop/cop/security/json_load_spec.rb
@@ -29,7 +29,7 @@
     it "registers an offense for JSON.#{method}" do
       inspect_source(cop, "JSON.#{method}('{}')")
       expect(cop.offenses.size).to eq(1)
-      expect(cop.offenses.first.message).to include("JSON##{method}")
+      expect(cop.offenses.first.message).to include("JSON.#{method}")
     end
 
     it "autocorrects '.#{method}' to '.parse'" do