Skip to content

Commit

Permalink
Fix utf8 errors in headers and cookies and invalid parameter errors (#…
Browse files Browse the repository at this point in the history
…2902)

**Why**: So that these errors will render a 400 instead of a 500.
  • Loading branch information
jmhooper authored Apr 16, 2019
1 parent 4f7a38f commit 0bf8d63
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 34 deletions.
38 changes: 11 additions & 27 deletions lib/rack_request_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ def initialize(request)
end

def values_to_check
param_values + ahoy_headers + ahoy_cookies + [host_header]
param_values + header_values + cookie_values
end

private
Expand All @@ -15,35 +15,19 @@ def param_values
all_values(request.params)
end

def all_values(hash)
hash.values.flat_map { |value| value.is_a?(Hash) ? all_values(value) : [value] }
end

def ahoy_headers
[ahoy_visit_header, ahoy_visitor_header]
end

def ahoy_visit_header
request.fetch_header('HTTP_AHOY_VISIT') { '' }
end

def ahoy_visitor_header
request.fetch_header('HTTP_AHOY_VISITOR') { '' }
end

def ahoy_cookies
[ahoy_visit_cookie, ahoy_visitor_cookie]
def header_values
header_values = []
request.env.each do |key, value|
header_values.push(value) if key.start_with? 'HTTP_'
end
header_values
end

def ahoy_visit_cookie
request.cookies['ahoy_visit']
def cookie_values
request.cookies.values
end

def ahoy_visitor_cookie
request.cookies['ahoy_visitor']
end

def host_header
request.fetch_header('HTTP_HOST') { '' }
def all_values(hash)
hash.values.flat_map { |value| value.is_a?(Hash) ? all_values(value) : [value] }
end
end
31 changes: 25 additions & 6 deletions lib/utf8_sanitizer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,22 @@ def initialize(app)
@app = app
end

# :reek:DuplicateMethodCall
# :reek:TooManyStatements
def call(env)
request = Rack::Request.new(env)
parser = RackRequestParser.new(request)
parser = RackRequestParser.new(Rack::Request.new(env))
values_to_check = parser.values_to_check

if invalid_strings(values_to_check)
Rails.logger.info(event_attributes(env, parser.request))
Rails.logger.info(invalid_utf8_event(env, parser.request))

return [400, {}, ['Bad request']]
end

@app.call(env)
rescue Rack::QueryParser::InvalidParameterError => err
Rails.logger.info(invalid_parameter_event(err))
[400, {}, ['Bad request']]
end

private
Expand All @@ -34,14 +38,14 @@ def invalid_string?(string)
!string.force_encoding('UTF-8').valid_encoding?
end

def event_attributes(env, request)
def invalid_utf8_event(env, request)
{
event: 'Invalid UTF-8 encoding',
user_uuid: env['warden']&.user&.uuid || AnonymousUser.new.uuid,
ip: remote_ip(request),
user_agent: request.user_agent,
user_agent: sanitized_user_agent(request),
timestamp: Time.zone.now,
hostname: request.host,
hostname: sanitized_hostname(request),
visitor_id: sanitized_visitor_id(request),
content_type: env['CONTENT_TYPE'],
}.to_json
Expand All @@ -51,8 +55,23 @@ def remote_ip(request)
@remote_ip ||= (request.env['action_dispatch.remote_ip'] || request.ip).to_s
end

def sanitized_hostname(request)
Utf8Cleaner.new(request.host).remove_invalid_utf8_bytes
end

def sanitized_user_agent(request)
Utf8Cleaner.new(request.user_agent).remove_invalid_utf8_bytes
end

def sanitized_visitor_id(request)
string_to_clean = request.cookies['ahoy_visitor']
Utf8Cleaner.new(string_to_clean).remove_invalid_utf8_bytes
end

def invalid_parameter_event(error)
{
event: 'Invalid parameter error',
message: error.message,
}
end
end
3 changes: 2 additions & 1 deletion spec/requests/openid_connect_cors_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,8 @@
describe 'domain name as the origin' do
it 'does not load any ServiceProvider objects' do
stub_const 'ServiceProvider', double
get openid_connect_configuration_path, headers: { 'HTTP_ORIGIN' => Figaro.env.domain_name }
get openid_connect_configuration_path,
headers: { 'HTTP_ORIGIN' => Figaro.env.domain_name.dup }

aggregate_failures do
expect(response).to be_ok
Expand Down

0 comments on commit 0bf8d63

Please sign in to comment.