Merge branch '4310-lock-before-fail' closes #4310 (again)

[arvados.git] / services / api / lib / load_param.rb

diff --git a/services/api/lib/load_param.rb b/services/api/lib/load_param.rb
index 7acf014ec6aaed42ecf91ac9501a8a852b7ba8bc..8d5a9d21ee20d39bb9673d5e4671dfbdc97e3a2e 100644 (file)
--- a/services/api/lib/load_param.rb
+++ b/services/api/lib/load_param.rb
@@ -34,8 +34,10 @@ module LoadParam
     elsif params[:filters].is_a? String and !params[:filters].empty?
       begin
         f = Oj.load params[:filters]
-        raise unless f.is_a? Array
-        @filters += f
+        if not f.nil?
+          raise unless f.is_a? Array
+          @filters += f
+        end
       rescue
         raise ArgumentError.new("Could not parse \"filters\" param as an array")
       end
@@ -68,7 +70,7 @@ module LoadParam
     end
 
     @orders = []
-    if params[:order]
+    if (params[:order].is_a?(Array) && !params[:order].empty?) || !params[:order].blank?
       od = []
       (case params[:order]
        when String
@@ -87,10 +89,23 @@ module LoadParam
         order = order.to_s
         attr, direction = order.strip.split " "
         direction ||= 'asc'
+        # The attr can have its table unspecified if it happens to be for the current "model_class" (the first case)
+        # or it can be fully specified with the database tablename (the second case) (e.g. "collections.name").
+        # NB that the security check for the second case table_name will not work if the model
+        # has used set_table_name to use an alternate table name from the Rails standard.
+        # I could not find a perfect way to handle this well, but ActiveRecord::Base.send(:descendants)
+        # would be a place to start if this ever becomes necessary.
         if attr.match /^[a-z][_a-z0-9]+$/ and
             model_class.columns.collect(&:name).index(attr) and
             ['asc','desc'].index direction.downcase
           @orders << "#{table_name}.#{attr} #{direction.downcase}"
+        elsif attr.match /^([a-z][_a-z0-9]+)\.([a-z][_a-z0-9]+)$/ and
+            ['asc','desc'].index(direction.downcase) and
+            ActiveRecord::Base.connection.tables.include?($1) and
+            $1.classify.constantize.columns.collect(&:name).index($2)
+          # $1 in the above checks references the first match from the regular expression, which is expected to be the database table name
+          # $2 is of course the actual database column name
+          @orders << "#{attr} #{direction.downcase}"
         end
       end
     end
@@ -105,7 +120,7 @@ module LoadParam
     when String
       begin
         @select = Oj.load params[:select]
-        raise unless @select.is_a? Array
+        raise unless @select.is_a? Array or @select.nil?
       rescue
         raise ArgumentError.new("Could not parse \"select\" param as an array")
       end
@@ -124,5 +139,4 @@ module LoadParam
     @distinct = false if (params[:distinct] == false || params[:distinct] == "false")
   end
 
-
 end