From 9dfb5c26676a7f76fe9eafab5246cfb910d2d6e3 Mon Sep 17 00:00:00 2001
From: Joe Turki <git@joeturki.com>
Date: Thu, 30 Jan 2025 22:30:30 -0600
Subject: [PATCH] Allow for empty extension values

While not spec compliant, some implementations allow for empty extension
values. This aligns with our behavior for empty foundation values.
And makes the parser more forgiving for bad implementations.
---
 candidate_base.go | 22 ++++++++++------------
 candidate_test.go | 47 +++++++++++++++++++++++++++++++++++++----------
 2 files changed, 47 insertions(+), 22 deletions(-)

diff --git a/candidate_base.go b/candidate_base.go
index f2ef422..e7660d6 100644
--- a/candidate_base.go
+++ b/candidate_base.go
@@ -1032,20 +1032,18 @@ func unmarshalCandidateExtensions(raw string) (extensions []CandidateExtension,
 		}
 		i = next
 
-		if i >= len(raw) {
-			return extensions, "", fmt.Errorf(
-				"%w: missing value for %s in %s", errParseExtension, key, raw, //nolint: errorlint // we are wrapping the error
-			)
+		// while not spec-compliant, we allow for empty values, as seen in the wild
+		var value string
+		if i < len(raw) {
+			value, next, err = readCandidateByteString(raw, i)
+			if err != nil {
+				return extensions, "", fmt.Errorf(
+					"%w: failed to read value %v", errParseExtension, err, //nolint: errorlint // we are wrapping the error
+				)
+			}
+			i = next
 		}
 
-		value, next, err := readCandidateByteString(raw, i)
-		if err != nil {
-			return extensions, "", fmt.Errorf(
-				"%w: failed to read value %v", errParseExtension, err, //nolint: errorlint // we are wrapping the error
-			)
-		}
-		i = next
-
 		if key == "tcptype" {
 			rawTCPTypeRaw = value
 
diff --git a/candidate_test.go b/candidate_test.go
index 514a9de..4694cec 100644
--- a/candidate_test.go
+++ b/candidate_test.go
@@ -698,8 +698,20 @@ func TestCandidateExtensionsMarshal(t *testing.T) {
 			"1052353102 1 tcp 2128609279 192.168.0.196 0 typ host",
 		},
 		{
-			[]CandidateExtension{},
-			"1052353102 1 tcp 2128609279 192.168.0.196 0 typ host",
+			[]CandidateExtension{
+				{"tcptype", "active"},
+				{"empty-value-1", ""},
+				{"empty-value-2", ""},
+			},
+			"1052353102 1 tcp 2128609279 192.168.0.196 0 typ host tcptype active empty-value-1  empty-value-2",
+		},
+		{
+			[]CandidateExtension{
+				{"tcptype", "active"},
+				{"empty-value-1", ""},
+				{"empty-value-2", ""},
+			},
+			"1052353102 1 tcp 2128609279 192.168.0.196 0 typ host tcptype active empty-value-1  empty-value-2 ",
 		},
 	}
 
@@ -967,16 +979,10 @@ func TestUnmarshalCandidateExtensions(t *testing.T) {
 			},
 			fail: false,
 		},
-		{
-			name:     "invalid extension string",
-			value:    "invalid",
-			expected: []CandidateExtension{},
-			fail:     true,
-		},
 		{
 			name:     "invalid extension",
-			value:    " a b",
-			expected: []CandidateExtension{{"a", "b"}, {"c", "d"}},
+			value:    " a b d",
+			expected: []CandidateExtension{{"", "a"}, {"b", "d"}},
 			fail:     true,
 		},
 	}
@@ -1357,6 +1363,27 @@ func TestCandidateAddExtension(t *testing.T) {
 
 		require.Error(t, candidate.AddExtension(CandidateExtension{"tcptype", "INVALID"}))
 	})
+
+	t.Run("Add empty extension", func(t *testing.T) {
+		candidate, err := NewCandidateHost(&CandidateHostConfig{
+			Network:    NetworkTypeUDP4.String(),
+			Address:    "fcd9:e3b8:12ce:9fc5:74a5:c6bb:d8b:e08a",
+			Port:       53987,
+			Priority:   500,
+			Foundation: "750",
+		})
+		if err != nil {
+			t.Error(err)
+		}
+
+		require.Error(t, candidate.AddExtension(CandidateExtension{"", ""}))
+
+		require.NoError(t, candidate.AddExtension(CandidateExtension{"a", ""}))
+
+		extensions := candidate.Extensions()
+
+		require.Equal(t, []CandidateExtension{{"a", ""}}, extensions)
+	})
 }
 
 func TestCandidateRemoveExtension(t *testing.T) {