[ARVADOS] created: c17687082226d198aa2479522f22d6879166cd1d

Git user git at public.curoverse.com
Fri Apr 29 20:34:59 EDT 2016


        at  c17687082226d198aa2479522f22d6879166cd1d (commit)


commit c17687082226d198aa2479522f22d6879166cd1d
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date:   Fri Apr 29 17:10:41 2016 -0400

    9119: Use Oj strict mode for encoding and decoding JSON.  Fixup other code to
    ensure that only valid types are passed to Oj.dump.

diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index e91e3ce..f562f51 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -182,11 +182,34 @@ class ApplicationController < ActionController::Base
     send_json err, status: status
   end
 
+  def self.fixup_json response
+    # Clean up response object so that it can be passed to Oj.dump in strict
+    # mode.
+    case response
+    when Hash
+      realresponse = {}
+      response.each do |k, v|
+        realresponse[k.to_s] = ApplicationController.fixup_json v
+      end
+      realresponse
+    when Array
+      response.map { |v| ApplicationController.fixup_json v }
+    when Time, ActiveSupport::TimeWithZone
+      response.strftime "%Y-%m-%dT%H:%M:%S.%NZ"
+    when Symbol
+      response.to_s
+    else
+      response
+    end
+  end
+
   def send_json response, opts={}
+    response = ApplicationController.fixup_json response
+
     # The obvious render(json: ...) forces a slow JSON encoder. See
     # #3021 and commit logs. Might be fixed in Rails 4.1.
     render({
-             text: Oj.dump(response, mode: :compat).html_safe,
+             text: Oj.dump(response, mode: :strict, escape_mode: :xss_safe),
              content_type: 'application/json'
            }.merge opts)
   end
@@ -327,7 +350,7 @@ class ApplicationController < ActionController::Base
     return @attrs if @attrs
     @attrs = params[resource_name]
     if @attrs.is_a? String
-      @attrs = Oj.load @attrs, symbol_keys: true
+      @attrs = Oj.strict_load @attrs, symbol_keys: true
     end
     unless @attrs.is_a? Hash
       message = "No #{resource_name}"
@@ -441,7 +464,7 @@ class ApplicationController < ActionController::Base
 
   def load_json_value(hash, key, must_be_class=nil)
     if hash[key].is_a? String
-      hash[key] = Oj.load(hash[key], symbol_keys: false)
+      hash[key] = Oj.strict_load(hash[key], symbol_keys: false)
       if must_be_class and !hash[key].is_a? must_be_class
         raise TypeError.new("parameter #{key.to_s} must be a #{must_be_class.to_s}")
       end
diff --git a/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb b/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb
index 83968be..5229d80 100644
--- a/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb
+++ b/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb
@@ -15,7 +15,7 @@ class Arvados::V1::ApiClientAuthorizationsController < ApplicationController
       new(user_id: system_user.id,
           api_client_id: params[:api_client_id] || current_api_client.andand.id,
           created_by_ip_address: remote_ip,
-          scopes: Oj.load(params[:scopes] || '["all"]'))
+          scopes: Oj.strict_load(params[:scopes] || '["all"]'))
     @object.save!
     show
   end
diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index 44733cd..2643d22 100644
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -77,7 +77,7 @@ class Arvados::V1::CollectionsController < ApplicationController
       collections = Collection.readable_by(*@read_users).where(portable_data_hash: loc.to_s)
       c = collections.limit(2).all
       if c.size == 1
-        visited[loc.to_s] = c[0]
+        visited[loc.to_s] = c[0].as_api_response
       elsif c.size > 1
         name = collections.limit(1).where("name <> ''").first
         if name
diff --git a/services/api/lib/eventbus.rb b/services/api/lib/eventbus.rb
index ac53876..9bf95f5 100644
--- a/services/api/lib/eventbus.rb
+++ b/services/api/lib/eventbus.rb
@@ -162,7 +162,7 @@ class EventBus
     begin
       begin
         # Parse event data as JSON
-        p = (Oj.load event.data).symbolize_keys
+        p = (Oj.strict_load event.data).symbolize_keys
         filter = Filter.new(p)
       rescue Oj::Error => e
         ws.send ({status: 400, message: "malformed request"}.to_json)
diff --git a/services/api/lib/load_param.rb b/services/api/lib/load_param.rb
index d7b9bb7..5b22274 100644
--- a/services/api/lib/load_param.rb
+++ b/services/api/lib/load_param.rb
@@ -17,7 +17,7 @@ module LoadParam
       @where = params[:where]
     elsif params[:where].is_a? String
       begin
-        @where = Oj.load(params[:where])
+        @where = Oj.strict_load(params[:where])
         raise unless @where.is_a? Hash
       rescue
         raise ArgumentError.new("Could not parse \"where\" param as an object")
@@ -33,7 +33,7 @@ module LoadParam
       @filters += params[:filters]
     elsif params[:filters].is_a? String and !params[:filters].empty?
       begin
-        f = Oj.load params[:filters]
+        f = Oj.strict_load params[:filters]
         if not f.nil?
           raise unless f.is_a? Array
           @filters += f
