Skip to content

Commit

Permalink
revert #3153
Browse files Browse the repository at this point in the history
  • Loading branch information
@GustavoCaso
GustavoCaso committed Nov 10, 2023
1 parent 687661d commit 93957a0
Show file tree
Hide file tree
Showing 8 changed files with 0 additions and 342 deletions.
10 changes: 0 additions & 10 deletions .gitlab/benchmarks.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,16 +128,6 @@ candidate-tracer-appsec-with-api-security:
DD_EXPERIMENTAL_API_SECURITY_ENABLED: "true"
DD_API_SECURITY_REQUEST_SAMPLE_RATE: "1.0"

candidate-tracer-appsec-with-api-security-without-response-body:
extends: .benchmarks
variables:
DD_BENCHMARKS_CONFIGURATION: "tracing-and-appsec-with-api-security-without-response-body"
DD_PROFILING_ENABLED: "false"
DD_APPSEC_ENABLED: "true"
DD_EXPERIMENTAL_API_SECURITY_ENABLED: "true"
DD_API_SECURITY_REQUEST_SAMPLE_RATE: "1.0"
DD_API_SECURITY_PARSE_RESPONSE_BODY: "false"

# -----------------------------------------------------
# Microbenchmarks that report to statsd
# -----------------------------------------------------
Expand Down
6 changes: 0 additions & 6 deletions lib/datadog/appsec/configuration/settings.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,12 +188,6 @@ def self.add_settings!(base)
end
end
end

option :parse_response_body do |o|
o.type :bool
o.env 'DD_API_SECURITY_PARSE_RESPONSE_BODY'
o.default true
end
end
end
end
Expand Down
46 changes: 0 additions & 46 deletions lib/datadog/appsec/contrib/rack/gateway/response.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,55 +19,9 @@ def initialize(body, status, headers, scope:)
@scope = scope
end

def parsed_body
return unless Datadog.configuration.appsec.parse_response_body

unless body.instance_of?(Array)
Datadog.logger.debug do
"Response body type unsupported: #{body.class}"
end
return
end

return unless json_content_type?

result = ''.dup
all_body_parts_are_string = true

body.each do |body_part|
if body_part.is_a?(String)
result.concat(body_part)
else
all_body_parts_are_string = false
break
end
end

return unless all_body_parts_are_string

begin
JSON.parse(result)
rescue JSON::ParserError => e
Datadog.logger.debug { "Failed to parse response body. Error #{e.class}. Message #{e.message}" }
nil
end
end

def response
@response ||= ::Rack::Response.new(body, status, headers)
end

private

VALID_JSON_TYPES = [
'application/json',
'text/json'
].freeze

def json_content_type?
content_type = headers['content-type']
VALID_JSON_TYPES.include?(content_type)
end
end
end
end
Expand Down
5 changes: 0 additions & 5 deletions lib/datadog/appsec/contrib/rack/reactive/response.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,13 @@ module Response
ADDRESSES = [
'response.status',
'response.headers',
'response.body',
].freeze
private_constant :ADDRESSES

def self.publish(op, gateway_response)
catch(:block) do
op.publish('response.status', gateway_response.status)
op.publish('response.headers', gateway_response.headers)
op.publish('response.body', gateway_response.parsed_body)

nil
end
Expand All @@ -31,16 +29,13 @@ def self.subscribe(op, waf_context)
response_status = values[0]
response_headers = values[1]
response_headers_no_cookies = response_headers.dup.tap { |h| h.delete('set-cookie') }
response_body = values[2]

waf_args = {
'server.response.status' => response_status.to_s,
'server.response.headers' => response_headers,
'server.response.headers.no_cookies' => response_headers_no_cookies,
}

waf_args['server.response.body'] = response_body if response_body

waf_timeout = Datadog.configuration.appsec.waf_timeout
result = waf_context.run(waf_args, waf_timeout)

