[ARVADOS] created: d7c84d69bb62d61bc671b2d5e0ad4ed42dbeb7c0

Git user git at public.curoverse.com
Wed Mar 1 16:14:55 EST 2017


        at  d7c84d69bb62d61bc671b2d5e0ad4ed42dbeb7c0 (commit)


commit d7c84d69bb62d61bc671b2d5e0ad4ed42dbeb7c0
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed Mar 1 16:09:05 2017 -0500

    11168: Change db serialize from YAML to JSON.

diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 2285119..2072520 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -1,3 +1,5 @@
+require 'safe_json'
+
 module ApiTemplateOverride
   def allowed_to_render?(fieldset, field, model, options)
     return false if !super
@@ -209,7 +211,7 @@ class ApplicationController < ActionController::Base
     # 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: SafeJSON.dump(response).html_safe,
              content_type: 'application/json'
            }.merge opts)
   end
@@ -467,7 +469,7 @@ class ApplicationController < ActionController::Base
 
   def load_json_value(hash, key, must_be_class=nil)
     if hash[key].is_a? String
-      hash[key] = Oj.strict_load(hash[key], symbol_keys: false)
+      hash[key] = SafeJSON.load(hash[key])
       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 76acc70..334538b 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
@@ -1,3 +1,5 @@
+require 'safe_json'
+
 class Arvados::V1::ApiClientAuthorizationsController < ApplicationController
   accept_attribute_as_json :scopes, Array
   before_filter :current_api_client_is_trusted, :except => [:current]
@@ -16,7 +18,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.strict_load(params[:scopes] || '["all"]'))
+          scopes: SafeJSON.load(params[:scopes] || '["all"]'))
     @object.save!
     show
   end
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index b2e1bea..b39c6ab 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -1,5 +1,6 @@
 require 'has_uuid'
 require 'record_filters'
+require 'serializers'
 
 class ArvadosModel < ActiveRecord::Base
   self.abstract_class = true
@@ -454,6 +455,19 @@ class ArvadosModel < ActiveRecord::Base
     end
   end
 
+  def self.where_serialized(colname, value)
+    where("#{colname.to_s} IN (?)", [value.to_yaml, SafeJSON.dump(value)])
+  end
+
+  Serializer = {
+    Hash => HashSerializer,
+    Array => ArraySerializer,
+  }
+
+  def self.serialize(colname, type)
+    super(colname, Serializer[type])
+  end
+
   def ensure_serialized_attribute_type
     # Specifying a type in the "serialize" declaration causes rails to
     # raise an exception if a different data type is retrieved from
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index ea48ffa..e6ce8f0 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -1,4 +1,5 @@
 require 'whitelist_update'
+require 'safe_json'
 
 class Container < ArvadosModel
   include HasUuid
@@ -83,13 +84,13 @@ class Container < ArvadosModel
 
   def self.find_reusable(attrs)
     candidates = Container.
-      where('command = ?', attrs[:command].to_yaml).
+      where_serialized(:command, attrs[:command]).
       where('cwd = ?', attrs[:cwd]).
-      where('environment = ?', self.deep_sort_hash(attrs[:environment]).to_yaml).
+      where_serialized(:environment, self.deep_sort_hash(attrs[:environment])).
       where('output_path = ?', attrs[:output_path]).
       where('container_image = ?', attrs[:container_image]).
-      where('mounts = ?', self.deep_sort_hash(attrs[:mounts]).to_yaml).
-      where('runtime_constraints = ?', self.deep_sort_hash(attrs[:runtime_constraints]).to_yaml)
+      where_serialized(:mounts, self.deep_sort_hash(attrs[:mounts])).
+      where_serialized(:runtime_constraints, self.deep_sort_hash(attrs[:runtime_constraints]))
 
     # Check for Completed candidates whose output and log are both readable.
     select_readable_pdh = Collection.
diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb
index d078a61..55ae5c7 100644
--- a/services/api/app/models/job.rb
+++ b/services/api/app/models/job.rb
@@ -1,3 +1,5 @@
+require 'safe_json'
+
 class Job < ArvadosModel
   include HasUuid
   include KindAndEtag
@@ -321,7 +323,7 @@ class Job < ArvadosModel
   protected
 
   def self.sorted_hash_digest h
-    Digest::MD5.hexdigest(Oj.dump(deep_sort_hash(h)))
+    Digest::MD5.hexdigest(SafeJSON.dump(deep_sort_hash(h)))
   end
 
   def foreign_key_attributes
diff --git a/services/api/config/initializers/lograge.rb b/services/api/config/initializers/lograge.rb
index 4b1aea9..6d1dd4a 100644
--- a/services/api/config/initializers/lograge.rb
+++ b/services/api/config/initializers/lograge.rb
@@ -1,10 +1,12 @@
+require 'safe_json'
+
 Server::Application.configure do
   config.lograge.enabled = true
   config.lograge.formatter = Lograge::Formatters::Logstash.new
   config.lograge.custom_options = lambda do |event|
     exceptions = %w(controller action format id)
     params = event.payload[:params].except(*exceptions)
-    params_s = Oj.dump(params)
+    params_s = SafeJSON.dump(params)
     if params_s.length > Rails.configuration.max_request_log_params_size
       { params_truncated: params_s[0..Rails.configuration.max_request_log_params_size] + "[...]" }
     else
diff --git a/services/api/lib/create_superuser_token.rb b/services/api/lib/create_superuser_token.rb
index 72b1ae7..84100c2 100755
--- a/services/api/lib/create_superuser_token.rb
+++ b/services/api/lib/create_superuser_token.rb
@@ -29,12 +29,13 @@ module CreateSuperUserToken
         apiClient =  ApiClient.find_or_create_by_url_prefix_and_is_trusted("ssh://root@localhost/", true)
 
         # Check if there is an unexpired superuser token corresponding to this api client
-        api_client_auth = ApiClientAuthorization.where(
-                'user_id = ? AND
-                 api_client_id = ? AND
-                 scopes = ? AND
-                 (expires_at IS NULL OR expires_at > CURRENT_TIMESTAMP)',
-               system_user.id, apiClient.id, ['all'].to_yaml).first
+        api_client_auth =
+          ApiClientAuthorization.
+          where(user_id: system_user.id).
+          where(api_client_id: apiClient.id).
+          where_serialized(:scopes, ['all']).
+          where('(expires_at IS NULL OR expires_at > CURRENT_TIMESTAMP)').
+          first
 
         # none exist; create one with the supplied token
         if !api_client_auth
diff --git a/services/api/lib/eventbus.rb b/services/api/lib/eventbus.rb
index 5e413d5..11b178d 100644
--- a/services/api/lib/eventbus.rb
+++ b/services/api/lib/eventbus.rb
@@ -3,10 +3,11 @@
 Thread.abort_on_exception = true
 
 require 'eventmachine'
-require 'oj'
 require 'faye/websocket'
-require 'record_filters'
 require 'load_param'
+require 'oj'
+require 'record_filters'
+require 'safe_json'
 require 'set'
 require 'thread'
 
@@ -79,7 +80,7 @@ class EventBus
   end
 
   def send_message(ws, obj)
-    ws.send(Oj.dump(obj, mode: :compat))
+    ws.send(SafeJSON.dump(obj))
   end
 
   # Push out any pending events to the connection +ws+
@@ -181,7 +182,7 @@ class EventBus
     begin
       begin
         # Parse event data as JSON
-        p = (Oj.strict_load event.data).symbolize_keys
+        p = SafeJSON.load(event.data).symbolize_keys
         filter = Filter.new(p)
       rescue Oj::Error => e
         send_message(ws, {status: 400, message: "malformed request"})
diff --git a/services/api/lib/load_param.rb b/services/api/lib/load_param.rb
index dee0f23..3e1e8b5 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.strict_load(params[:where])
+        @where = SafeJSON.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.strict_load params[:filters]
+        f = SafeJSON.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.strict_load(params[:order])
+           od = SafeJSON.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.strict_load params[:select]
+        @select = SafeJSON.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/safe_json.rb b/services/api/lib/safe_json.rb
new file mode 100644
index 0000000..dbf53e0
--- /dev/null
+++ b/services/api/lib/safe_json.rb
@@ -0,0 +1,8 @@
+class SafeJSON
+  def self.dump(o)
+    return Oj.dump(o, mode: :compat)
+  end
+  def self.load(s)
+    Oj.strict_load(s, symbol_keys: false)
+  end
+end
diff --git a/services/api/lib/serializers.rb b/services/api/lib/serializers.rb
new file mode 100644
index 0000000..d7a81e1
--- /dev/null
+++ b/services/api/lib/serializers.rb
@@ -0,0 +1,41 @@
+require 'safe_json'
+
+class HashSerializer
+  def self.load(s)
+    if s.nil?
+      {}
+    elsif s[0] == "{"
+      SafeJSON.load(s)
+    elsif s[0..2] == "---"
+      Psych.safe_load(s)
+    else
+      raise "invalid serialized data #{s[0..5].inspect}"
+    end
+  end
+  def self.dump(h)
+    SafeJSON.dump(h)
+  end
+  def self.object_class
+    ::Hash
+  end
+end
+
+class ArraySerializer
+  def self.load(s)
+    if s.nil?
+      []
+    elsif s[0] == "["
+      SafeJSON.load(s)
+    elsif s[0..2] == "---"
+      Psych.safe_load(s)
+    else
+      raise "invalid serialized data #{s[0..5].inspect}"
+    end
+  end
+  def self.dump(a)
+    SafeJSON.dump(a)
+  end
+  def self.object_class
+    ::Array
+  end
+end
diff --git a/services/api/test/integration/collections_performance_test.rb b/services/api/test/integration/collections_performance_test.rb
index f6f39fe..c284750 100644
--- a/services/api/test/integration/collections_performance_test.rb
+++ b/services/api/test/integration/collections_performance_test.rb
@@ -1,3 +1,4 @@
+require 'safe_json'
 require 'test_helper'
 require 'helpers/manifest_examples'
 require 'helpers/time_block'
