From d174a4e91f6e9f6df26b76047b29e80947d9a40c Mon Sep 17 00:00:00 2001
From: Jared Harper <129781402+swi-jared@users.noreply.github.com>
Date: Thu, 20 Jun 2024 14:04:34 -0700
Subject: [PATCH] [NH-81807] Use a default port (443) for collector address
 (#102)

---
 internal/config/config.go          | 18 +++++++++++++++++-
 internal/config/config_test.go     |  6 +++---
 internal/config/validators.go      | 12 +++++++++++-
 internal/config/validators_test.go | 12 ++++++++++++
 4 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/internal/config/config.go b/internal/config/config.go
index c9ef73d0..2af19e2a 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -11,6 +11,7 @@
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
+
 // Package config is responsible for loading the configuration from various
 // sources, e.g., environment variables, configuration files and user input.
 // It also accepts dynamic settings from the collector server.
@@ -25,6 +26,7 @@ package config
 
 import (
 	"fmt"
+	"net"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -750,7 +752,21 @@ func (c *Config) loadConfigFile() error {
 func (c *Config) GetCollector() string {
 	c.RLock()
 	defer c.RUnlock()
-	return c.Collector
+	target := c.Collector
+	// We have to detect whether the port is in the target string, however,
+	// accounting for ipv6 means we can't simply search for `:`. This is the
+	// second time we use SplitHostPort, the first is inside the validator. Since
+	// this is only ever called once on startup, I think this is a fair tradeoff.
+	if _, _, err := net.SplitHostPort(target); err != nil {
+		if strings.Contains(err.Error(), "missing port in address") {
+			target = target + ":443"
+		} else {
+			// We should never reach this because we've detected an invalid
+			// value in the validator
+			panic(err)
+		}
+	}
+	return target
 }
 
 // GetServiceKey returns the service key
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
index dbd9bc3d..e8b0f7cf 100644
--- a/internal/config/config_test.go
+++ b/internal/config/config_test.go
@@ -64,7 +64,7 @@ func TestLoadConfig(t *testing.T) {
 	c = NewConfig(
 		WithCollector("hello.world"),
 		WithServiceKey(key2))
-	assert.Equal(t, "hello.world", c.GetCollector())
+	assert.Equal(t, "hello.world:443", c.GetCollector())
 	assert.Equal(t, ToServiceKey(key2), c.GetServiceKey())
 
 	os.Setenv(envSolarWindsAPMServiceKey, key1)
@@ -111,12 +111,12 @@ func TestConfig_HasLocalSamplingConfig(t *testing.T) {
 
 func TestPrintDelta(t *testing.T) {
 	changed := newConfig().reset()
-	changed.Collector = "test.com:443"
+	changed.Collector = "test.com"
 	changed.PrependDomain = true
 	changed.ReporterProperties.EventFlushInterval = 100
 
 	assert.Equal(t,
-		` - Collector (SW_APM_COLLECTOR) = test.com:443 (default: apm.collector.na-01.cloud.solarwinds.com:443)
+		` - Collector (SW_APM_COLLECTOR) = test.com (default: apm.collector.na-01.cloud.solarwinds.com:443)
  - PrependDomain (SW_APM_PREPEND_DOMAIN) = true (default: false)
  - ReporterProperties.EventFlushInterval (SW_APM_EVENTS_FLUSH_INTERVAL) = 100 (default: 2)`,
 		getDelta(newConfig().reset(), changed, "").sanitize().String())
diff --git a/internal/config/validators.go b/internal/config/validators.go
index 3d3d8218..bcdab413 100644
--- a/internal/config/validators.go
+++ b/internal/config/validators.go
@@ -11,10 +11,13 @@
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
+
 package config
 
 import (
 	"fmt"
+	"github.com/solarwinds/apm-go/internal/log"
+	"net"
 	"regexp"
 	"strconv"
 	"strings"
@@ -86,7 +89,14 @@ func ToServiceKey(s string) string {
 
 // IsValidHost verifies if the host is in a valid format
 func IsValidHost(host string) bool {
-	// TODO
+	if _, _, err := net.SplitHostPort(host); err != nil {
+		if strings.Contains(err.Error(), "missing port in address") {
+			// OK!
+		} else {
+			log.Warningf("Invalid host format: %s", host)
+			return false
+		}
+	}
 	return host != ""
 }
 
diff --git a/internal/config/validators_test.go b/internal/config/validators_test.go
index 74973e44..a73b3e15 100644
--- a/internal/config/validators_test.go
+++ b/internal/config/validators_test.go
@@ -15,6 +15,7 @@ package config
 
 import (
 	"fmt"
+	"github.com/stretchr/testify/require"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -111,3 +112,14 @@ func TestToServiceKey(t *testing.T) {
 		assert.Equal(t, tc.after, ToServiceKey(tc.before), fmt.Sprintf("Case #%d", idx))
 	}
 }
+
+func TestIsValidHost(t *testing.T) {
+	require.True(t, IsValidHost("localhost"))
+	require.True(t, IsValidHost("localhost:321"))
+	require.True(t, IsValidHost("[2001:db8::ff00:42:8329]"))
+	require.True(t, IsValidHost("[2001:db8::ff00:42:8329]:1234"))
+	require.False(t, IsValidHost(""))
+	require.False(t, IsValidHost("localhost:321:321"))
+	require.False(t, IsValidHost("2001:db8::ff00:42:8329"))
+	require.False(t, IsValidHost("2001:db8::ff00:42:8329:1234"))
+}