Enable strict strings mode for `SQLite3Adapter`

Co-authored-by: Jean Boussier <jean.boussier@gmail.com>

Gemfile.lock

@@ -499,7 +499,7 @@ GEM
       actionpack (>= 5.2)
       activesupport (>= 5.2)
       sprockets (>= 3.0.0)
-    sqlite3 (1.4.2)
+    sqlite3 (1.4.3)
     stackprof (0.2.17)
     stimulus-rails (1.0.2)
       railties (>= 6.0.0)

activerecord/CHANGELOG.md

@@ -29,6 +29,27 @@
 
     *Cameron Bothner and Mitch Vollebregt*
 
+*   Enable strict strings mode for `SQLite3Adapter`.
+
+    Configures SQLite with a strict strings mode, which disables double-quoted string literals.
+
+    SQLite has some quirks around double-quoted string literals.
+    It first tries to consider double-quoted strings as identifier names, but if they don't exist
+    it then considers them as string literals. Because of this, typos can silently go unnoticed.
+    For example, it is possible to create an index for a non existing column.
+    See [SQLite documentation](https://www.sqlite.org/quirks.html#double_quoted_string_literals_are_accepted) for more details.
+
+    If you don't want this behavior, you can disable it via:
+
+    ```ruby
+    # config/application.rb
+    config.active_record.sqlite3_adapter_strict_strings_by_default = false
+    ```
+
+    Fixes #27782.
+
+    *fatkodima*, *Jean Boussier*
+
 *   Resolve issue where a relation cache_version could be left stale.
 
     Previously, when `reset` was called on a relation object it did not reset the cache_versions

activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb

@@ -84,11 +84,11 @@ module ActiveRecord
           table_sql = query_value(<<-SQL, "SCHEMA")
             SELECT sql
             FROM sqlite_master
-            WHERE name = #{quote_table_name(table_name)} AND type = 'table'
+            WHERE name = #{quote(table_name)} AND type = 'table'
             UNION ALL
             SELECT sql
             FROM sqlite_temp_master
-            WHERE name = #{quote_table_name(table_name)} AND type = 'table'
+            WHERE name = #{quote(table_name)} AND type = 'table'
           SQL
 
           table_sql.to_s.scan(/CONSTRAINT\s+(?<name>\w+)\s+CHECK\s+\((?<expression>(:?[^()]|\(\g<expression>\))+)\)/i).map do |name, expression|

activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb

@@ -34,7 +34,7 @@ module ActiveRecord
 
       db = SQLite3::Database.new(
         config[:database].to_s,
-        config.merge(results_as_hash: true)
+        config.merge(results_as_hash: true, strict: ConnectionAdapters::SQLite3Adapter.strict_strings_by_default)
       )
 
       ConnectionAdapters::SQLite3Adapter.new(db, logger, nil, config)
@@ -61,6 +61,16 @@ module ActiveRecord
       include SQLite3::SchemaStatements
       include SQLite3::DatabaseStatements
 
+      ##
+      # :singleton-method:
+      # Configure the SQLite3Adapter to be used in a strict strings mode.
+      # This will disable double-quoted string literals, because otherwise typos can silently go unnoticed.
+      # For example, it is possible to create an index for a non existing column.
+      # If you wish to enable this mode you can add the following line to your application.rb file:
+      #
+      #   config.active_record.sqlite3_adapter_strict_strings_by_default = true
+      class_attribute :strict_strings_by_default, default: false
+
       NATIVE_DATABASE_TYPES = {
         primary_key:  "integer PRIMARY KEY AUTOINCREMENT NOT NULL",
         string:       { name: "varchar" },

activerecord/lib/active_record/railtie.rb

@@ -220,6 +220,14 @@ To keep using the current cache store, you can turn off cache versioning entirel
       end
     end
 
+    initializer "active_record.sqlite3_adapter_strict_strings_by_default" do
+      if config.active_record.sqlite3_adapter_strict_strings_by_default
+        ActiveSupport.on_load(:active_record_sqlite3adapter) do
+          self.strict_strings_by_default = true
+        end
+      end
+    end
+
     initializer "active_record.set_configs" do |app|
       configs = app.config.active_record
 
@@ -246,6 +254,7 @@ To keep using the current cache store, you can turn off cache versioning entirel
           :query_log_tags,
           :cache_query_log_tags,
           :sqlite3_production_warning,
+          :sqlite3_adapter_strict_strings_by_default,
           :check_schema_cache_dump_version,
           :use_schema_cache_dump
         )

activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb

@@ -141,7 +141,7 @@ module ActiveRecord
           assert_equal 2, result.columns.length
           assert_equal %w{ id data }, result.columns
 
-          @conn.exec_query('INSERT INTO ex (id, data) VALUES (1, "foo")')
+          @conn.exec_query("INSERT INTO ex (id, data) VALUES (1, 'foo')")
           result = @conn.exec_query("SELECT id, data FROM ex")
           assert_equal 1, result.rows.length
           assert_equal 2, result.columns.length
@@ -152,7 +152,7 @@ module ActiveRecord
 
       def test_exec_query_with_binds
         with_example_table "id int, data string" do
-          @conn.exec_query('INSERT INTO ex (id, data) VALUES (1, "foo")')
+          @conn.exec_query("INSERT INTO ex (id, data) VALUES (1, 'foo')")
           result = @conn.exec_query(
             "SELECT id, data FROM ex WHERE id = ?", nil, [Relation::QueryAttribute.new(nil, 1, Type::Value.new)])
 
@@ -165,7 +165,7 @@ module ActiveRecord
 
       def test_exec_query_typecasts_bind_vals
         with_example_table "id int, data string" do
-          @conn.exec_query('INSERT INTO ex (id, data) VALUES (1, "foo")')
+          @conn.exec_query("INSERT INTO ex (id, data) VALUES (1, 'foo')")
 
           result = @conn.exec_query(
             "SELECT id, data FROM ex WHERE id = ?", nil, [Relation::QueryAttribute.new("id", "1-fuu", Type::Integer.new)])
@@ -616,6 +616,25 @@ module ActiveRecord
         end
       end
 
+      def test_strict_strings_by_default
+        conn = Base.sqlite3_connection(database: ":memory:", adapter: "sqlite3")
+        conn.create_table :testings
+
+        assert_nothing_raised do
+          conn.add_index :testings, :non_existent
+        end
+
+        with_strict_strings_by_default do
+          conn = Base.sqlite3_connection(database: ":memory:", adapter: "sqlite3")
+          conn.create_table :testings
+
+          error = assert_raises(StandardError) do
+            conn.add_index :testings, :non_existent2
+          end
+          assert_match(/no such column: non_existent2/, error.message)
+        end
+      end
+
       private
         def assert_logged(logs)
           subscriber = SQLSubscriber.new
@@ -633,6 +652,13 @@ module ActiveRecord
           SQL
           super(@conn, table_name, definition, &block)
         end
+
+        def with_strict_strings_by_default
+          SQLite3Adapter.strict_strings_by_default = true
+          yield
+        ensure
+          SQLite3Adapter.strict_strings_by_default = false
+        end
     end
   end
 end

activerecord/test/config.example.yml

@@ -94,9 +94,11 @@ connections:
     arunit:
       database: <%= FIXTURES_ROOT %>/fixture_database.sqlite3
       timeout:  5000
+      strict:   true
     arunit2:
       database: <%= FIXTURES_ROOT %>/fixture_database_2.sqlite3
       timeout:  5000
+      strict:   true
 
   sqlite3_mem:
     arunit:

guides/source/configuring.md

@@ -66,6 +66,7 @@ Below are the default values associated with each target version. In cases of co
 - [`config.active_support.default_message_verifier_serializer`](#config-active-support-default-message-verifier-serializer): `:json`
 - [`config.action_controller.allow_deprecated_parameters_hash_equality`](#config-action-controller-allow-deprecated-parameters-hash-equality): `false`
 - [`config.log_file_size`](#config-log-file-size): `100.megabytes`
+- [`config.active_record.sqlite3_adapter_strict_strings_by_default`](#config-active-record-sqlite3-adapter-strict-strings-by-default): `false`
 
 #### Default Values for Target Version 7.0
 
@@ -985,6 +986,24 @@ regular expressions.
 
 Specifies if source locations of methods that call database queries should be logged below relevant queries. By default, the flag is `true` in development and `false` in all other environments.
 
+#### `config.active_record.sqlite3_adapter_strict_strings_by_default`
+
+Specifies whether the SQLite3Adapter should be used in a strict strings mode.
+The use of a strict strings mode disables double-quoted string literals.
+
+SQLite has some quirks around double-quoted string literals.
+It first tries to consider double-quoted strings as identifier names, but if they don't exist
+it then considers them as string literals. Because of this, typos can silently go unnoticed.
+For example, it is possible to create an index for a non existing column.
+See [SQLite documentation](https://www.sqlite.org/quirks.html#double_quoted_string_literals_are_accepted) for more details.
+
+The default value depends on the `config.load_defaults` target version:
+
+| Starting with version | The default value is |
+| --------------------- | -------------------- |
+| (original)            | `false`              |
+| 7.1                   | `true`               |
+
 #### `config.active_record.async_query_executor`
 
 Specifies how asynchronous queries are pooled.

guides/source/upgrading_ruby_on_rails.md

@@ -262,6 +262,23 @@ config.cache_store = :mem_cache_store, "cache.example.com", pool: false
 
 See the [caching with Rails](https://guides.rubyonrails.org/caching_with_rails.html#connection-pool-options) guide for more information.
 
+### `SQLite3Adapter` now configured to be used in a strict strings mode
+
+The use of a strict strings mode disables double-quoted string literals.
+
+SQLite has some quirks around double-quoted string literals.
+It first tries to consider double-quoted strings as identifier names, but if they don't exist
+it then considers them as string literals. Because of this, typos can silently go unnoticed.
+For example, it is possible to create an index for a non existing column.
+See [SQLite documentation](https://www.sqlite.org/quirks.html#double_quoted_string_literals_are_accepted) for more details.
+
+If you don't want to use `SQLite3Adapter` in a strict mode, you can disable this behavior:
+
+```ruby
+# config/application.rb
+config.active_record.sqlite3_adapter_strict_strings_by_default = false
+```
+
 Upgrading from Rails 6.1 to Rails 7.0
 -------------------------------------
 

railties/lib/rails/application/configuration.rb

@@ -284,6 +284,10 @@ module Rails
           if respond_to?(:action_controller)
             action_controller.allow_deprecated_parameters_hash_equality = false
           end
+
+          if respond_to?(:active_record)
+            active_record.sqlite3_adapter_strict_strings_by_default = true
+          end
         else
           raise "Unknown version #{target_version.to_s.inspect}"
         end

railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_1.rb.tt

@@ -35,3 +35,12 @@
 # state which matches what was committed to the database, typically the last
 # instance to save.
 # Rails.application.config.active_record.run_commit_callbacks_on_first_saved_instances_in_transaction = false
+
+# Configures SQLite with a strict strings mode, which disables double-quoted string literals.
+#
+# SQLite has some quirks around double-quoted string literals.
+# It first tries to consider double-quoted strings as identifier names, but if they don't exist
+# it then considers them as string literals. Because of this, typos can silently go unnoticed.
+# For example, it is possible to create an index for a non existing column.
+# See https://www.sqlite.org/quirks.html#double_quoted_string_literals_are_accepted for more details.
+# Rails.application.config.active_record.sqlite3_adapter_strict_strings_by_default = true

railties/test/application/configuration_test.rb

@@ -2408,6 +2408,40 @@ module ApplicationTests
       assert_equal false, ActiveRecord::Base.run_commit_callbacks_on_first_saved_instances_in_transaction
     end
 
+    test "SQLite3Adapter.strict_strings_by_default is true by default for new apps" do
+      app_file "config/initializers/active_record.rb", <<~RUBY
+        ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
+      RUBY
+
+      app "development"
+
+      assert_equal true, ActiveRecord::ConnectionAdapters::SQLite3Adapter.strict_strings_by_default
+    end
+
+    test "SQLite3Adapter.strict_strings_by_default is false by default for upgraded apps" do
+      remove_from_config '.*config\.load_defaults.*\n'
+      app_file "config/initializers/active_record.rb", <<~RUBY
+        ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
+      RUBY
+
+      app "development"
+
+      assert_equal false, ActiveRecord::ConnectionAdapters::SQLite3Adapter.strict_strings_by_default
+    end
+
+    test "SQLite3Adapter.strict_strings_by_default can be configured via config.active_record.sqlite3_adapter_strict_strings_by_default" do
+      remove_from_config '.*config\.load_defaults.*\n'
+      add_to_config "config.active_record.sqlite3_adapter_strict_strings_by_default = true"
+
+      app_file "config/initializers/active_record.rb", <<~RUBY
+        ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
+      RUBY
+
+      app "development"
+
+      assert_equal true, ActiveRecord::ConnectionAdapters::SQLite3Adapter.strict_strings_by_default
+    end
+
     test "ActiveSupport::MessageEncryptor.use_authenticated_message_encryption is true by default for new apps" do
       app "development"