@@ -14,7 +15,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})
+      SafeJSON.dump({"manifest_text" => bigmanifest})
     end
     time_block 'create' do
       post '/arvados/v1/collections', {collection: json}, auth(:active)
@@ -45,7 +46,7 @@ class CollectionsApiPerformanceTest < ActionDispatch::IntegrationTest
                                  bytes_per_block: 2**26,
                                  api_token: api_token(:active))
     json = time_block "JSON encode #{hugemanifest.length>>20}MiB manifest" do
-      Oj.dump({manifest_text: hugemanifest})
+      SafeJSON.dump({manifest_text: hugemanifest})
     end
     vmpeak "post" 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 a9993b2..841a808 100644
--- a/services/api/test/integration/websocket_test.rb
+++ b/services/api/test/integration/websocket_test.rb
@@ -1,6 +1,7 @@
-require 'test_helper'
-require 'oj'
 require 'database_cleaner'
+require 'oj'
+require 'safe_json'
+require 'test_helper'
 
 DatabaseCleaner.strategy = :deletion
 
@@ -112,7 +113,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
 
     ws_helper do |ws|
       ws.on :message do |event|
-        d = Oj.strict_load event.data
+        d = SafeJSON.load event.data
         status = d["status"]
         ws.close
       end
@@ -130,7 +131,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.strict_load event.data
+        d = SafeJSON.load event.data
         status = d["status"]
         ws.close
       end
@@ -152,7 +153,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.strict_load event.data
+        d = SafeJSON.load event.data
         case state
         when 1
           assert_equal 200, d["status"]
@@ -189,7 +190,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.strict_load event.data
+        d = SafeJSON.load event.data
         case state
         when 1
           assert_equal 200, d["status"]
@@ -229,7 +230,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.strict_load event.data
+        d = SafeJSON.load event.data
         case state
         when 1
           assert_equal 200, d["status"]
@@ -268,7 +269,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.strict_load event.data
+        d = SafeJSON.load event.data
         case state
         when 1
           assert_equal 200, d["status"]
@@ -312,7 +313,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.strict_load event.data
+        d = SafeJSON.load event.data
         case state
         when 1
           assert_equal 200, d["status"]
@@ -350,7 +351,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.strict_load event.data
+        d = SafeJSON.load event.data
         case state
         when 1
           assert_equal 200, d["status"]
@@ -394,7 +395,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.strict_load event.data
+        d = SafeJSON.load event.data
         case state
         when 1
           assert_equal 200, d["status"]
@@ -442,7 +443,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.strict_load event.data
+        d = SafeJSON.load event.data
         case state
         when 1
           assert_equal 200, d["status"]
@@ -487,7 +488,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.strict_load event.data
+        d = SafeJSON.load event.data
         case state
         when 1
           assert_equal 200, d["status"]
@@ -557,7 +558,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.strict_load event.data
+        d = SafeJSON.load event.data
         case state
         when 1
           assert_equal 200, d["status"]
@@ -581,7 +582,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.strict_load event.data
+        d = SafeJSON.load event.data
         status = d["status"]
         ws.close
       end
@@ -599,7 +600,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.strict_load event.data
+        d = SafeJSON.load event.data
         status = d["status"]
         ws.close
       end
@@ -617,7 +618,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.strict_load event.data
+        d = SafeJSON.load event.data
         status = d["status"]
         ws.close
       end
@@ -640,7 +641,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.strict_load event.data
+        d = SafeJSON.load event.data
         case state
         when (1..Rails.configuration.websocket_max_filters)
           assert_equal 200, d["status"]
@@ -675,7 +676,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.strict_load event.data
+        d = SafeJSON.load event.data
         case state
         when 1
           assert_equal 200, d["status"]
@@ -712,7 +713,7 @@ class WebsocketTest < ActionDispatch::IntegrationTest
       end
 
       ws.on :message do |event|
-        d = Oj.strict_load event.data
+        d = SafeJSON.load event.data
         case state
         when 1
           assert_equal 200, d["status"]
diff --git a/services/api/test/unit/create_superuser_token_test.rb b/services/api/test/unit/create_superuser_token_test.rb
index ba81360..bd55b15 100644
--- a/services/api/test/unit/create_superuser_token_test.rb
+++ b/services/api/test/unit/create_superuser_token_test.rb
@@ -89,7 +89,7 @@ class CreateSuperUserTokenTest < ActiveSupport::TestCase
     active_user_token = api_client_authorizations("admin_vm").api_token
     ApiClientAuthorization.
       where(user_id: system_user.id).
-      update_all(scopes: ["GET /"])
+      update_all(scopes: SafeJSON.dump(["GET /"]))
     fixture_tokens = ApiClientAuthorization.all.collect(&:api_token)
     new_token = create_superuser_token
     refute_includes(fixture_tokens, new_token)

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list