[ARVADOS] created: 6c92a0d8acd0a5525bbe566a3e202b93d498b370
git at public.curoverse.com
git at public.curoverse.com
Wed May 28 12:37:55 EDT 2014
at 6c92a0d8acd0a5525bbe566a3e202b93d498b370 (commit)
commit 6c92a0d8acd0a5525bbe566a3e202b93d498b370
Author: Tom Clegg <tom at curoverse.com>
Date: Wed May 28 12:34:59 2014 -0400
2893: Add test for un-symbolizing existing db content.
diff --git a/services/api/test/fixtures/links.yml b/services/api/test/fixtures/links.yml
index a7821aa..b58f7c6 100644
--- a/services/api/test/fixtures/links.yml
+++ b/services/api/test/fixtures/links.yml
@@ -403,3 +403,26 @@ multilevel_collection_1_readable_by_active:
name: can_read
head_uuid: 1fd08fc162a5c6413070a8bd0bffc818+150
properties: {}
+
+has_symbol_keys_in_database_somehow:
+ uuid: zzzzz-o0j2j-enl1wg58310loc6
+ owner_uuid: zzzzz-tpzed-000000000000000
+ created_at: 2014-05-28 16:24:02.314722162 Z
+ modified_by_client_uuid:
+ modified_by_user_uuid: zzzzz-tpzed-000000000000000
+ modified_at: 2014-05-28 16:24:02.314484982 Z
+ tail_uuid: ~
+ link_class: test
+ name: ~
+ head_uuid: ~
+ properties:
+ :foo: "bar"
+ baz:
+ - waz
+ - :waz
+ - :waz
+ - 1
+ - ~
+ - false
+ - true
+ updated_at: 2014-05-28 16:24:02.314296411 Z
diff --git a/services/api/test/unit/arvados_model_test.rb b/services/api/test/unit/arvados_model_test.rb
index 5fc307e..a0c1327 100644
--- a/services/api/test/unit/arvados_model_test.rb
+++ b/services/api/test/unit/arvados_model_test.rb
@@ -49,4 +49,13 @@ class ArvadosModelTest < ActiveSupport::TestCase
end
end
end
+
+ test "Stringify symbols coming from serialized attribute in database" do
+ fixed = Link.find_by_uuid(links(:has_symbol_keys_in_database_somehow).uuid)
+ assert_equal(["baz", "foo"], fixed.properties.keys.sort,
+ "Hash symbol keys from DB did not get stringified.")
+ assert_equal(['waz', 'waz', 'waz', 1, nil, false, true],
+ fixed.properties['baz'],
+ "Array symbol values from DB did not get stringified.")
+ end
end
commit 4ac438924f1e365cf5231140c9640c004ad4f1ff
Author: Tom Clegg <tom at curoverse.com>
Date: Wed May 28 12:22:18 2014 -0400
2893: Prevent symbol keys/values in serialized attributes.
diff --git a/services/api/app/controllers/arvados/v1/nodes_controller.rb b/services/api/app/controllers/arvados/v1/nodes_controller.rb
index 990397b..3fbf5fc 100644
--- a/services/api/app/controllers/arvados/v1/nodes_controller.rb
+++ b/services/api/app/controllers/arvados/v1/nodes_controller.rb
@@ -23,7 +23,7 @@ class Arvados::V1::NodesController < ApplicationController
@object.ping({ ip: params[:local_ipv4] || request.env['REMOTE_ADDR'],
ping_secret: params[:ping_secret],
ec2_instance_id: params[:instance_id] })
- if @object.info[:ping_secret] == params[:ping_secret]
+ if @object.info['ping_secret'] == params[:ping_secret]
render json: @object.as_api_response(:superuser)
else
raise "Invalid ping_secret after ping"
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index bfd228e..2817d69 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -20,6 +20,7 @@ class ArvadosModel < ActiveRecord::Base
after_create :log_create
after_update :log_update
after_destroy :log_destroy
+ after_find :convert_serialized_symbols_to_strings
validate :ensure_serialized_attribute_type
validate :normalize_collection_uuids
validate :ensure_valid_uuids
@@ -261,6 +262,37 @@ class ArvadosModel < ActiveRecord::Base
true
end
+ def self.has_any_symbols? x
+ if x.is_a? Hash
+ x.each do |k,v|
+ return true if has_any_symbols?(k) or has_any_symbols?(v)
+ end
+ elsif x.is_a? Array
+ x.each do |k|
+ return true if has_any_symbols?(k)
+ end
+ else
+ return (x.class == Symbol)
+ end
+ false
+ end
+
+ def self.recursive_stringify x
+ if x.is_a? Hash
+ Hash[x.collect do |k,v|
+ [recursive_stringify(k), recursive_stringify(v)]
+ end]
+ elsif x.is_a? Array
+ x.collect do |k|
+ recursive_stringify k
+ end
+ elsif x.is_a? Symbol
+ x.to_s
+ else
+ x
+ end
+ 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
@@ -270,8 +302,22 @@ class ArvadosModel < ActiveRecord::Base
# developer.
self.class.serialized_attributes.each do |colname, attr|
if attr.object_class
- unless self.attributes[colname].is_a? attr.object_class
- self.errors.add colname.to_sym, "must be a #{attr.object_class.to_s}"
+ if self.attributes[colname].class != attr.object_class
+ self.errors.add colname.to_sym, "must be a #{attr.object_class.to_s}, not a #{self.attributes[colname].class.to_s}"
+ elsif self.class.has_any_symbols? attributes[colname]
+ self.errors.add colname.to_sym, "must not contain symbols: #{attributes[colname].inspect}"
+ end
+ end
+ end
+ end
+
+ def convert_serialized_symbols_to_strings
+ self.class.serialized_attributes.each do |colname, attr|
+ if attr.object_class == Hash
+ if self.class.has_any_symbols? attributes[colname]
+ attributes[colname] = self.class.recursive_stringify attributes[colname]
+ self.send(colname + '=',
+ self.class.recursive_stringify(attributes[colname]))
end
end
end
diff --git a/services/api/app/models/node.rb b/services/api/app/models/node.rb
index 512f0e0..2ca05f6 100644
--- a/services/api/app/models/node.rb
+++ b/services/api/app/models/node.rb
@@ -37,7 +37,7 @@ class Node < ArvadosModel
end
def crunch_worker_state
- case self.info.andand[:slurm_state]
+ case self.info.andand['slurm_state']
when 'alloc', 'comp'
'busy'
when 'idle'
@@ -64,8 +64,8 @@ class Node < ArvadosModel
def ping(o)
raise "must have :ip and :ping_secret" unless o[:ip] and o[:ping_secret]
- if o[:ping_secret] != self.info[:ping_secret]
- logger.info "Ping: secret mismatch: received \"#{o[:ping_secret]}\" != \"#{self.info[:ping_secret]}\""
+ if o[:ping_secret] != self.info['ping_secret']
+ logger.info "Ping: secret mismatch: received \"#{o[:ping_secret]}\" != \"#{self.info['ping_secret']}\""
raise ArvadosModel::UnauthorizedError.new("Incorrect ping_secret")
end
self.last_ping_at = Time.now
@@ -81,16 +81,16 @@ class Node < ArvadosModel
# Record instance ID if not already known
if o[:ec2_instance_id]
- if !self.info[:ec2_instance_id]
- self.info[:ec2_instance_id] = o[:ec2_instance_id]
+ if !self.info['ec2_instance_id']
+ self.info['ec2_instance_id'] = o[:ec2_instance_id]
if (Rails.configuration.compute_node_ec2_tag_enable rescue true)
tag_cmd = ("ec2-create-tags #{o[:ec2_instance_id]} " +
"--tag 'Name=#{self.uuid}'")
`#{tag_cmd}`
end
- elsif self.info[:ec2_instance_id] != o[:ec2_instance_id]
+ elsif self.info['ec2_instance_id'] != o[:ec2_instance_id]
logger.debug "Multiple nodes have credentials for #{self.uuid}"
- raise "#{self.uuid} is already running at #{self.info[:ec2_instance_id]} so rejecting ping from #{o[:ec2_instance_id]}"
+ raise "#{self.uuid} is already running at #{self.info['ec2_instance_id']} so rejecting ping from #{o[:ec2_instance_id]}"
end
end
@@ -108,9 +108,9 @@ class Node < ArvadosModel
raise "No available node slots" if try_slot == MAX_SLOTS
end while true
self.hostname = self.class.hostname_for_slot(self.slot_number)
- if info[:ec2_instance_id]
+ if info['ec2_instance_id']
if (Rails.configuration.compute_node_ec2_tag_enable rescue true)
- `ec2-create-tags #{self.info[:ec2_instance_id]} --tag 'hostname=#{self.hostname}'`
+ `ec2-create-tags #{self.info['ec2_instance_id']} --tag 'hostname=#{self.hostname}'`
end
end
end
@@ -120,7 +120,7 @@ class Node < ArvadosModel
def start!(ping_url_method)
ensure_permission_to_save
- ping_url = ping_url_method.call({ id: self.uuid, ping_secret: self.info[:ping_secret] })
+ ping_url = ping_url_method.call({ id: self.uuid, ping_secret: self.info['ping_secret'] })
if (Rails.configuration.compute_node_ec2run_args and
Rails.configuration.compute_node_ami)
ec2_args = ["--user-data '#{ping_url}'",
@@ -138,23 +138,23 @@ class Node < ArvadosModel
ec2run_cmd = ''
ec2spot_cmd = ''
end
- self.info[:ec2_run_command] = ec2run_cmd
- self.info[:ec2_spot_command] = ec2spot_cmd
- self.info[:ec2_start_command] = ec2spot_cmd
+ self.info['ec2_run_command'] = ec2run_cmd
+ self.info['ec2_spot_command'] = ec2spot_cmd
+ self.info['ec2_start_command'] = ec2spot_cmd
logger.info "#{self.uuid} ec2_start_command= #{ec2spot_cmd.inspect}"
result = `#{ec2spot_cmd} 2>&1`
- self.info[:ec2_start_result] = result
+ self.info['ec2_start_result'] = result
logger.info "#{self.uuid} ec2_start_result= #{result.inspect}"
result.match(/INSTANCE\s*(i-[0-9a-f]+)/) do |m|
instance_id = m[1]
- self.info[:ec2_instance_id] = instance_id
+ self.info['ec2_instance_id'] = instance_id
if (Rails.configuration.compute_node_ec2_tag_enable rescue true)
`ec2-create-tags #{instance_id} --tag 'Name=#{self.uuid}'`
end
end
result.match(/SPOTINSTANCEREQUEST\s*(sir-[0-9a-f]+)/) do |m|
sir_id = m[1]
- self.info[:ec2_sir_id] = sir_id
+ self.info['ec2_sir_id'] = sir_id
if (Rails.configuration.compute_node_ec2_tag_enable rescue true)
`ec2-create-tags #{sir_id} --tag 'Name=#{self.uuid}'`
end
@@ -165,7 +165,7 @@ class Node < ArvadosModel
protected
def ensure_ping_secret
- self.info[:ping_secret] ||= rand(2**256).to_s(36)
+ self.info['ping_secret'] ||= rand(2**256).to_s(36)
end
def dnsmasq_update
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index d6dd9dc..d219915 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -339,20 +339,20 @@ class User < ArvadosModel
perm_exists = false
login_perms.each do |perm|
if perm.properties['username'] == repo_name
- perm_exists = true
+ perm_exists = perm
break
end
end
- if !perm_exists
+ if perm_exists
+ login_perm = perm_exists
+ else
login_perm = Link.create(tail_uuid: self.uuid,
head_uuid: vm[:uuid],
link_class: 'permission',
name: 'can_login',
properties: {'username' => repo_name})
logger.info { "login permission: " + login_perm[:uuid] }
- else
- login_perm = login_perms.first
end
return login_perm
diff --git a/services/api/test/functional/arvados/v1/users_controller_test.rb b/services/api/test/functional/arvados/v1/users_controller_test.rb
index 1fefcb6..f02d62b 100644
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -144,6 +144,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
email: "foo at example.com"
}
}
+ assert_response :success
response_items = JSON.parse(@response.body)['items']
@@ -795,6 +796,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
def find_obj_in_resp (response_items, object_type, head_kind=nil)
return_obj = nil
+ response_items
response_items.each { |x|
if !x
next
diff --git a/services/api/test/unit/arvados_model_test.rb b/services/api/test/unit/arvados_model_test.rb
index e9e872f..5fc307e 100644
--- a/services/api/test/unit/arvados_model_test.rb
+++ b/services/api/test/unit/arvados_model_test.rb
@@ -31,4 +31,22 @@ class ArvadosModelTest < ActiveSupport::TestCase
assert a.uuid.length==27, "Auto assigned uuid length is wrong."
end
+ [ {:a => 'foo'},
+ {'a' => {'foo' => {:bar => 'baz'}}},
+ {'a' => {'foo' => {'bar' => :baz}}},
+ {'a' => {'foo' => ['bar', :baz]}},
+ {'a' => {['foo', :foo] => ['bar', 'baz']}},
+ ].each do |x|
+ test "refuse symbol keys in serialized attribute: #{x.inspect}" do
+ set_user_from_auth :admin_trustedclient
+ assert_nothing_raised do
+ Link.create!(link_class: 'test',
+ properties: {})
+ end
+ assert_raises ActiveRecord::RecordInvalid do
+ Link.create!(link_class: 'test',
+ properties: x)
+ end
+ end
+ end
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list