From aef39fbd419658c939a2400a5fd11ffe5bd18685 Mon Sep 17 00:00:00 2001
From: fallwith <fallwith@gmail.com>
Date: Mon, 19 Aug 2024 15:06:56 -0700
Subject: [PATCH 1/2] ServerlessHandler: error handling for URI parsing

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.
---
 .rubocop.yml                                  |  1 +
 lib/new_relic/agent/serverless_handler.rb     |  7 +++-
 .../agent/serverless_handler_test.rb          | 41 +++++++++++++++++++
 3 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/.rubocop.yml b/.rubocop.yml
index 9d1c9ea0eb..3cc6cc5d2d 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -1250,6 +1250,7 @@ Style/MethodCallWithArgsParentheses:
     - add_development_dependency
     - catch
     - debug
+    - error
     - exit
     - expect
     - fail
diff --git a/lib/new_relic/agent/serverless_handler.rb b/lib/new_relic/agent/serverless_handler.rb
index b31f9eb696..cae9d3d3cd 100644
--- a/lib/new_relic/agent/serverless_handler.rb
+++ b/lib/new_relic/agent/serverless_handler.rb
@@ -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
diff --git a/test/new_relic/agent/serverless_handler_test.rb b/test/new_relic/agent/serverless_handler_test.rb
index 0c051b24ca..7bb1c54b01 100644
--- a/test/new_relic/agent/serverless_handler_test.rb
+++ b/test/new_relic/agent/serverless_handler_test.rb
@@ -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

From c869cd920660ec65c5b77ab641c34f7ff7cbefc2 Mon Sep 17 00:00:00 2001
From: fallwith <fallwith@gmail.com>
Date: Mon, 19 Aug 2024 15:58:58 -0700
Subject: [PATCH 2/2] serverless handler tested: comment disambiguation

simply say "without AWS" to refer to local testing
---
 test/new_relic/agent/serverless_handler_test.rb | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/new_relic/agent/serverless_handler_test.rb b/test/new_relic/agent/serverless_handler_test.rb
index 7bb1c54b01..baa516065f 100644
--- a/test/new_relic/agent/serverless_handler_test.rb
+++ b/test/new_relic/agent/serverless_handler_test.rb
@@ -423,9 +423,9 @@ 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'
+      # when testing locally without AWS 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,