Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Replace nil checks in Quoting with a NullColumn. #14091

Closed
wants to merge 1 commit into from

4 participants

@patricksrobertson

This commit takes the nil check in the Connection adapters and replaces it with an
implementation of the NullObject pattern to avoid uncessary code branching. I would
have preferred to inject the NullColumn object directly, instead of using a nilguard,
but was unable to locate the code that passes a nil argument into the quote method.

Even without that injection, this eliminates three code branches that weren't necessary.

@patricksrobertson patricksrobertson Replace nil checks in quoting with a NullColumn.
This commit takes the nil check in the Connection adapters and replaces it with an
implementation of the NullObject pattern to avoid uncessary code branching. I would
have preferred to inject the NullColumn object directly, instead of using a nilguard,
but was unable to locate the code that passes a nil argument into the quote method.
b428e07
@senny
Owner

Internals have changed since this PR was submitted. I'm not sure it's still necessary.

/cc @sgrif

@sgrif
Collaborator

That is correct.

@sgrif sgrif closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 18, 2014
  1. @patricksrobertson

    Replace nil checks in quoting with a NullColumn.

    patricksrobertson authored
    This commit takes the nil check in the Connection adapters and replaces it with an
    implementation of the NullObject pattern to avoid uncessary code branching. I would
    have preferred to inject the NullColumn object directly, instead of using a nilguard,
    but was unable to locate the code that passes a nil argument into the quote method.
This page is out of date. Refresh to see the latest.
View
2  activerecord/lib/active_record/connection_adapters/abstract/quoting.rb
@@ -6,13 +6,13 @@ module Quoting
# Quotes the column value to help prevent
# {SQL injection attacks}[http://en.wikipedia.org/wiki/SQL_injection].
def quote(value, column = nil)
+ column ||= NullColumn.new
# records are quoted as their primary key
return value.quoted_id if value.respond_to?(:quoted_id)
case value
when String, ActiveSupport::Multibyte::Chars
value = value.to_s
- return "'#{quote_string(value)}'" unless column
case column.type
when :integer then value.to_i.to_s
View
1  activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -12,6 +12,7 @@ module ConnectionAdapters # :nodoc:
extend ActiveSupport::Autoload
autoload :Column
+ autoload :NullColumn
autoload :ConnectionSpecification
autoload_at 'active_record/connection_adapters/abstract/schema_definitions' do
View
26 activerecord/lib/active_record/connection_adapters/null_column.rb
@@ -0,0 +1,26 @@
+module ActiveRecord
+ module ConnectionAdapters # :nodoc:
+ # NullColumn is an implementation of the NullObject pattern that
+ # allows the quoting modules in the ConnectionAdapters to avoid
+ # checking whether a column was passed in or not. It implements
+ # just enough functionality to allow the default behavior of
+ # quoting to occur.
+ class NullColumn # :nodoc:
+ def type
+ :null
+ end
+
+ def limit
+ nil
+ end
+
+ def precision
+ nil
+ end
+
+ def scale
+ nil
+ end
+ end
+ end
+end
View
3  activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb
@@ -16,8 +16,7 @@ def unescape_bytea(value)
# Quotes PostgreSQL-specific data types for SQL input.
def quote(value, column = nil) #:nodoc:
- return super unless column
-
+ column ||= ActiveRecord::ConnectionAdapters::NullColumn.new
sql_type = type_to_sql(column.type, column.limit, column.precision, column.scale)
case value
View
3  activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
@@ -226,7 +226,8 @@ def supports_explain?
# QUOTING ==================================================
def quote(value, column = nil)
- if value.kind_of?(String) && column && column.type == :binary
+ column ||= ActiveRecord::ConnectionAdapters::NullColumn.new
+ if value.kind_of?(String) && column.type == :binary
s = value.unpack("H*")[0]
"x'#{s}'"
else
Something went wrong with that request. Please try again.