@@ -72,7 +72,7 @@ module LoadParam
       (case params[:order]
        when String
          if params[:order].starts_with? '['
-           od = Oj.load(params[:order])
+           od = Oj.strict_load(params[:order])
            raise unless od.is_a? Array
            od
          else
@@ -142,7 +142,7 @@ module LoadParam
       @select = params[:select]
     when String
       begin
-        @select = Oj.load params[:select]
+        @select = Oj.strict_load params[:select]
         raise unless @select.is_a? Array or @select.nil?
       rescue
         raise ArgumentError.new("Could not parse \"select\" param as an array")
diff --git a/services/api/lib/record_filters.rb b/services/api/lib/record_filters.rb
index 350c380..caf62c7 100644
--- a/services/api/lib/record_filters.rb
+++ b/services/api/lib/record_filters.rb
@@ -126,6 +126,8 @@ module RecordFilters
             end
           end
           cond_out << cond.join(' OR ')
+        else
+          raise ArgumentError.new("Invalid operator '#{operator}'")
         end
       end
       conds_out << cond_out.join(' OR ') if cond_out.any?
diff --git a/services/api/test/integration/collections_performance_test.rb b/services/api/test/integration/collections_performance_test.rb
index 892060a..877d112 100644
--- a/services/api/test/integration/collections_performance_test.rb
+++ b/services/api/test/integration/collections_performance_test.rb
@@ -14,7 +14,7 @@ class CollectionsApiPerformanceTest < ActionDispatch::IntegrationTest
                     api_token: api_token(:active))
     end
     json = time_block "JSON encode #{bigmanifest.length>>20}MiB manifest" do
-      Oj.dump({manifest_text: bigmanifest})
+      Oj.dump({"manifest_text" => bigmanifest}, mode: :strict)
     end
     time_block 'create' do
       post '/arvados/v1/collections', {collection: json}, auth(:active)
diff --git a/services/api/test/integration/websocket_test.rb b/services/api/test/integration/websocket_test.rb
index c4d6d5e..313a22d 100644
--- a/services/api/test/integration/websocket_test.rb
+++ b/services/api/test/integration/websocket_test.rb
@@ -56,7 +56,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
 
     ws_helper do |ws|
       ws.on :message do |event|
-        d = Oj.load event.data
+        d = Oj.strict_load event.data
         status = d["status"]
         ws.close
       end
@@ -75,7 +75,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.load event.data
+        d = Oj.strict_load event.data
         status = d["status"]
         ws.close
       end
@@ -97,7 +97,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.load event.data
+        d = Oj.strict_load event.data
         case state
         when 1
           assert_equal 200, d["status"]
@@ -134,7 +134,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.load event.data
+        d = Oj.strict_load event.data
         case state
         when 1
           assert_equal 200, d["status"]
@@ -174,7 +174,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.load event.data
+        d = Oj.strict_load event.data
         case state
         when 1
           assert_equal 200, d["status"]
@@ -213,7 +213,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.load event.data
+        d = Oj.strict_load event.data
         case state
         when 1
           assert_equal 200, d["status"]
@@ -257,7 +257,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.load event.data
+        d = Oj.strict_load event.data
         case state
         when 1
           assert_equal 200, d["status"]
@@ -297,7 +297,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.load event.data
+        d = Oj.strict_load event.data
         case state
         when 1
           assert_equal 200, d["status"]
@@ -342,7 +342,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.load event.data
+        d = Oj.strict_load event.data
         case state
         when 1
           assert_equal 200, d["status"]
@@ -390,7 +390,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.load event.data
+        d = Oj.strict_load event.data
         case state
         when 1
           assert_equal 200, d["status"]
@@ -435,7 +435,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.load event.data
+        d = Oj.strict_load event.data
         case state
         when 1
           assert_equal 200, d["status"]
@@ -507,7 +507,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.load event.data
+        d = Oj.strict_load event.data
         case state
         when 1
           assert_equal 200, d["status"]
@@ -531,7 +531,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.load event.data
+        d = Oj.strict_load event.data
         status = d["status"]
         ws.close
       end
@@ -549,7 +549,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.load event.data
+        d = Oj.strict_load event.data
         status = d["status"]
         ws.close
       end
@@ -567,7 +567,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.load event.data
+        d = Oj.strict_load event.data
         status = d["status"]
         ws.close
       end
@@ -590,7 +590,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.load event.data
+        d = Oj.strict_load event.data
         case state
         when (1..EventBus::MAX_FILTERS)
           assert_equal 200, d["status"]
@@ -625,7 +625,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.load event.data
+        d = Oj.strict_load event.data
         case state
         when 1
           assert_equal 200, d["status"]
@@ -664,7 +664,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.load event.data
+        d = Oj.strict_load event.data
         case state
         when 1
           assert_equal 200, d["status"]
diff --git a/services/api/test/test_helper.rb b/services/api/test/test_helper.rb
index 7579abf..08828bc 100644
--- a/services/api/test/test_helper.rb
+++ b/services/api/test/test_helper.rb
@@ -26,7 +26,7 @@ require 'mocha/mini_test'
 
 module ArvadosTestSupport
   def json_response
-    Oj.load response.body
+    Oj.strict_load response.body
   end
 
   def api_token(api_client_auth_name)

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list