Project

General

Profile

Actions

Bug #2893

closed

Prevent symbols and fancy Hash subclasses from going in or out of the database in serialized attributes

Added by Tom Clegg over 10 years ago. Updated over 10 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Story points:
1.0

Subtasks 1 (0 open1 closed)

Task #2894: Review 2893-no-symbols-in-dbResolvedRadhika Chippada05/29/2014Actions
Actions #1

Updated by Tom Clegg over 10 years ago

  • Assigned To set to Tom Clegg
Actions #2

Updated by Radhika Chippada over 10 years ago

Review comments:

1. How about the method name "has_symbols?" instead of "has_any_symbols?"

2. convert_serialized_symbols_to_strings method

This method checks "if attr.object_class == Hash"
Even though we have used only Hash type objects for serialized attributes, can't I have an array defined as seriazed attribute? For ex: "serialize :my_attr, Array"
Should this method hence also support Array objects as well? What if I said "serialize :my_attr, String" or some other db supported object?
Should this method intead omit this check and invoke "if self.class.has_any_symbols? attributes[colname]" for all cases or all supported cases?

3. Please add the below additional tests to ArvadosModelTest -> test "refuse symbol keys in serialized attribute: #{x.inspect}"

--- a/services/api/test/unit/arvados_model_test.rb
+++ b/services/api/test/unit/arvados_model_test.rb
@ -32,6 +32,12 @ class ArvadosModelTest < ActiveSupport::TestCase

[ {:a => 'foo'},
+ {'a' => :foo},
+ {:a => ['foo', 'bar']},
+ {'a' => [:foo, 'bar']},
+ {:a => [:foo, :bar]},
+ {:a => {'foo' => {'bar' => 'baz'}}},
+ {'a' => {:foo => {'bar' => 'baz'}}}, {'a' => {'foo' => {:bar => 'baz'}}}, {'a' => {'foo' => {'bar' => :baz}}},
Actions #3

Updated by Tom Clegg over 10 years ago

  • Target version changed from 2014-05-28 Pipeline Factory to 2014-06-17 Curating and Crunch
Actions #4

Updated by Tom Clegg over 10 years ago

Now at 15cc0bc

  • Renamed to has_symbols?
  • Commented convert_serialized_symbols_to_strings
  • Convert all serialized types in convert_serialized_symbols_to_strings, not just Hash
  • Added test cases as suggested
  • Improved readability of has_symbols?
Actions #5

Updated by Anonymous over 10 years ago

  • Status changed from In Progress to Resolved

Applied in changeset arvados|commit:21485541dec5b6df36aaba7d4c2a1e96ba65dec6.

Actions

Also available in: Atom PDF