dd-trace-rb icon indicating copy to clipboard operation
dd-trace-rb copied to clipboard

dd-trace causes `bad URI(is not URI?)` with invalid URIs

Open gingerlime opened this issue 3 years ago • 3 comments

Current behaviour If the URL contains a |, then rails actually handles the request (it might return route not found / 404 etc), but dd-trace seems to explode. If I remove dd-trace from our system, it works as expected.

curl "http://localhost:3000/|"

Puma caught this error: bad URI(is not URI?): "http://localhost:3000/|" (URI::InvalidURIError)
/usr/local/lib/ruby/3.0.0/uri/rfc3986_parser.rb:67:in `split'
/usr/local/lib/ruby/3.0.0/uri/rfc3986_parser.rb:72:in `parse'
/usr/local/lib/ruby/3.0.0/uri/common.rb:171:in `parse'
/usr/local/bundle/gems/ddtrace-1.5.0/lib/datadog/tracing/contrib/utils/quantization/http.rb:26:in `base_url'
/usr/local/bundle/gems/ddtrace-1.5.0/lib/datadog/tracing/contrib/rack/middlewares.rb:185:in `set_request_tags!'
/usr/local/bundle/gems/ddtrace-1.5.0/lib/datadog/tracing/contrib/rack/middlewares.rb:110:in `call'
/usr/local/bundle/gems/railties-7.0.4/lib/rails/engine.rb:530:in `call'
/usr/local/bundle/gems/puma-5.6.5/lib/puma/configuration.rb:252:in `call'
/usr/local/bundle/gems/puma-5.6.5/lib/puma/request.rb:77:in `block in handle_request'
/usr/local/bundle/gems/puma-5.6.5/lib/puma/thread_pool.rb:340:in `with_force_shutdown'
/usr/local/bundle/gems/puma-5.6.5/lib/puma/request.rb:76:in `handle_request'
/usr/local/bundle/gems/puma-5.6.5/lib/puma/server.rb:443:in `process_client'
/usr/local/bundle/gems/puma-5.6.5/lib/puma/thread_pool.rb:147:in `block in spawn_thread

Expected behaviour It should handle invalid URIs safely.

Steps to reproduce Start a rails app with dd-trace ; Go to http://localhost:3000/| or run curl "http://localhost:3000/|"

Environment

  • ddtrace version: 1.5.0
  • Configuration block (Datadog.configure ...):
Datadog.configure do |c|
  c.tracing.enabled = false  # can reproduce both when true or false
  c.env = Rails.env
  c.agent.port = 8127
  c.service = "app_name"
  # when commenting-out the rails instrumentation it works
  c.tracing.instrument :rails,
                       :controller_service => "rails-controller",
                       :cache_service => "rails-cache"

  # Normally these are not commented out, but I can reproduce even with those commented-out

  # c.tracing.instrument :redis
  # c.tracing.instrument :sidekiq
  # c.tracing.instrument :http
  # c.tracing.instrument :rack,
  #                      :quantize => {
  #                        :base => :exclude,
  #                        :query => :show,
  #                        :fragment => :show
  #                      }
  c.telemetry.enabled = false
end
  • Ruby version: 3
  • Operating system: Ubuntu 20.04
  • Relevant library versions: Rails 7.0.4

gingerlime avatar Oct 11 '22 15:10 gingerlime

I tested 1.4.2 and it doesn't exhibit the same problem, so this was introduced in 1.5.0

gingerlime avatar Oct 11 '22 15:10 gingerlime

I get the same problem with 1.5.0. Turns out that i have gibberish in the URL [...] and that causes a 500 error. If i remove the library, then rails is able raise a 404. Tested on 1.4.2, and it's working properly

sailor avatar Oct 11 '22 20:10 sailor

Thanks for the report folks! Looking into this.

lloeki avatar Oct 12 '22 13:10 lloeki