Closed TomNaessens closed 1 month ago
Good catch! Thanks for the report.
I'm not exactly sure what the right solution is
Probably something like:
h[k.gsub(/^HTTP_/, '').tap(&:downcase!).tr('_', '-')] = v if k =~ /^HTTP_/
And to save another allocation:
h[k.gsub(/^HTTP_/, '').tap(&:downcase!).tap { |s| s.tr!('_', '-') }] = v if k =~ /^HTTP_/
Note: not using gsub!
because we don't want to mutate the original key.
I'm wanting to open a PR and write some tests for this, one last question: what's the expected behaviour when such a header is sent? With the .tap
s, "malformed" headers not containing text will end up in the hash as nil => value
. Is that intended, or should they just not end up in the HeaderCollection at all?
Hello ! We have pushed a fix that should be included in the next release ! As for the headers that does not contain any text, they should be filtered out by Rack, but if it does happen, we still do want to send them to the WAF.
👋 @TomNaessens We've released a 2.3.0
, give it a try!
Awesome, thank you!
Hi all, 👋
We often have bots hit our systems with malformed HTTP headers. One of the malformed HTTP headers they apparently try (a lot), is a plain number (e.g.
HTTP_2
). This causes the following line to throw an "undefined method `tr' for nil:NilClass (NoMethodError)": https://github.com/DataDog/dd-trace-rb/blob/f867bd9f13d26a6662a3d409510c5c55bf5cd5f5/lib/datadog/appsec/contrib/rack/gateway/request.rb#L44k =~ /^HTTP_/
is matched,k.gsub(/^HTTP_/, '')
results into"2"
, which becomesnil
throughdowncase!
:I'm not exactly sure what the right solution is, given that we're already working with a new object after the
gsub
, a regulardowncase
(without!
) might work (although that will create another object):To reproduce, spin up a Rails app with dd trace, enable appsec, instrument Rack, and send a request with an integer as HTTP header: