Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

request.cookies: Discarded invalid value #900

Closed
sanjibukai opened this issue Apr 30, 2019 · 5 comments · Fixed by #983
Closed

request.cookies: Discarded invalid value #900

sanjibukai opened this issue Apr 30, 2019 · 5 comments · Fixed by #983
Assignees
Projects
Milestone

Comments

@sanjibukai
Copy link

Hi everybody,
I have setup sentry for a ruby/rails application.
And everything is working fine except none of the context data is working.
I tried with Raven.user_context, Raven.tags_context and also Raven.extra_context.
For every event I got the following error

request.cookies: Discarded invalid value

Reason      the cookie is missing a name/value pair
Value       ********

I googled this but I didn't get any relevant information about this error even on the Sentry website.
Does anyone be able to explain me what I'm doing wrong or if there is a bug somewhere?

I'm using ruby 2.6.1, rails 5.2.0 and sentry-raven 2.9.0.

2019-04-30-150530_1335x202_scrot

@FlorianDoublet
Copy link

We just run over that issue. I advise you to read https://docs.sentry.io/clients/ruby/processors/

In fact the default processor of raven-ruby sanitize Cookies by default. The solution for you could be to define the processors you want to use, for example :

config.processors = [
      Raven::Processor::RemoveCircularReferences,
      Raven::Processor::UTF8Conversion,
      Raven::Processor::SanitizeData,
      Raven::Processor::PostData,
      Raven::Processor::HTTPHeaders
    ]

@ttoomey
Copy link

ttoomey commented May 14, 2019

I also saw this on passenger 5. From the stack trace, it looks like something is trying to parse the cookie value after the processor has masked it out. This causes an exception since the masked value no longer has a valid format.

If you don't want to disable the cookie sanitizer, a quick workaround might be to monkeypatch Raven::Processor::Cookies so that it blanks out the value instead of using Raven::Processor::Cookies::STRING_MASK="********"

@josh-m-sharpe
Copy link

josh-m-sharpe commented Jun 4, 2019

Similar to @FlorianDoublet's solution above, this removes the cookie processor instead of redefining the list:

config.processors -= [Raven::Processor::Cookies]

@ndbroadbent
Copy link

Thanks @josh-m-sharpe, I just added your changes to my app in a custom processor, and it works great!

config/initializers/_sentry.rb

config.processors -= [Raven::Processor::Cookies]
config.processors += [Raven::Processor::FixedCookies]

lib/raven/processor/fixed_cookies.rb

# frozen_string_literal: true

# From: https://github.com/getsentry/raven-ruby/pull/904
# See also: https://github.com/getsentry/raven-ruby/issues/900

module Raven
  class Processor
    class FixedCookies < ::Raven::Processor
      def process(data)
        process_if_symbol_keys(data) if data[:request]
        process_if_string_keys(data) if data['request']

        data
      end

      private

      def process_if_symbol_keys(data)
        data[:request][:cookies] = data[:request][:cookies].merge(data[:request][:cookies]) { |_key, _val| STRING_MASK } if data[:request][:cookies]

        return unless data[:request][:headers] && data[:request][:headers]['Cookie']

        data[:request][:headers]['Cookie'] = STRING_MASK
      end

      def process_if_string_keys(data)
        data['request']['cookies'] = data['request']['cookies'].merge(data['request']['cookies']) { |_key, _val| STRING_MASK } if data['request']['cookies']

        return unless data['request']['headers'] && data['request']['headers']['Cookie']

        data['request']['headers']['Cookie'] = STRING_MASK
      end
    end
  end
end

@ju-popov
Copy link

great!

@st0012 st0012 added this to Needs triage in 3.x Aug 13, 2020
@st0012 st0012 moved this from Needs triage to High priority in 3.x Aug 13, 2020
@st0012 st0012 added this to the 3.0.1 milestone Aug 13, 2020
@st0012 st0012 self-assigned this Aug 13, 2020
3.x automation moved this from High priority to Closed Aug 13, 2020
@st0012 st0012 moved this from Closed to v3.0.1 in 3.x Aug 14, 2020
@st0012 st0012 moved this from v3.0.1 to Closed in 3.x Aug 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
3.x
  
Closed
Development

Successfully merging a pull request may close this issue.

7 participants