Skip to content

Commit e8ad1e9

Browse files
Avoid silently overwriting with merge
At the moment merge! with an array will overwrite any existing hash attributes. It should probably raise a MergeError. It also makes sense to raise this error when attempting to merge a hash into an array, or when trying to merge a non-hash-or-array.
1 parent 02fc9c8 commit e8ad1e9

File tree

3 files changed

+39
-5
lines changed

3 files changed

+39
-5
lines changed

lib/jbuilder.rb

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -270,14 +270,14 @@ def _merge_block(key)
270270
def _merge_values(current_value, updates)
271271
if _blank?(updates)
272272
current_value
273-
elsif _blank?(current_value) || updates.nil?
273+
elsif _blank?(current_value) || updates.nil? || current_value.empty? && ::Array === updates
274274
updates
275-
elsif ::Array === updates
276-
::Array === current_value ? current_value + updates : updates
277-
elsif ::Hash === current_value
275+
elsif ::Array === current_value && ::Array === updates
276+
current_value + updates
277+
elsif ::Hash === current_value && ::Hash === updates
278278
current_value.merge(updates)
279279
else
280-
raise "Can't merge #{updates.inspect} with #{current_value.inspect}"
280+
raise MergeError.build(current_value, updates)
281281
end
282282
end
283283

lib/jbuilder/errors.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,11 @@ def self.build(key)
1414
new(message)
1515
end
1616
end
17+
18+
class MergeError < ::StandardError
19+
def self.build(current_value, updates)
20+
message = "Can't merge #{updates.inspect} into #{current_value.inspect}"
21+
new(message)
22+
end
23+
end
1724
end

test/jbuilder_test.rb

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -686,4 +686,31 @@ class JbuilderTest < ActiveSupport::TestCase
686686
end
687687
end
688688
end
689+
690+
test "throws MergeError when trying to merge array with non-empty hash" do
691+
assert_raise Jbuilder::MergeError do
692+
jbuild do |json|
693+
json.name "Daniel"
694+
json.merge! []
695+
end
696+
end
697+
end
698+
699+
test "throws MergeError when trying to merge hash with array" do
700+
assert_raise Jbuilder::MergeError do
701+
jbuild do |json|
702+
json.array!
703+
json.merge!({})
704+
end
705+
end
706+
end
707+
708+
test "throws MergeError when trying to merge invalid objects" do
709+
assert_raise Jbuilder::MergeError do
710+
jbuild do |json|
711+
json.name "Daniel"
712+
json.merge! "Nope"
713+
end
714+
end
715+
end
689716
end

0 commit comments

Comments
 (0)