X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/e3e54264e8bc767e1ec773cff4e5bdf4c4934a36..72aa70eec7693bfb5d46a4bdac3619b3c6b1f79c:/services/api/lib/load_param.rb

diff --git a/services/api/lib/load_param.rb b/services/api/lib/load_param.rb
index fb580f237c..3f1a3b223a 100644
--- a/services/api/lib/load_param.rb
+++ b/services/api/lib/load_param.rb
@@ -6,9 +6,12 @@
 #   @where, @filters, @limit, @offset, @orders
 module LoadParam
 
-  # Default limit on number of rows to return in a single query.
+  # Default number of rows to return in a single query.
   DEFAULT_LIMIT = 100
 
+  # Maximum number of rows to return in a single query, even if the client asks for more.
+  MAX_LIMIT = 1000
+
   # Load params[:where] into @where
   def load_where_param
     if params[:where].nil? or params[:where] == ""
@@ -55,7 +58,7 @@ module LoadParam
       unless params[:limit].to_s.match(/^\d+$/)
         raise ArgumentError.new("Invalid value for limit parameter")
       end
-      @limit = params[:limit].to_i
+      @limit = [params[:limit].to_i, MAX_LIMIT].min
     else
       @limit = DEFAULT_LIMIT
     end
@@ -89,11 +92,22 @@ 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]+)$/
+        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
@@ -128,5 +142,4 @@ module LoadParam
     @distinct = false if (params[:distinct] == false || params[:distinct] == "false")
   end
 
-
 end