Merge pull request QueueClassic#30 from jasoncodes/sql_safety

Use parameterised queries to prevent SQL escaping issues.

lib/queue_classic/database.rb

@@ -54,12 +54,14 @@ def wait_for_notify(t)
       log("done waiting for notify")
     end
 
-    def execute(sql)
-      log("executing=#{sql}")
+    def execute(sql, *params)
+      log("executing #{sql.inspect}, #{params.inspect}")
       begin
-        connection.exec(sql)
+        params = nil if params.empty?
+        connection.exec(sql, params)
       rescue PGError => e
         log("execute exception=#{e.inspect}")
+        raise
       end
     end
 

lib/queue_classic/durable_array.rb

@@ -8,7 +8,7 @@ def initialize(database)
     end
 
     def <<(details)
-      execute("INSERT INTO #{@table_name} (details) VALUES ('#{JSON.dump(details)}')")
+      execute("INSERT INTO #{@table_name} (details) VALUES ($1);", JSON.dump(details))
       @database.notify if ENV["QC_LISTENING_WORKER"] == "true"
     end
 
@@ -17,24 +17,20 @@ def count
     end
 
     def delete(job)
-      execute("DELETE FROM #{@table_name} WHERE id = #{job.id}")
+      execute("DELETE FROM #{@table_name} WHERE id = $1;", job.id)
       job
     end
 
-    def find(job)
-      find_one {"SELECT * FROM #{@table_name} WHERE id = #{job.id}"}
-    end
-
     def search_details_column(q)
-      find_many { "SELECT * FROM #{@table_name} WHERE details LIKE '%#{q}%'" }
+      find_many { ["SELECT * FROM #{@table_name} WHERE details LIKE $1;", "%#{q}%"] }
     end
 
     def first
-      find_one { "SELECT * FROM lock_head('#{@table_name}', #{@top_boundary})" }
+      find_one { ["SELECT * FROM lock_head($1, $2);", @table_name, @top_boundary] }
     end
 
     def each
-      execute("SELECT * FROM #{@table_name} ORDER BY id ASC").each do |r|
+      execute("SELECT * FROM #{@table_name} ORDER BY id ASC;").each do |r|
         yield Job.new(r)
       end
     end
@@ -44,11 +40,11 @@ def find_one(&blk)
     end
 
     def find_many
-      execute(yield).map {|r| Job.new(r)}
+      execute(*yield).map { |r| Job.new(r) }
     end
 
-    def execute(sql)
-      @database.execute(sql)
+    def execute(sql, *params)
+      @database.execute(sql, *params)
     end
 
   end

lib/queue_classic/queue.rb

@@ -39,16 +39,11 @@ class Queue
     extend AbstractQueue
 
     def self.array
-      if defined? @@array
-        @@array
-      else
-        @@database = Database.new
-        @@array = DurableArray.new(@@database)
-      end
+      @@array ||= DurableArray.new(database)
     end
 
     def self.database
-      @@database
+      @@database ||= Database.new
     end
 
     def initialize(queue_name)

test/database_test.rb

@@ -25,4 +25,20 @@
     assert   @database.connection.notifies.nil?
   end
 
+  test "execute should return rows" do
+    result = @database.execute 'SELECT 11 foo, 22 bar;'
+    assert_equal [{'foo'=>'11', 'bar'=>'22'}], result.to_a
+  end
+
+  test "should raise error on failure" do
+    assert_raises PGError do
+      @database.execute 'SELECT unknown FROM missing;'
+    end
+  end
+
+  test "execute should accept parameters" do
+    result = @database.execute 'SELECT $2::int b, $1::int a, $1::int + $2::int c;', 123, '456'
+    assert_equal [{"a"=>"123", "b"=>"456", "c"=>"579"}], result.to_a
+  end
+
 end

test/durable_array_test.rb

@@ -29,6 +29,12 @@
     assert_equal job, @array.first.details
   end
 
+  test "passes through strings with quotes" do
+    job = {"foo'bar\"baz" => 'abc\\def'}
+    @array << job
+    assert_equal job, @array.first.details
+  end
+
   test "first returns first job when many are in the array" do
     @array << {"job" => "one"}
     @array << {"job" => "two"}
@@ -68,13 +74,13 @@
     assert_equal([{"job" => "one"},{"job" => "two"}], results)
   end
 
-  test "seach" do
+  test "search" do
     @array << {"job" => "A.signature"}
     jobs = @array.search_details_column("A.signature")
     assert_equal "A.signature", jobs.first.signature
   end
 
-  test "seach when data will not match" do
+  test "search when data will not match" do
     @array << {"job" => "A.signature"}
     jobs = @array.search_details_column("B.signature")
     assert_equal [], jobs