Skip to content

Commit

Permalink
Explicitly passing Rack related configurations (#1662)
Browse files Browse the repository at this point in the history
* Explicitly passing rack related configurations

* Update changelog
  • Loading branch information
st0012 authored Jan 4, 2022
1 parent d583528 commit 7fb3092
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 31 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,15 @@ When `config.send_default_pii` is set as `true`, `:http_logger` will include que

- Rewrite documents with yard [#1635](https://github.com/getsentry/sentry-ruby/pull/1635)
- Document Transaction and Span classes [#1653](https://github.com/getsentry/sentry-ruby/pull/1653)
- Document Client and Scope classes [#1659](https://github.com/getsentry/sentry-ruby/pull/1659)

### Refactoring

- Minor improvements on Net::HTTP patch [#1651](https://github.com/getsentry/sentry-ruby/pull/1651)
- Deprecate unnecessarily exposed attributes [#1652](https://github.com/getsentry/sentry-ruby/pull/1652)
- Refactor Net::HTTP patch [#1656](https://github.com/getsentry/sentry-ruby/pull/1656)
- Deprecate Event#configuration [#1661](https://github.com/getsentry/sentry-ruby/pull/1661)
- Explicitly passing Rack related configurations [#1662](https://github.com/getsentry/sentry-ruby/pull/1662)

## 4.8.1

Expand Down
5 changes: 3 additions & 2 deletions sentry-ruby/lib/sentry/event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class Event

MAX_MESSAGE_SIZE_IN_BYTES = 1024 * 8

SKIP_INSPECTION_ATTRIBUTES = [:@modules, :@backtrace, :@stacktrace_builder, :@send_default_pii, :@trusted_proxies]
SKIP_INSPECTION_ATTRIBUTES = [:@modules, :@backtrace, :@stacktrace_builder, :@send_default_pii, :@trusted_proxies, :@rack_env_whitelist]

include CustomInspection

Expand Down Expand Up @@ -55,6 +55,7 @@ def initialize(configuration:, integration_meta: nil, message: nil)
@send_default_pii = configuration.send_default_pii
@trusted_proxies = configuration.trusted_proxies
@stacktrace_builder = configuration.stacktrace_builder
@rack_env_whitelist = configuration.rack_env_whitelist

@message = (message || "").byteslice(0..MAX_MESSAGE_SIZE_IN_BYTES)

Expand Down Expand Up @@ -132,7 +133,7 @@ def to_json_compatible
end

def add_request_interface(env)
@request = Sentry::RequestInterface.build(env: env)
@request = Sentry::RequestInterface.build(env: env, send_default_pii: @send_default_pii, rack_env_whitelist: @rack_env_whitelist)
end

def add_threads_interface(backtrace: nil, **options)
Expand Down
22 changes: 11 additions & 11 deletions sentry-ruby/lib/sentry/interfaces/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ class RequestInterface < Interface

attr_accessor :url, :method, :data, :query_string, :cookies, :headers, :env

def self.build(env:)
env = clean_env(env)
def self.build(env:, send_default_pii:, rack_env_whitelist:)
env = clean_env(env, send_default_pii)
request = ::Rack::Request.new(env)
self.new(request: request)
self.new(request: request, send_default_pii: send_default_pii, rack_env_whitelist: rack_env_whitelist)
end

def self.clean_env(env)
unless Sentry.configuration.send_default_pii
def self.clean_env(env, send_default_pii)
unless send_default_pii
# need to completely wipe out ip addresses
RequestInterface::IP_HEADERS.each do |header|
env.delete(header)
Expand All @@ -34,10 +34,10 @@ def self.clean_env(env)
env
end

def initialize(request:)
def initialize(request:, send_default_pii:, rack_env_whitelist:)
env = request.env

if Sentry.configuration.send_default_pii
if send_default_pii
self.data = read_data_from(request)
self.cookies = request.cookies
self.query_string = request.query_string
Expand All @@ -47,7 +47,7 @@ def initialize(request:)
self.method = request.request_method

self.headers = filter_and_format_headers(env)
self.env = filter_and_format_env(env)
self.env = filter_and_format_env(env, rack_env_whitelist)
end

private
Expand Down Expand Up @@ -116,11 +116,11 @@ def is_server_protocol?(key, value, protocol_version)
key == 'HTTP_VERSION' && value == protocol_version
end

def filter_and_format_env(env)
return env if Sentry.configuration.rack_env_whitelist.empty?
def filter_and_format_env(env, rack_env_whitelist)
return env if rack_env_whitelist.empty?

env.select do |k, _v|
Sentry.configuration.rack_env_whitelist.include? k.to_s
rack_env_whitelist.include? k.to_s
end
end
end
Expand Down
35 changes: 17 additions & 18 deletions sentry-ruby/spec/sentry/interfaces/request_interface_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,11 @@

RSpec.describe Sentry::RequestInterface do
let(:env) { Rack::MockRequest.env_for("/test") }
let(:send_default_pii) { false }
let(:rack_env_whitelist) { Sentry::Configuration::RACK_ENV_WHITELIST_DEFAULT }

subject do
described_class.build(env: env)
end

before do
Sentry.init do |config|
config.dsn = DUMMY_DSN
end
described_class.build(env: env, send_default_pii: send_default_pii, rack_env_whitelist: rack_env_whitelist)
end

describe "rack_env_whitelist" do
Expand All @@ -26,15 +22,20 @@
expect(subject.env).to_not include(additional_env)
end

it 'formats rack env according to the provided whitelist' do
Sentry.configuration.rack_env_whitelist = %w(random_param query_string)
expect(subject.env).to eq(additional_env)
context "with provided whitelist" do
let(:rack_env_whitelist) { %w(random_param query_string) }

it 'formats rack env according to the provided whitelist' do
expect(subject.env).to eq(additional_env)
end
end

it 'keeps the original env intact when an empty whitelist is provided' do
Sentry.configuration.rack_env_whitelist = []
context "with empty whitelist" do
let(:rack_env_whitelist) { [] }

expect(subject.env).to eq(env)
it 'keeps the original env intact' do
expect(subject.env).to eq(env)
end
end
end

Expand Down Expand Up @@ -71,7 +72,7 @@
it 'does not call #to_s for unnecessary env variables' do
expect(mock).not_to receive(:to_s)

described_class.build(env: env)
subject
end
end
end
Expand Down Expand Up @@ -124,7 +125,7 @@ def to_s

env.merge!("HTTP_FOO" => "BAR", "rails_object" => obj)

expect { described_class.build(env: env) }.to_not raise_error
expect { described_class.build(env: env, send_default_pii: send_default_pii, rack_env_whitelist: rack_env_whitelist) }.to_not raise_error
end
end

Expand All @@ -147,9 +148,7 @@ def to_s
end

context "with config.send_default_pii = true" do
before do
Sentry.configuration.send_default_pii = true
end
let(:send_default_pii) { true }

it "stores cookies" do
env.merge!(::Rack::RACK_REQUEST_COOKIE_HASH => "cookies!")
Expand Down

0 comments on commit 7fb3092

Please sign in to comment.