From eed569772291c5d3c196049641082b34aa7dbe4b Mon Sep 17 00:00:00 2001 From: Doug Tabuchi Date: Thu, 2 Jul 2026 17:45:56 -0400 Subject: [PATCH] Prevent set_attrs deep merge from writing nested keys into top-level attrs When set_attrs is called with clobber=false and the incoming hash contains a nested hash under a key the target does not yet have (target[k] is nil), the recursive call received nil as its target. The 'target ||= @attrs' fallback then silently redirected the merge to the TOP-LEVEL attrs hash, writing the nested hash's keys there. Example: on an issue built from a sparse search result whose attrs lack fields.project, save! merging {"fields" => {"project" => {"key" => "REL"}}} overwrote the issue's own top-level "key" (e.g. "REL-77") with the project key "REL", corrupting every URL built from issue.key afterwards. Fix: before recursing, ensure target[k] is a Hash. Create it when missing, and replace it when a scalar is being superseded by a hash (previously that case raised IndexError by indexing into a String). The nil target fallback now only applies to the initial call, as the 'internal use only' comment intended. --- lib/jira/base.rb | 1 + spec/jira/base_spec.rb | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/lib/jira/base.rb b/lib/jira/base.rb index fb288603..5e77447b 100644 --- a/lib/jira/base.rb +++ b/lib/jira/base.rb @@ -396,6 +396,7 @@ def set_attrs(hash, clobber = true, target = nil) else hash.each do |k, v| if v.is_a?(Hash) + target[k] = {} unless target[k].is_a?(Hash) set_attrs(v, clobber, target[k]) else target[k] = v diff --git a/spec/jira/base_spec.rb b/spec/jira/base_spec.rb index 6756d8fe..3ba64b8c 100644 --- a/spec/jira/base_spec.rb +++ b/spec/jira/base_spec.rb @@ -339,6 +339,27 @@ class HasManyExample < JIRA::Base # :nodoc: subject.set_attrs({ 'foo' => { 'fum' => 'dum' } }, false) expect(subject.foo).to eq('bar' => 'baz', 'fum' => 'dum') end + + it 'creates missing intermediate hashes instead of merging into the top level when clobber is false' do + subject.attrs = { 'id' => '10500', 'key' => 'REL-77', 'fields' => { 'summary' => 'x' } } + subject.set_attrs({ 'fields' => { 'project' => { 'key' => 'REL' } } }, false) + expect(subject.attrs['key']).to eq('REL-77') + expect(subject.attrs['fields']['project']['key']).to eq('REL') + expect(subject.attrs['fields']['summary']).to eq('x') + end + + it 'deeply merges into an existing nested hash when clobber is false' do + subject.attrs = { 'fields' => { 'project' => { 'id' => '10000' }, 'summary' => 'x' } } + subject.set_attrs({ 'fields' => { 'project' => { 'key' => 'REL' } } }, false) + expect(subject.attrs['fields']['project']).to eq('id' => '10000', 'key' => 'REL') + expect(subject.attrs['fields']['summary']).to eq('x') + end + + it 'replaces a non-hash value with the new hash when clobber is false' do + subject.attrs = { 'fields' => { 'project' => 'REL' } } + subject.set_attrs({ 'fields' => { 'project' => { 'key' => 'REL' } } }, false) + expect(subject.attrs['fields']['project']).to eq('key' => 'REL') + end end describe 'delete' do