From b2fd70c510ef53aef8323c72d027a166329d24da Mon Sep 17 00:00:00 2001 From: Daniel Taylor Date: Thu, 7 Nov 2024 13:20:20 -0800 Subject: [PATCH 1/2] fix: make content negotiation more resilient to bad input --- negotiation/negotiation.go | 35 +++++++++++++++++++++------------ negotiation/negotiation_test.go | 5 +++++ 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/negotiation/negotiation.go b/negotiation/negotiation.go index ae6358c7..ca8db4ab 100644 --- a/negotiation/negotiation.go +++ b/negotiation/negotiation.go @@ -59,33 +59,38 @@ func SelectQValueFast(header string, allowed []string) string { bestQ := 0.0 name := "" - start := -1 + start := 0 end := 0 for pos, char := range header { // Format is like "a; q=0.5, b;q=1.0,c; q=0.3" if char == ';' { name = header[start : end+1] - start = -1 + start = pos + 1 + end = start continue } if char == ',' || pos == len(header)-1 { q := 1.0 - if pos == len(header)-1 { - // This is the end, so it must be the name. - // Example: "application/yaml" - name = header[start : pos+1] - } else if name == "" { - // No name yet means we did not encounter a `;`. + if char != ',' && char != ' ' && char != '\t' { + // Update the end if it's not a comma or whitespace (i.e. end of string). + end = pos + } + if name == "" { + // No name yet means we did not encounter a `;`. Either this is a `,` + // or the end of the string so whatever we have is the name. // Example: "a, b, c" - name = header[start:pos] + name = header[start : end+1] } else { - if parsed, _ := strconv.ParseFloat(header[start+2:end+1], 64); parsed > 0 { - q = parsed + if len(header) > end+1 { + if parsed, _ := strconv.ParseFloat(header[start+2:end+1], 64); parsed > 0 { + q = parsed + } } } - start = -1 + start = pos + 1 + end = start found := false for _, n := range allowed { @@ -97,6 +102,7 @@ func SelectQValueFast(header string, allowed []string) string { if !found { // Skip formats we don't support. + name = "" continue } @@ -104,12 +110,15 @@ func SelectQValueFast(header string, allowed []string) string { bestQ = q best = name } + name = "" continue } if char != ' ' && char != '\t' { + // Only advance end if it's not whitespace. end = pos - if start == -1 { + if header[start] == ' ' || header[start] == '\t' { + // Trim leading whitespace. start = pos } } diff --git a/negotiation/negotiation_test.go b/negotiation/negotiation_test.go index 8f3696b4..ff3146bc 100644 --- a/negotiation/negotiation_test.go +++ b/negotiation/negotiation_test.go @@ -50,6 +50,11 @@ func TestNoMatchFast(t *testing.T) { assert.Equal(t, "", SelectQValueFast("a; q=1.0, b;q=1.0,c; q=0.3", []string{"d", "e"})) } +func TestMalformedFast(t *testing.T) { + assert.Equal(t, "", SelectQValueFast("a;,", []string{"d", "e"})) + assert.Equal(t, "a", SelectQValueFast(",a ", []string{"a", "b"})) +} + var BenchResult string func BenchmarkMatch(b *testing.B) { From 58ddf7d2ca47adc10766705202946c6c0994f735 Mon Sep 17 00:00:00 2001 From: Daniel Taylor Date: Thu, 7 Nov 2024 13:49:46 -0800 Subject: [PATCH 2/2] test: additional tests suggested by ai bot --- negotiation/negotiation_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/negotiation/negotiation_test.go b/negotiation/negotiation_test.go index ff3146bc..781952e3 100644 --- a/negotiation/negotiation_test.go +++ b/negotiation/negotiation_test.go @@ -53,6 +53,9 @@ func TestNoMatchFast(t *testing.T) { func TestMalformedFast(t *testing.T) { assert.Equal(t, "", SelectQValueFast("a;,", []string{"d", "e"})) assert.Equal(t, "a", SelectQValueFast(",a ", []string{"a", "b"})) + assert.Equal(t, "", SelectQValueFast("a;;", []string{"a", "b"})) + assert.Equal(t, "", SelectQValueFast(";,", []string{"a", "b"})) + assert.Equal(t, "a", SelectQValueFast("a;q=invalid", []string{"a", "b"})) } var BenchResult string