Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions lib/couchbase-orm/n1ql.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ def self.sanitize(value)
def self.config(new_config = nil)
Thread.current['__couchbaseorm_n1ql_config__'] = new_config if new_config
Thread.current['__couchbaseorm_n1ql_config__'] || {
scan_consistency: DEFAULT_SCAN_CONSISTENCY
scan_consistency: DEFAULT_SCAN_CONSISTENCY,
adhoc: true
}
end

Expand Down Expand Up @@ -95,12 +96,17 @@ def convert_values(keys, values)
end
end

def build_where(keys, values)
def build_where(keys, values, params: nil)
where = values == NO_VALUE ? '' : keys.zip(Array.wrap(values))
.reject { |key, value| key.nil? && value.nil? }
.map { |key, value| build_match(key, value) }
.map { |key, value| build_match(key, value, params: params) }
.join(" AND ")
"type=\"#{design_document}\" #{"AND " + where unless where.blank?}"
if params
type_placeholder = bind(design_document, params)
"type=#{type_placeholder} #{"AND " + where unless where.blank?}"
else
"type=\"#{design_document}\" #{"AND " + where unless where.blank?}"
end
end

# order-by-clause ::= ORDER BY ordering-term [ ',' ordering-term ]*
Expand All @@ -119,12 +125,18 @@ def run_query(keys, values, query_fn, custom_order: nil, descending: false, limi
N1qlProxy.new(query_fn.call(bucket, values, Couchbase::Options::Query.new(**options)))
else
bucket_name = bucket.name
where = build_where(keys, values)
params = []
where = build_where(keys, values, params: params)
order = custom_order || build_order(keys, descending)
limit = build_limit(limit)
n1ql_query = "select raw meta().id from `#{bucket_name}` where #{where} order by #{order} #{limit}"
result = cluster.query(n1ql_query, Couchbase::Options::Query.new(**options))
CouchbaseOrm.logger.debug "N1QL query: #{n1ql_query} return #{result.rows.to_a.length} rows with scan_consistency : #{options[:scan_consistency]}"

adhoc = options.delete(:adhoc) { CouchbaseOrm::N1ql.config[:adhoc] }
query_options = options.merge(positional_parameters: params, adhoc: adhoc)
result = cluster.query(n1ql_query, Couchbase::Options::Query.new(**query_options))
CouchbaseOrm.logger.debug {
"N1QL query: #{n1ql_query} params: #{params.inspect} return #{result.rows.to_a.length} rows with scan_consistency: #{options[:scan_consistency]} adhoc: #{adhoc}"
}
N1qlProxy.new(result)
end
end
Expand Down
70 changes: 43 additions & 27 deletions lib/couchbase-orm/relation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,32 +20,38 @@ def to_s
end

def to_n1ql
to_n1ql_with_params.first
end

def to_n1ql_with_params
bucket_name = @model.bucket.name
where = build_where
params = []
where = build_where_with_params(params)
order = build_order
limit = build_limit
"select raw meta().id from `#{bucket_name}` where #{where} order by #{order} #{limit}"
["select raw meta().id from `#{bucket_name}` where #{where} order by #{order} #{limit}", params]
end

def execute(n1ql_query)
result = @model.cluster.query(n1ql_query, Couchbase::Options::Query.new(scan_consistency: CouchbaseOrm::N1ql.config[:scan_consistency]))
CouchbaseOrm.logger.debug { "Relation query: #{n1ql_query} return #{result.rows.to_a.length} rows with scan_consistency : #{CouchbaseOrm::N1ql.config[:scan_consistency]}" }
def execute(n1ql_query, params = [])
result = @model.cluster.query(n1ql_query, build_query_options(positional_parameters: params))
CouchbaseOrm.logger.debug { "Relation query: #{n1ql_query} params: #{params.inspect} return #{result.rows.to_a.length} rows" }
N1qlProxy.new(result)
end

def query
CouchbaseOrm::logger.debug("Query: #{self}")
n1ql_query = to_n1ql
execute(n1ql_query)
n1ql_query, params = to_n1ql_with_params
execute(n1ql_query, params)
end

def update_all(**cond)
bucket_name = @model.bucket.name
where = build_where
params = []
where = build_where_with_params(params)
limit = build_limit
update = build_update(**cond)
update = build_update_with_params(params, **cond)
n1ql_query = "update `#{bucket_name}` set #{update} where #{where} #{limit}"
execute(n1ql_query)
execute(n1ql_query, params)
end

def ids
Expand All @@ -61,14 +67,16 @@ def strict_loading?
end

def first
result = @model.cluster.query(self.limit(1).to_n1ql, Couchbase::Options::Query.new(scan_consistency: CouchbaseOrm::N1ql.config[:scan_consistency]))
n1ql_query, params = self.limit(1).to_n1ql_with_params
result = @model.cluster.query(n1ql_query, build_query_options(positional_parameters: params))
return unless (first_id = result.rows.to_a.first)

@model.find(first_id, with_strict_loading: @strict_loading)
end

def last
result = @model.cluster.query(to_n1ql, Couchbase::Options::Query.new(scan_consistency: CouchbaseOrm::N1ql.config[:scan_consistency]))
n1ql_query, params = to_n1ql_with_params
result = @model.cluster.query(n1ql_query, build_query_options(positional_parameters: params))
last_id = result.rows.to_a.last
@model.find(last_id, with_strict_loading: @strict_loading) if last_id
end
Expand Down Expand Up @@ -184,47 +192,55 @@ def build_order
order.empty? ? "meta().id" : order
end

def build_where
build_conds([[:type, @model.design_document]] + @where)
def build_where_with_params(params)
build_conds_with_params([[:type, @model.design_document]] + @where, params)
end

def build_conds(conds)
def build_conds_with_params(conds, params)
conds.map do |key, value, opt|
if key
opt == :not ?
@model.build_not_match(key, value) :
@model.build_match(key, value)
opt == :not ?
@model.build_not_match(key, value, params: params) :
@model.build_match(key, value, params: params)
else
value
end
end.join(" AND ")
end

def build_update(**cond)
def build_update_with_params(params, **cond)
cond.map do |key, value|
for_clause=""
for_clause = ""
if value.is_a?(Hash) && value[:_for]
path_clause = value.delete(:_for)
var_clause = path_clause.to_s.split(".").last.singularize

_when = value.delete(:_when)
when_clause = _when ? build_conds(_when.to_a) : ""
_set = value.delete(:_set)
when_clause = _when ? build_conds_with_params(_when.to_a, params) : ""

_set = value.delete(:_set)
value = _set if _set

for_clause = " for #{var_clause} in #{path_clause} when #{when_clause} end"
end
if value.is_a?(Hash)
value.map do |k, v|
"#{key}.#{k} = #{v}"
"#{key}.#{k} = #{@model.bind(v, params)}"
end.join(", ") + for_clause
else
"#{key} = #{@model.quote(value)}#{for_clause}"
"#{key} = #{@model.bind(value, params)}#{for_clause}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-critical critical

In build_update_with_params, when value is a Hash (used for updating nested attributes), the values are directly interpolated into the query string as #{v} without being parameterized or quoted:

if value.is_a?(Hash)
    value.map do |k, v|
        "#{key}.#{k} = #{v}"
    end.join(", ") + for_clause

This not only defeats prepared-statement caching for nested attribute updates but also introduces a N1QL injection vulnerability if user-controlled input is passed to nested attributes.

To fix this, we should parameterize these values using @model.bind(v, params) as well:

if value.is_a?(Hash)
    value.map do |k, v|
        "#{key}.#{k} = #{@model.bind(v, params)}"
    end.join(", ") + for_clause

end
end.join(", ")
end

def build_query_options(positional_parameters: [])
opts = { scan_consistency: CouchbaseOrm::N1ql.config[:scan_consistency] }
opts[:positional_parameters] = positional_parameters unless positional_parameters.empty?
adhoc = CouchbaseOrm::N1ql.config[:adhoc]
opts[:adhoc] = adhoc unless adhoc.nil?
Couchbase::Options::Query.new(**opts)
end

def method_missing(method, *args, &block)
if @model.respond_to?(method)
scoping {
Expand Down
7 changes: 6 additions & 1 deletion lib/couchbase-orm/utilities/has_many.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,12 @@ def build_index_n1ql(klass, remote_class, remote_method, through_key, foreign_ke
klass.class_eval do
n1ql remote_method, emit_key: 'id', query_fn: proc { |bucket, values, options|
raise ArgumentError, "values[0] must not be blank" if values[0].blank?
cluster.query("SELECT raw #{through_key} FROM `#{bucket.name}` where type = \"#{design_document}\" and #{foreign_key} = #{quote(values[0])}", options)
n1ql_query = "SELECT raw #{through_key} FROM `#{bucket.name}` where type = $1 and #{foreign_key} = $2"
params = [design_document, values[0]]
cluster.query(n1ql_query, Couchbase::Options::Query.new(
positional_parameters: params,
scan_consistency: options.scan_consistency
))
}
end
else
Expand Down
75 changes: 50 additions & 25 deletions lib/couchbase-orm/utilities/query_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,44 +4,67 @@ module QueryHelper

module ClassMethods

def build_match(key, value)
def serialize_for_binding(value)
if value.is_a?(Array)
value.map { |v| serialize_for_binding(v) }
elsif [DateTime, Time].any? { |clazz| value.is_a?(clazz) } || (value.respond_to?(:acts_like?) && value.acts_like?(:time))
value.iso8601(@precision || 0)
elsif value.is_a?(Date)
value.to_s
else
value
end
end

def bind(value, params)
if value.nil?
nil
else
params << serialize_for_binding(value)
"$#{params.length}"
end
end
Comment on lines +7 to +26

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

There are two important improvement opportunities in serialize_for_binding and bind:

  1. Array Parameterization (Prepared-Statement Caching): N1QL natively supports passing arrays as a single positional parameter (e.g., name IN $1). The current implementation converts arrays into individual positional parameters (e.g., name IN [$1, $2]), which produces a different query string for every different array length. This completely defeats prepared-statement caching for IN queries (which is one of the main goals of this PR). Passing the array itself as a single parameter keeps the query string identical regardless of array length, enabling prepared-statement caching.

  2. ActiveSupport::TimeWithZone Support: ActiveSupport::TimeWithZone (the default time class in Rails) does not inherit from Time or DateTime, so timezone-aware times will not be serialized to ISO8601 strings and will fail or serialize incorrectly. Adding value.acts_like?(:time) ensures robust support for Rails applications.

            def serialize_for_binding(value)
                if value.is_a?(Array)
                    value.map { |v| serialize_for_binding(v) }
                elsif [DateTime, Time].any? { |clazz| value.is_a?(clazz) } || (value.respond_to?(:acts_like?) && value.acts_like?(:time))
                    value.iso8601(@precision || 0)
                elsif value.is_a?(Date)
                    value.to_s
                else
                    value
                end
            end

            def bind(value, params)
                if value.nil?
                    nil
                else
                    params << serialize_for_binding(value)
                    "$#{params.length}"
                end
            end


def build_match(key, value, params: nil)
use_is_null = self.properties_always_exists_in_document
key = "meta().id" if key.to_s == "id"
resolve = ->(v) { params ? bind(v, params) : quote(v) }
case
when value.nil? && use_is_null
"#{key} IS NULL"
when value.nil? && !use_is_null
"#{key} IS NOT VALUED"
when value.is_a?(Hash) && attribute_types[key.to_s].is_a?(CouchbaseOrm::Types::Array)
"any #{key.to_s.singularize} in #{key} satisfies (#{build_match_hash("#{key.to_s.singularize}", value)}) end"
"any #{key.to_s.singularize} in #{key} satisfies (#{build_match_hash("#{key.to_s.singularize}", value, params: params)}) end"
when value.is_a?(Hash) && !attribute_types[key.to_s].is_a?(CouchbaseOrm::Types::Array)
build_match_hash(key, value)
build_match_hash(key, value, params: params)
when value.is_a?(Array) && value.include?(nil)
"(#{build_match(key, nil)} OR #{build_match(key, value.compact)})"
"(#{build_match(key, nil, params: params)} OR #{build_match(key, value.compact, params: params)})"
when value.is_a?(Array)
"#{key} IN #{quote(value)}"
"#{key} IN #{resolve.call(value)}"
when value.is_a?(Range)
build_match_range(key, value)
build_match_range(key, value, params: params)
else
"#{key} = #{quote(value)}"
"#{key} = #{resolve.call(value)}"
end
end

def build_match_hash(key, value)
def build_match_hash(key, value, params: nil)
matches = []
resolve = ->(v) { params ? bind(v, params) : quote(v) }
value.each do |k, v|
case k
when :_gt
matches << "#{key} > #{quote(v)}"
matches << "#{key} > #{resolve.call(v)}"
when :_gte
matches << "#{key} >= #{quote(v)}"
matches << "#{key} >= #{resolve.call(v)}"
when :_lt
matches << "#{key} < #{quote(v)}"
matches << "#{key} < #{resolve.call(v)}"
when :_lte
matches << "#{key} <= #{quote(v)}"
matches << "#{key} <= #{resolve.call(v)}"
when :_ne
matches << "#{key} != #{quote(v)}"
matches << "#{key} != #{resolve.call(v)}"

# TODO v2
# when :_in
# matches << "#{key} IN #{quote(v)}"
Expand All @@ -65,7 +88,7 @@ def build_match_hash(key, value)
# matches << "#{key} MATCH #{quote(v)}"
# when :_nmatch
# matches << "#{key} NOT MATCH #{quote(v)}"

# TODO v3
# when :_any
# matches << "#{key} ANY #{quote(v)}"
Expand All @@ -80,39 +103,41 @@ def build_match_hash(key, value)
#when :_nwithin
# matches << "#{key} NOT WITHIN #{quote(v)}"
else
matches << build_match("#{key}.#{k}", v)
matches << build_match("#{key}.#{k}", v, params: params)
end
end

matches.join(" AND ")
end

def build_match_range(key, value)
def build_match_range(key, value, params: nil)
resolve = ->(v) { params ? bind(v, params) : quote(v) }
matches = []
matches << "#{key} >= #{quote(value.begin)}"
matches << "#{key} >= #{resolve.call(value.begin)}"
if value.exclude_end?
matches << "#{key} < #{quote(value.end)}"
matches << "#{key} < #{resolve.call(value.end)}"
else
matches << "#{key} <= #{quote(value.end)}"
matches << "#{key} <= #{resolve.call(value.end)}"
end
matches.join(" AND ")
end


def build_not_match(key, value)
def build_not_match(key, value, params: nil)
use_is_null = self.properties_always_exists_in_document
key = "meta().id" if key.to_s == "id"
resolve = ->(v) { params ? bind(v, params) : quote(v) }
case
when value.nil? && use_is_null
"#{key} IS NOT NULL"
when value.nil? && !use_is_null
"#{key} IS VALUED"
when value.is_a?(Array) && value.include?(nil)
"(#{build_not_match(key, nil)} AND #{build_not_match(key, value.compact)})"
"(#{build_not_match(key, nil, params: params)} AND #{build_not_match(key, value.compact, params: params)})"
when value.is_a?(Array)
"#{key} NOT IN #{quote(value)}"
"#{key} NOT IN #{resolve.call(value)}"
else
"#{key} != #{quote(value)}"
"#{key} != #{resolve.call(value)}"
end
end

Expand Down
17 changes: 13 additions & 4 deletions spec/n1ql_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -172,19 +172,28 @@ class N1QLTest < CouchbaseOrm::Base
it "should log the default scan_consistency when n1ql query is executed" do
allow(CouchbaseOrm.logger).to receive(:debug)
N1QLTest.by_rating_reverse()
expect(CouchbaseOrm.logger).to have_received(:debug).at_least(:once).with("N1QL query: select raw meta().id from `#{CouchbaseOrm::Connection.bucket.name}` where type=\"n1_ql_test\" order by name DESC return 0 rows with scan_consistency : #{described_class::DEFAULT_SCAN_CONSISTENCY}")
expect(CouchbaseOrm.logger).to have_received(:debug).at_least(:once) do |&block|
msg = block ? block.call : nil
msg == "N1QL query: select raw meta().id from `#{CouchbaseOrm::Connection.bucket.name}` where type=$1 order by name DESC params: [\"n1_ql_test\"] return 0 rows with scan_consistency: #{described_class::DEFAULT_SCAN_CONSISTENCY} adhoc: true"
end
end

it "should log the set scan_consistency when n1ql query is executed with a specific scan_consistency" do
allow(CouchbaseOrm.logger).to receive(:debug)
default_n1ql_config = CouchbaseOrm::N1ql.config
CouchbaseOrm::N1ql.config({ scan_consistency: :not_bounded })
CouchbaseOrm::N1ql.config({ scan_consistency: :not_bounded, adhoc: true })
N1QLTest.by_rating_reverse()
expect(CouchbaseOrm.logger).to have_received(:debug).at_least(:once).with("N1QL query: select raw meta().id from `#{CouchbaseOrm::Connection.bucket.name}` where type=\"n1_ql_test\" order by name DESC return 0 rows with scan_consistency : not_bounded")
expect(CouchbaseOrm.logger).to have_received(:debug).at_least(:once) do |&block|
msg = block ? block.call : nil
msg == "N1QL query: select raw meta().id from `#{CouchbaseOrm::Connection.bucket.name}` where type=$1 order by name DESC params: [\"n1_ql_test\"] return 0 rows with scan_consistency: not_bounded adhoc: true"
end

CouchbaseOrm::N1ql.config(default_n1ql_config)
N1QLTest.by_rating_reverse()
expect(CouchbaseOrm.logger).to have_received(:debug).at_least(:once).with("N1QL query: select raw meta().id from `#{CouchbaseOrm::Connection.bucket.name}` where type=\"n1_ql_test\" order by name DESC return 0 rows with scan_consistency : #{described_class::DEFAULT_SCAN_CONSISTENCY}")
expect(CouchbaseOrm.logger).to have_received(:debug).at_least(:once) do |&block|
msg = block ? block.call : nil
msg == "N1QL query: select raw meta().id from `#{CouchbaseOrm::Connection.bucket.name}` where type=$1 order by name DESC params: [\"n1_ql_test\"] return 0 rows with scan_consistency: #{described_class::DEFAULT_SCAN_CONSISTENCY} adhoc: true"
end
end

after(:all) do
Expand Down
Loading
Loading