[ARVADOS] created: 1.3.0-859-gf337797b4
Git user
git at public.curoverse.com
Fri May 17 00:24:14 UTC 2019
at f337797b4543359a990359c178fea3c138feca14 (commit)
commit f337797b4543359a990359c178fea3c138feca14
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date: Thu May 16 21:20:33 2019 -0300
15227: Fixes type checking on json attributes.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 77e3c75af..89eabdcee 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -468,11 +468,21 @@ class ApplicationController < ActionController::Base
end
def load_json_value(hash, key, must_be_class=nil)
- if hash[key].is_a? String
- 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
+ return if hash[key].nil?
+
+ val = hash[key]
+ if val.is_a? ActionController::Parameters
+ val = val.to_unsafe_hash
+ elsif val.is_a? String
+ val = SafeJSON.load(val)
+ hash[key] = val
+ end
+ # When assigning a Hash to an ActionController::Parameters and then
+ # retrieve it, we get another ActionController::Parameters instead of
+ # a Hash. This doesn't happen with other types. This is why 'val' is
+ # being used to do type checking below.
+ if must_be_class and !val.is_a? must_be_class
+ raise TypeError.new("parameter #{key.to_s} must be a #{must_be_class.to_s}")
end
end
@@ -482,7 +492,7 @@ class ApplicationController < ActionController::Base
accept_attribute_as_json :properties, Hash
accept_attribute_as_json :info, Hash
def accept_attribute_as_json(attr, must_be_class)
- if params[resource_name] and resource_attrs.is_a? Hash
+ if params[resource_name] and [Hash, ActionController::Parameters].include?(resource_attrs.class)
if resource_attrs[attr].is_a? Hash
# Convert symbol keys to strings (in hashes provided by
# resource_attrs)
commit 921783ecec893414d328becf3d3da2c34fd5e2f0
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date: Thu May 16 21:19:57 2019 -0300
15227: More tests to expose the bug.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>
diff --git a/services/api/test/functional/arvados/v1/collections_controller_test.rb b/services/api/test/functional/arvados/v1/collections_controller_test.rb
index 244fa0ce5..089895864 100644
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -924,19 +924,25 @@ EOS
assert_equal 'value1', json_response['properties']['property1']
end
- test "create collection with properties" do
- authorize_with :active
- manifest_text = ". d41d8cd98f00b204e9800998ecf8427e 0:0:foo.txt\n"
- post :create, params: {
- collection: {
- manifest_text: manifest_text,
- portable_data_hash: "d30fe8ae534397864cb96c544f4cf102+47",
- properties: {'property_1' => 'value_1'}
+ [
+ {'property_1' => 'value_1'},
+ "{\"property_1\":\"value_1\"}",
+ ].each do |p|
+ test "create collection with valid properties param #{p.inspect}" do
+ authorize_with :active
+ manifest_text = ". d41d8cd98f00b204e9800998ecf8427e 0:0:foo.txt\n"
+ post :create, params: {
+ collection: {
+ manifest_text: manifest_text,
+ portable_data_hash: "d30fe8ae534397864cb96c544f4cf102+47",
+ properties: p
+ }
}
- }
- assert_response :success
- assert_not_nil json_response['uuid']
- assert_equal 'value_1', json_response['properties']['property_1']
+ assert_response :success
+ assert_not_nil json_response['uuid']
+ assert_equal Hash, json_response['properties'].class, 'Collection properties attribute should be of type hash'
+ assert_equal 'value_1', json_response['properties']['property_1']
+ end
end
[
@@ -944,7 +950,7 @@ EOS
[],
42,
'some string',
- '["json", "encoded", "list"]',
+ '["json", "encoded", "array"]',
].each do |p|
test "create collection with non-valid properties param #{p.inspect}" do
authorize_with :active
diff --git a/services/api/test/integration/collections_api_test.rb b/services/api/test/integration/collections_api_test.rb
index ab1a3e69d..eb44b9b34 100644
--- a/services/api/test/integration/collections_api_test.rb
+++ b/services/api/test/integration/collections_api_test.rb
@@ -358,7 +358,7 @@ class CollectionsApiTest < ActionDispatch::IntegrationTest
assert_not_nil json_response['properties']
assert_empty json_response['properties']
- # update collection's description
+ # update collection's properties
put "/arvados/v1/collections/#{json_response['uuid']}",
params: {
format: :json,
@@ -366,6 +366,35 @@ class CollectionsApiTest < ActionDispatch::IntegrationTest
},
headers: auth(:active)
assert_response :success
+ assert_equal Hash, json_response['properties'].class, 'Collection properties attribute should be of type hash'
+ assert_equal 'value_1', json_response['properties']['property_1']
+ end
+
+ test "create collection and update it with json encoded hash properties" do
+ # create collection to be searched for
+ signed_manifest = Collection.sign_manifest(". bad42fa702ae3ea7d888fef11b46f450+44 0:44:my_test_file.txt\n", api_token(:active))
+ post "/arvados/v1/collections",
+ params: {
+ format: :json,
+ collection: {manifest_text: signed_manifest}.to_json,
+ },
+ headers: auth(:active)
+ assert_response 200
+ assert_not_nil json_response['uuid']
+ assert_not_nil json_response['properties']
+ assert_empty json_response['properties']
+
+ # update collection's properties
+ put "/arvados/v1/collections/#{json_response['uuid']}",
+ params: {
+ format: :json,
+ collection: {
+ properties: "{\"property_1\":\"value_1\"}"
+ }
+ },
+ headers: auth(:active)
+ assert_response :success
+ assert_equal Hash, json_response['properties'].class, 'Collection properties attribute should be of type hash'
assert_equal 'value_1', json_response['properties']['property_1']
end
end
commit 509ce6ab05d6453efed3426bf9a82ae6e8fa6d06
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date: Wed May 15 15:50:24 2019 -0300
15227: Adds test exposing bug.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>
diff --git a/services/api/test/functional/arvados/v1/collections_controller_test.rb b/services/api/test/functional/arvados/v1/collections_controller_test.rb
index cc545b2fd..244fa0ce5 100644
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -940,6 +940,28 @@ EOS
end
[
+ false,
+ [],
+ 42,
+ 'some string',
+ '["json", "encoded", "list"]',
+ ].each do |p|
+ test "create collection with non-valid properties param #{p.inspect}" do
+ authorize_with :active
+ post :create, params: {
+ collection: {
+ name: "test collection with non-valid properties param '#{p.inspect}'",
+ manifest_text: '',
+ properties: p
+ }
+ }
+ assert_response 422
+ response_errors = json_response['errors']
+ assert_not_nil response_errors, 'Expected error in response'
+ end
+ end
+
+ [
[". d41d8cd98f00b204e9800998ecf8427e 0:34:foo.txt\n", 1, 34],
[". d41d8cd98f00b204e9800998ecf8427e 0:34:foo.txt 0:30:foo.txt 0:30:foo1.txt 0:30:foo2.txt 0:30:foo3.txt 0:30:foo4.txt\n", 5, 184],
[". d41d8cd98f00b204e9800998ecf8427e 0:0:.\n", 0, 0]
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list