Sentry-ruby: Raven.capture_type has side-effect to destructively update rack env on sanitization

[ohbarye]

I'm not sure whether it's a bug or a specification.

Describe the bug

Raven::Processor::SanitizeData destructively updates Rack's env with a mask string ******** when a request's content type is 'multipart/form-data'.

Because...

• Raven::Rack directly extracts Rack's env by calling Rack::Request#POST when Rack::Request#form_data? is true.
  
  
  
  • https://github.com/getsentry/sentry-ruby/blob/82e1ffe711af287ddc23e8517bdb8275beff94d5/sentry-raven/lib/raven/integrations/rack.rb#L87-L89
  
  
• And Raven::Event#to_hash creates a hash with the same pointer to the env value.
• Raven::Client passes the hash to processors
  
  
  
  • https://github.com/getsentry/sentry-ruby/blob/82e1ffe711af287ddc23e8517bdb8275beff94d5/sentry-raven/lib/raven/client.rb#L74-L75
  
  
• Raven::Processor::SanitizeData destructively updates Rack's env with merge!
  
  
  
  • https://github.com/getsentry/sentry-ruby/blob/82e1ffe711af287ddc23e8517bdb8275beff94d5/sentry-raven/lib/raven/processor/sanitizedata.rb#L44-L50
  
  

To Reproduce

I prepared a demo project to reproduce the behavior.
Please see https://github.com/ohbarye/senry-raven-sanitize-demo

1. Set up sentry-raven with sanitize_fields.
2. Write code to call Raven.capture_exception in a web application using Raven::Rack.
3. Send a request with content-type: multipart/form-data.
   
   
   
   • e.g. curl -X POST http://localhost:9292 -F 'name=foo' -F 'age=32'
   
   
4. Check env["rack.request.form_hash"] before/after calling Raven.capture_exception. It overrides an original rack env.

Expected behavior

Raven.capture_type does not have any side-effects to destructively updates rack env.

In my use-case, my application calls Raven.capture_exception in a rack middleware if an incoming request is invalid, subsequently, Rails processes the request. But Rails cannot check original parameters (form data) since some parts of parameters are destructively masked by Raven::Processor::SanitizeData.

Actual behavior

Raven.capture_type has a side-effect to destructively updates rack env.

Environment

• Ruby Version: 2.7.1
• SDK Version: sentry-raven 3.1.1
• Integration Versions (if any):
  
  
  
  • rack 2.1.1
  
  

Raven Config

https://github.com/ohbarye/senry-raven-sanitize-demo/blob/7e200b1e71354684e2291580b3a5d5e079093499/config.ru#L4-L7

Raven.configure do |config|
  config.dsn = ENV['SENTRY_DSN']
  config.sanitize_fields = %w[name]
end

[st0012]

@ohbarye thanks for the detailed report 🙏
I think you did point out the root cause. the old SDK tries to use merge! whenever possible for reducing memory allocation when processing the data. and the downside of it is causing issues like this 😞

the most effective solution to me right now is to duplicate the form data and only process the copy. but I believe it'll allocate much more memory when the request body is large. so I'll need some time to look for any possible alternatives in the following days (when I'm not busy with the new SDK, unfortunately).

in the meantime, I recommend you to try server-side data scrubbing. it's SDK independent so you don't need to migrate to the new SDK for using it.

[ohbarye]

@st0012
I understand the current situation and my necessity of trying an approach except for waiting for an immediate fix.
Maybe, as you recommended, server-side data scrubbing or migration to the new SDK would be my decision.

Thanks for your reply! 👍

[st0012]

@ohbarye I hope you've migrated to the new SDK and aren't bothered by this issue anymore 🙂

[ohbarye]

@st0012 Out project has successfully migrated to the new one recently. Thank you!

[st0012]

@ohbarye glad to hear that! I'll close this then 😉