Skip to content

Commit

Permalink
Allow for empty extension values
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
JoeTurki committed Jan 31, 2025
1 parent 47dad55 commit 9dfb5c2
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 22 deletions.
22 changes: 10 additions & 12 deletions candidate_base.go
Original file line number Diff line number Diff line change
Expand Up @@ -1032,19 +1032,17 @@ 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
)
}

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
)
// 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
}
i = next

if key == "tcptype" {
rawTCPTypeRaw = value
Expand Down
47 changes: 37 additions & 10 deletions candidate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 ",
},
}

Expand Down Expand Up @@ -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,
},
}
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 9dfb5c2

Please sign in to comment.