Fixed sqlite_adapter to support SecureRandom.hex by adding US_ASCII encoding to type_cast. #5226

Closed
wants to merge 1 commit into
from

2 participants

@seryl

Setting a String column to SecureRandom.hex causes a no method error.

SecureRandom.hex returns a :string type, only without utf-8 or ASCII_8BIT encodings.

But this makes it all better.

I figured we should add this since SecureRandom is part of core ruby.

@seryl seryl Added US_ASCII encoding to sqlite type_cast.
* Fixes an issue trying to set a string from SecureRandom.hex (which has US_ASCII encoding)
9e1e869
@tenderlove
Ruby on Rails member

I don't understand the bug. Can you show some demo code that causes the problem?

@seryl

Given a schema:

ActiveRecord::Schema.define(:version => 1) do

  create_table "sample_table", :force => true do |t|
    t.string "broken_string"
  end

end

And corresponding model:

class SampleTable < ActiveRecord::Base
  attr_accessible :broken_string
end

I have this bin file:

#!/usr/bin/env ruby
$:.unshift File.join(File.dirname(__FILE__), '..', 'app')

require 'break_sqlite'

BreakSqlite.new

which loads app/break_sqlite.rb:

$:.unshift File.dirname(__FILE__)
require 'securerandom'
require 'active_record'
require 'models/sample_table'

class BreakSqlite
  DATABASE_CONFIG = {
    :adapter => "sqlite3",
    :database => "db/development.sqlite3"
  }

  def initialize
    ActiveRecord::Base.establish_connection(DATABASE_CONFIG)
    ActiveRecord::Base.connection

    require 'pry'
    binding.pry
  end

  def example
    st = SampleTable.new
    st.broken_string = SecureRandom.hex
    st.save
  end
end

Breaks with:

jtoft@mogu: Desktop/testing> rake db:migrate          
==  CreateSampleTables: migrating =============================================
-- create_table(:sample_tables)
   -> 0.0007s
==  CreateSampleTables: migrated (0.0008s) ====================================

jtoft@mogu: Desktop/testing> ./bin/break_sqlite 

From: /Users/jtoft/Desktop/testing/app/break_sqlite.rb @ line 17 in BreakSqlite#initialize:
    12:   def initialize
    13:     ActiveRecord::Base.establish_connection(DATABASE_CONFIG)
    14:     ActiveRecord::Base.connection
    15:     
    16:     require 'pry'
 => 17:     binding.pry
    18:   end
    19:   
    20:   def example
    21:     st = SampleTable.new
    22:     st.broken_string = SecureRandom.hex
[1] pry(#<BreakSqlite>)> example
ActiveRecord::StatementInvalid: NoMethodError: undefined method `error' for nil:NilClass: INSERT INTO "sample_tables" ("broken_string") VALUES (?)
from /Users/jtoft/.rvm/gems/ruby-1.9.3-p0@testing/gems/activerecord-3.2.1/lib/active_record/connection_adapters/sqlite_adapter.rb:212:in `type_cast'
[2] pry(#<BreakSqlite>)>

The problem occurs because of the following:

jtoft@mogu: Desktop/testing> sed '211,214!d' ~/.rvm/gems/ruby-1.9.3-p0@testing/gems/activerecord-3.2.1/lib/active_record/connection_adapters/sqlite_adapter.rb
  if column.type == :string && value.encoding == Encoding::ASCII_8BIT
    @logger.error "Binary data inserted for `string` type on column `#{column.name}`"
    value.encode! 'utf-8'
  end

When you pry into the sqlite_adapter, you'll see that the column.type here is :string, but the value.encoding is Encoding::US_ASCII.

So the exception is thrown because US_ASCII isn't handled.

Seem reasonable?

@tenderlove tenderlove closed this in b5c939d Mar 2, 2012
@carlosantoniodasilva carlosantoniodasilva pushed a commit to carlosantoniodasilva/rails that referenced this pull request Mar 3, 2012
@tenderlove tenderlove only log an error if there is a logger. fixes #5226
Conflicts:

	activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
af02291
@carlosantoniodasilva carlosantoniodasilva pushed a commit to carlosantoniodasilva/rails that referenced this pull request Mar 3, 2012
@tenderlove tenderlove only log an error if there is a logger. fixes #5226
Conflicts:

	activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb

Conflicts:

	activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
b1358c8
@ttosch ttosch pushed a commit that referenced this pull request Jan 19, 2015
@tenderlove tenderlove only log an error if there is a logger. fixes #5226
Conflicts:

	activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb

Conflicts:

	activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
cab456d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment