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

Some event sending fail when POST request with non ASCII string #1475

Closed
mkusaka opened this issue Jun 20, 2021 · 3 comments · Fixed by #1484
Closed

Some event sending fail when POST request with non ASCII string #1475

mkusaka opened this issue Jun 20, 2021 · 3 comments · Fixed by #1484
Assignees
Projects
Milestone

Comments

@mkusaka
Copy link

mkusaka commented Jun 20, 2021

Describe the bug
When POST request through fetch with non ASCII string, error event failed to send to sentry server.

With config.debug = true, following log printed in console.

Event sending failed: "\xE3" from ASCII-8BIT to UTF-8
/Users/username/.rbenv/versions/2.7.3/lib/ruby/2.7.0/json/common.rb:224:in `generate'
/Users/username/.rbenv/versions/2.7.3/lib/ruby/2.7.0/json/common.rb:224:in `generate'
/Users/username/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/sentry-ruby-core-4.5.1/lib/sentry/transport.rb:102:in `encode'
/Users/username/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/sentry-ruby-core-4.5.1/lib/sentry/transport.rb:42:in `send_event'
/Users/username/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/sentry-ruby-core-4.5.1/lib/sentry/client.rb:86:in `send_event'
/Users/username/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/sentry-ruby-core-4.5.1/lib/sentry/client.rb:110:in `block in dispatch_background_event'
/Users/username/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/sentry-rails-4.5.1/lib/sentry/rails/background_worker.rb:7:in `block (2 levels) in perform'
...

To Reproduce
see: https://github.com/mkusaka/sentry-ruby-issue-repro

Expected behavior
Event sending not fail when POST request through fetch with non ASCII string.

Actual behavior
Event sending fail when POST request through fetch with non ASCII string.

Environment

  • Ruby Version: 2.7.3
  • SDK Version:
    • sentry-ruby: 4.5.1
    • sentry-rails: 4.5.1
  • Integration Versions (if any):
    • rails: 5.2.6

Sentry Config

This is not necessary but could be helpful.

Sentry.init do |config|
  config.send_default_pii = true
  config.debug = true
end

additional info
I've been searching for a while, and below is my summary for search.

Why this error occurred?

The direct cause is Rack::Request's body data encoding. It is encoded to ASCII-8BIT, and non ascii string convert to escaped string. And JSON.generate try to convert to UTF-8, but no conversion for escaped string, then raise error.

  • If config.send_default_pii = true and user send POST request from NON HTML form, sentry-ruby try to send event with request body.
  • Rack::Request#body#read returns ASCII-8BIT string.
    • NON ASCII-8BIT string converted to an escaped string, such as \xE5 etc...
  • JSON.generate (which called at Sentry::Transport#encode), try to convert ASCII-8BIT string into UTF-8, then raise this error.

How to fix this issue

Force-encode request body string to UTF-8 make fix this issue.

For example, in:

if request.form_data?
request.POST
elsif request.body # JSON requests, etc
data = request.body.read(MAX_BODY_LIMIT)
request.body.rewind
data
end

      if request.form_data?
        request.POST
      elsif request.body # JSON requests, etc
-       data = request.body.read(MAX_BODY_LIMIT)
+       data = request.body.read(MAX_BODY_LIMIT).force_encoding(Encoding::UTF_8)
        request.body.rewind
        data
      end

But, I'm not sure this is a good solution, because I don't know force_encoding works well in other situations.

  • Rails seems to convert (ASCII) URL to UTF-8 at rails/rails@7e50492, so I believe this solution works well in many cases.

similar: #1303

@st0012 st0012 added this to To do in 4.x via automation Jun 21, 2021
@st0012
Copy link
Collaborator

st0012 commented Jun 21, 2021

@mkusaka thanks for this detail report! I'll take a look later this week 🙂

@st0012 st0012 modified the milestones: 4.5.2, 4.6.0 Jun 21, 2021
@st0012
Copy link
Collaborator

st0012 commented Jun 25, 2021

it's weird that I can reproduce this in your example app (thanks again for this), but not in sentry-rails' example app.

Processing by DocsController#create as */*
  Parameters: {"title"=>"あ", "content"=>"qweqwe", "doc"=>{"content"=>"qweqwe"}}
Completed 500 Internal Server Error in 1ms (ActiveRecord: 0.0ms)



Event sending failed: "\xE3" from ASCII-8BIT to UTF-8
Processing by PostsController#create as HTML
  Parameters: {"authenticity_token"=>TOKEN, "post"=>{"title"=>"あ", "content"=>"qweqw"}, "commit"=>"Create Post"}
Completed 500 Internal Server Error in 1ms (ActiveRecord: 0.0ms | Allocations: 1051)


Sending envelope [event] 3b5513966dda4e0c9a618ba955f67e99 to Sentry

截圖 2021-06-25 下午6 58 09

I wonder if the difference is the request format?

Processing by DocsController#create as */*
Processing by PostsController#create as HTML

anyway, I'm working on it now 🙂


Update

Yes it's because the request handles the data differently:

in the issue repo it's a string

"{\"title\":\"\xE3\x81\x82\",\"content\":\"qweqwe\"}"

in the example app it's a hash

{"authenticity_token"=>
  "nWO3LHTdUgorC3QrPAGSFuyWigHeVsylU0IREXrHbw/XwzsVNRrzBNKVzW7QLcpYhqmmUB3WC2o66LQblUZ4LA==",
 "post"=>{"title"=>"あ", "content"=>"qweqw"},
 "commit"=>"Create Post"}

@st0012 st0012 added this to the 4.6.0 milestone Jun 25, 2021
st0012 added a commit that referenced this issue Jun 25, 2021
4.x automation moved this from To do to Done Jun 25, 2021
st0012 added a commit that referenced this issue Jun 25, 2021
* Force encode request body if it's a string

This fixes #1475

* Update changelog
@st0012
Copy link
Collaborator

st0012 commented Jun 25, 2021

Fixed with #1484

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
4.x
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants