[ARVADOS] updated: 725b9459d878b11d8d45fa12c99a06b400171574

[git at public.curoverse.com]

Summary of changes:
 .../api/app/controllers/application_controller.rb  |   15 ++++++--
 .../api/test/integration/collections_api_test.rb   |   36 ++++++++++++++++++++
 2 files changed, 48 insertions(+), 3 deletions(-)

       via  725b9459d878b11d8d45fa12c99a06b400171574 (commit)
      from  22db59393203f550e0c77e65c650e664667492ab (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 725b9459d878b11d8d45fa12c99a06b400171574
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed Apr 23 21:02:07 2014 -0400

    Check sanity when applying filters, and provide useful error messages.

diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 4b13fca..fcc7618 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -144,9 +144,14 @@ class ApplicationController < ActionController::Base
     if @filters.is_a? Array and @filters.any?
       cond_out = []
       param_out = []
-      @filters.each do |attr, operator, operand|
-        if !model_class.searchable_columns(operator).index attr.to_s
-          raise ArgumentError.new("Invalid attribute '#{attr}' in condition")
+      @filters.each do |filter|
+        attr, operator, operand = filter
+        if !filter.is_a? Array
+          raise ArgumentError.new("Invalid element in filters array: #{filter.inspect} is not an array")
+        elsif !operator.is_a? String
+          raise ArgumentError.new("Invalid operator '#{operator}' (#{operator.class}) in filter")
+        elsif !model_class.searchable_columns(operator).index attr.to_s
+          raise ArgumentError.new("Invalid attribute '#{attr}' in filter")
         end
         case operator.downcase
         when '=', '<', '<=', '>', '>=', 'like'
@@ -159,11 +164,15 @@ class ApplicationController < ActionController::Base
               operand = Time.parse operand
             end
             param_out << operand
+          else
+            raise ArgumentError.new("Invalid operand type '#{operand.class}' for '#{operator}' operator in filter")
           end
         when 'in'
           if operand.is_a? Array
             cond_out << "#{table_name}.#{attr} IN (?)"
             param_out << operand
+          else
+            raise ArgumentError.new("Invalid argument '#{operand}' for 'in' operator in filter")
           end
         when 'is_a'
           operand = [operand] unless operand.is_a? Array
diff --git a/services/api/test/integration/collections_api_test.rb b/services/api/test/integration/collections_api_test.rb
index 5fa77c3..e13d336 100644
--- a/services/api/test/integration/collections_api_test.rb
+++ b/services/api/test/integration/collections_api_test.rb
@@ -15,6 +15,42 @@ class CollectionsApiTest < ActionDispatch::IntegrationTest
     assert_equal "arvados#collectionList", jresponse['kind']
   end
 
+  test "get index with invalid filters (array of strings) responds 422" do
+    get "/arvados/v1/collections", {
+      :format => :json,
+      :filters => ['uuid', '=', 'ad02e37b6a7f45bbe2ead3c29a109b8a+54'].to_json
+    }, auth(:active)
+    assert_response 422
+    assert_match /nvalid element.*not an array/, jresponse['errors'].join(' ')
+  end
+
+  test "get index with invalid filters (unsearchable column) responds 422" do
+    get "/arvados/v1/collections", {
+      :format => :json,
+      :filters => [['this_column_does_not_exist', '=', 'bogus']].to_json
+    }, auth(:active)
+    assert_response 422
+    assert_match /nvalid attribute/, jresponse['errors'].join(' ')
+  end
+
+  test "get index with invalid filters (invalid operator) responds 422" do
+    get "/arvados/v1/collections", {
+      :format => :json,
+      :filters => [['uuid', ':-(', 'displeased']].to_json
+    }, auth(:active)
+    assert_response 422
+    assert_match /nvalid operator/, jresponse['errors'].join(' ')
+  end
+
+  test "get index with invalid filters (invalid operand type) responds 422" do
+    get "/arvados/v1/collections", {
+      :format => :json,
+      :filters => [['uuid', '=', {foo: 'bar'}]].to_json
+    }, auth(:active)
+    assert_response 422
+    assert_match /nvalid operand type/, jresponse['errors'].join(' ')
+  end
+
   test "get index with where= (empty string)" do
     get "/arvados/v1/collections", {:format => :json, :where => ''}, auth(:active)
     assert_response :success

-----------------------------------------------------------------------


hooks/post-receive
--