Skip to content

rm reference to old constant (from Rails v2.2)#1184

Merged
st0012 merged 1 commit intogetsentry:masterfrom
tonyta:rm-cgi-session-cookiestore
Jan 6, 2021
Merged

rm reference to old constant (from Rails v2.2)#1184
st0012 merged 1 commit intogetsentry:masterfrom
tonyta:rm-cgi-session-cookiestore

Conversation

@tonyta
Copy link
Copy Markdown
Contributor

@tonyta tonyta commented Jan 5, 2021

Description

CGI::Session::CookieStore::TamperedWithCookie was defined as part of an extension of Ruby's CGI in Rails v2.2. So technically, it probably belongs in sentry-rails as opposed to sentry-ruby. However, this constant is no longer present in Rails >= 5.0 (the requirement for sentry-rails).

The TamperedWithCookie constant was originally referenced by Raven in a refactor of its backtrace cleaner in 2012. The CookieStore was removed from Rails in 2008 when ActionPack switched to using Rack-based sessions.

Note: this constant is also referred to by sentry-raven but I didn't bother removing it there since it's in maintenance mode.

CGI::Session::CookieStore::TamperedWithCookie was originally referenced
by Raven in a refactor of its backtrace cleaner in 2012.

  https://github.com/getsentry/sentry-ruby/pull/58/files#diff-2337b0e4ecfcd526e8dbf0aa6f3a5570a2228c2cc71afdf6ba0edd4d361a40c2R45

CGI::Session::CookieStore was removed from Rails in 2008 when ActionPack
switched to using Rack-based sessions.

  rails/rails@ed70830#diff-018dc029403b3cf53f21aef6fc77debda6e09c0739f6d7ff6a1331e40e5135eaL48
@codecov-io
Copy link
Copy Markdown

codecov-io commented Jan 5, 2021

Codecov Report

Merging #1184 (a1b029e) into master (e00745c) will increase coverage by 0.58%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1184      +/-   ##
==========================================
+ Coverage   97.94%   98.52%   +0.58%     
==========================================
  Files         192       97      -95     
  Lines        8036     4211    -3825     
==========================================
- Hits         7871     4149    -3722     
+ Misses        165       62     -103     
Impacted Files Coverage Δ
sentry-ruby/lib/sentry/configuration.rb 97.20% <ø> (ø)
...ntry-raven/spec/raven/integrations/sidekiq_spec.rb
...c/raven/processors/removecirculareferences_spec.rb
sentry-raven/spec/raven/integrations/rails_spec.rb
...ven/spec/raven/utils/exception_cause_chain_spec.rb
...y-raven/lib/raven/integrations/rails/active_job.rb
sentry-raven/lib/raven/cli.rb
sentry-raven/lib/raven/utils/request_id.rb
sentry-raven/lib/raven/transports/dummy.rb
sentry-raven/lib/raven/integrations/rails.rb
... and 86 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e00745c...a1b029e. Read the comment docs.

Copy link
Copy Markdown
Contributor

@st0012 st0012 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for pointing this out. I can confirm that it's completely gone in Rails 5+ versions.

@st0012 st0012 added this to the 4.1.3 milestone Jan 6, 2021
@st0012 st0012 merged commit 13a03a5 into getsentry:master Jan 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants