Skip to content

Commit

Permalink
ServerlessHandler: error handling for URI parsing
Browse files Browse the repository at this point in the history
To address issues with the recent serverless enhancements discovered
through local dev testing with the Node.js tool 'serverless':

- Don't append a port value to a URI string that already contains one
- If the URI string construction or parsing fails for any reason, log an
  error and return `nil` so that further processing of the function
  invocation is not impacted

These issues are thought to only be reproducible outside of AWS, but
it's good to be proactive with a bit of extra caution.
  • Loading branch information
fallwith committed Aug 19, 2024
1 parent 4f76606 commit aef39fb
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 1 deletion.
1 change: 1 addition & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1250,6 +1250,7 @@ Style/MethodCallWithArgsParentheses:
- add_development_dependency
- catch
- debug
- error
- exit
- expect
- fail
Expand Down
7 changes: 6 additions & 1 deletion lib/new_relic/agent/serverless_handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -186,13 +186,18 @@ def process_api_gateway_info
def http_uri(info)
return unless info[:host] && info[:path]

url_str = "https://#{info[:host]}:#{info[:port]}#{info[:path]}"
url_str = "https://#{info[:host]}"
url_str += ":#{info[:port]}" unless info[:host].match?(':')
url_str += "#{info[:path]}"

if info[:query_parameters]
qp = info[:query_parameters].map { |k, v| "#{k}=#{v}" }.join('&')
url_str += "?#{qp}"
end

URI.parse(url_str)
rescue StandardError => e
NewRelic::Agent.logger.error "ServerlessHandler failed to parse the source HTTP URI: #{e}"
end

def info_for_api_gateway_v2
Expand Down
41 changes: 41 additions & 0 deletions test/new_relic/agent/serverless_handler_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,47 @@ def test_metadata_for_payload_v2
refute_empty metadata[:agent_language]
end

# when testing locally with (the Node.js tool) serverless or whatnot, the
# base host value may contain ':3000', so make sure the constructed URI
# doesn't end up as 'https://localhost:3000:443'
def test_http_uri_with_existing_port
info = {:host => 'localhost:3000',
:port => 443,
:path => ''}

uri = fresh_handler.send(:http_uri, info)

assert_equal 'https://localhost:3000', uri.to_s
end

def test_http_uri_still_adds_the_port_when_needed
info = {:host => 'flame',
:port => '1138',
:path => '/broiler'}

uri = fresh_handler.send(:http_uri, info)

assert_equal 'https://flame:1138/broiler', uri.to_s
end

def test_http_uri_handles_errors
info = {:host => 'perfecto',
:port => 443,
:path => ''}
logger_mock = Minitest::Mock.new
logger_mock.expect :error, nil, [/failed to parse/]

URI.stub :parse, proc { |_uri| raise 'kaboom' } do
NewRelic::Agent.stub :logger, logger_mock do
uri = fresh_handler.send(:http_uri, info)

assert_nil uri, 'Expected http_uri to rescue and return nil'
end
end

logger_mock.verify
end

private

def handler
Expand Down

0 comments on commit aef39fb

Please sign in to comment.