Expand Down
10 changes: 0 additions & 10 deletions lib/datadog/appsec/contrib/rack/request_middleware.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,6 @@ def call(env)
request_return = AppSec::Response.negotiate(env, blocked_event.last[:actions]).to_rack if blocked_event
end

if request_return[2].respond_to?(:to_ary)
# Following the Rack specification. The response body should only call :each once.
# Calling :to_ary returns an array with identical content as the produced when calling :each
# replacing request_return[2] with that new value allow us to safely operate on the response body.
# On Gateway::Response#parsed_body we might iterate over the reposne body using :each
# https://github.com/rack/rack/blob/main/SPEC.rdoc#enumerable-body-
consumed_body = request_return[2].to_ary
request_return[2] = consumed_body if consumed_body
end

gateway_response = Gateway::Response.new(
request_return[2],
request_return[0],
Expand Down
38 changes: 0 additions & 38 deletions spec/datadog/appsec/configuration/settings_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -711,43 +711,5 @@ def patcher
end
end
end

describe 'parse_response_body' do
subject(:enabled) { settings.appsec.parse_response_body }

context 'when DD_API_SECURITY_PARSE_RESPONSE_BODY' do
around do |example|
ClimateControl.modify('DD_API_SECURITY_PARSE_RESPONSE_BODY' => api_security_parse_response_body) do
example.run
end
end

context 'is not defined' do
let(:api_security_parse_response_body) { nil }

it { is_expected.to eq true }
end

context 'is defined' do
let(:api_security_parse_response_body) { 'true' }

it { is_expected.to eq(true) }
end
end
end

context 'parse_response_body=' do
subject(:set_parse_response_body) { settings.appsec.parse_response_body = parse_response_body }

[true, false].each do |value|
context "when given #{value}" do
let(:parse_response_body) { value }

before { set_parse_response_body }

it { expect(settings.appsec.parse_response_body).to eq(value) }
end
end
end
end
end
84 changes: 0 additions & 84 deletions spec/datadog/appsec/contrib/rack/gateway/response_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,88 +42,4 @@
expect(response.response).to be_a(Rack::Response)
end
end

describe '#parsed_body' do
context 'json response' do
let(:content_type) { 'application/json' }

context 'when parse_response_body is disable' do
around do |example|
ClimateControl.modify('DD_API_SECURITY_PARSE_RESPONSE_BODY' => 'false') do
example.run
end
end

it 'returns a nil' do
expect(response.parsed_body).to be_nil
end
end

context 'when parse_response_body is enabled' do
context 'all body parts are strings' do
let(:body) { ['{ "f', 'oo":', ' "ba', 'r" }'] }

it 'returns a hash object' do
expect(response.parsed_body).to eq({ 'foo' => 'bar' })
end
end

context 'not all body parts are strings' do
let(:body_proc) { proc { ' "ba' } }
let(:body) { ['{ "f', 'oo":', body_proc, 'r" }'] }

it 'returns nil' do
expect(response.parsed_body).to be_nil
end
end

context 'fail to parse response body' do
let(:body) { [''] }

it 'returns nil' do
expect(response.parsed_body).to be_nil
end
end
end

context 'non supported response type' do
let(:content_type) { 'text/xml' }

it 'returns nil' do
expect(response.parsed_body).to be_nil
end
end

context 'without content-type header' do
let(:headers) { {} }

it 'returns nil' do
expect(response.parsed_body).to be_nil
end
end

context 'with a body that is not an Array' do
let(:body) { proc { ' "ba' } }

it 'returns nil' do
expect(response.parsed_body).to be_nil
end
end

context 'with a body that inherits from Array' do
let(:my_body_class) do
Class.new(Array) do
end
end

let(:body) do
my_body_class.new
end

it 'returns nil' do
expect(response.parsed_body).to be_nil
end
end
end
end
end
Loading

0 comments on commit 93957a0

Please sign in to comment.