[BUG] Rollbar.log may silently modify it's payload

[zhulik]

Description

Rollbar.log may modify payload passed by the user and corrupt data that must remain unchanged.

Version: 3.0.0

How to reproduce

require 'rollbar'

Rollbar.configure do |config|
  config.access_token = 'valid_token'
end

reference = { key: 'value' }

a = {
  key1: reference,
  key2: reference
}

payload = { key1: a, key2: a }

pp payload

Rollbar.log(:warning, 'Warning', data: payload)

pp payload

Console output:

:key1=>{:key1=>{:key=>"value"}, :key2=>{:key=>"value"}},
 :key2=>{:key1=>{:key=>"value"}, :key2=>{:key=>"value"}}}
I, [2020-10-01T11:04:36.540522 #1753801]  INFO -- : [Rollbar] Scheduling item
I, [2020-10-01T11:04:36.540639 #1753801]  INFO -- : [Rollbar] Sending item
I, [2020-10-01T11:04:36.540971 #1753801]  INFO -- : [Rollbar] Sending json
I, [2020-10-01T11:04:36.798777 #1753801]  INFO -- : [Rollbar] Success
I, [2020-10-01T11:04:36.799196 #1753801]  INFO -- : [Rollbar] Details: https://rollbar.com/instance/uuid?uuid=0c0cf012-f2aa-4452-8c88-188131d3e63f (only available if report was successful)
{:key1=>
  {:key1=>"removed circular reference: {:key=>\"value\"}",
   :key2=>"removed circular reference: {:key=>\"value\"}"},
 :key2=>
  {:key1=>"removed circular reference: {:key=>\"value\"}",
   :key2=>"removed circular reference: {:key=>\"value\"}"}}

Expected result

Payload remains unmodified.

Actual result

Payload is modified.

Additional example

Now let's freeze the a hash:

a = {
  key1: reference,
  key2: reference
}.freeze

Output:

{:key1=>{:key1=>{:key=>"value"}, :key2=>{:key=>"value"}},
 :key2=>{:key1=>{:key=>"value"}, :key2=>{:key=>"value"}}}
I, [2020-10-01T11:07:54.351523 #1754221]  INFO -- : [Rollbar] Scheduling item
I, [2020-10-01T11:07:54.351634 #1754221]  INFO -- : [Rollbar] Sending item
E, [2020-10-01T11:07:54.351912 #1754221] ERROR -- : [Rollbar] Error processing the item: FrozenError, can't modify frozen Hash. Item: {"data"=>{:timestamp=>1601543274, :environment=>"unspecified", :level=>:warning, :language=>"ruby", :framework=>"Plain", :server=>{:host=>"host", :pid=>1754221}, :notifier=>{:name=>"rollbar-gem", :version=>"3.0.0", :configured_options=>{:access_token=>"******"}}, :body=>{:message=>{:body=>"Warning", :extra=>{:data=>{:key1=>{:key1=>{:key=>"value"}, :key2=>{:key=>"value"}}, :key2=>{:key1=>{:key=>"value"}, :key2=>{:key=>"value"}}}}}}, :uuid=>"9a3378d0-6eb0-409a-8f0d-e504a2c2825d"}}
E, [2020-10-01T11:07:54.352063 #1754221] ERROR -- : [Rollbar] Reporting internal error encountered while sending data to Rollbar.
I, [2020-10-01T11:07:54.357062 #1754221]  INFO -- : [Rollbar] Sending item
I, [2020-10-01T11:07:54.359748 #1754221]  INFO -- : [Rollbar] Sending json
I, [2020-10-01T11:07:54.606831 #1754221]  INFO -- : [Rollbar] Success
I, [2020-10-01T11:07:54.607147 #1754221]  INFO -- : [Rollbar] Details: https://rollbar.com/instance/uuid?uuid=0161801c-1f49-4824-a2c8-3737cc73f193 (only available if report was successful)
{:key1=>{:key1=>{:key=>"value"}, :key2=>{:key=>"value"}},
 :key2=>{:key1=>{:key=>"value"}, :key2=>{:key=>"value"}}}

As you can see, the gem tries to modify it's payload directly instead of modifying a copy of the payload. I understand that there is a some sort of circuit breaker in the gem that may cause such behavior, but in my examples references are not even circular.

Thank you!

[waltjones]

@zhulik Thank you for the report. Yes, this is a bug, and it should use a copy of the input.

Regarding the circular reference, the code path checks whether it has seen the same object before. For performance reasons, it doesn't traverse the parent chain to verify a cycle. Maybe it's worth the perf hit, or maybe a better message would say "possible circular reference".

[troex]

Bump! This bug drives us crazy, we use Rollbar.info('message', payload) in code a lot and payload is often frozen on purpose in our system, this causes rollbar for rollbar error along wihout our data mixed. They solution is to use payload.dup but it's inconvinient to remember about it all the time.

Trace from older version:

File /app/vendor/bundle/ruby/3.0.0/gems/rollbar-3.2.0/lib/rollbar/middleware/rack.rb line 21 in block in call
File /app/vendor/bundle/ruby/3.0.0/gems/rollbar-3.2.0/lib/rollbar.rb line 145 in scoped
File /app/vendor/bundle/ruby/3.0.0/gems/rollbar-3.2.0/lib/rollbar/middleware/rack.rb line 18 in call

[saiqulhaq-hh]

this issue seems dangerous is it hard to fix this issue? is it not enough to add variable.dup on Rollbar code?