Skip to content

Commit

Permalink
handle multiple ACRH field lines (fixes #7)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jub0bs
jub0bs committed Aug 28, 2024
1 parent 80ed9ae commit a72eb19
Show file tree
Hide file tree
Showing 4 changed files with 198 additions and 132 deletions.
7 changes: 5 additions & 2 deletions doc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,11 @@ in the chain, and the ultimate handler. Follow the rules listed below:
Regarding the value of [list-based field] [Access-Control-Request-Headers]
specifically, intermediaries [MAY] add some [optional whitespace] around
the value's elements or add (inadvertently, perhaps) some empty elements
to that value, but they [SHOULD] do so within reason. For performance
(and at the cost of some interoperability),
to that value, but they [SHOULD] do so within reason;
moreover, intermediaries [MAY] split the value of that field across
multiple field lines of that name, but they [SHOULD NOT] add too many
empty field lines of that name.
For performance (and at the cost of some interoperability),
this library's middleware are indeed stricter in their handling of
this specific list-based field than required by [RFC 9110].
- Intermediaries [SHOULD NOT] alter or augment the [CORS response headers]
Expand Down
98 changes: 56 additions & 42 deletions internal/headers/sortedset.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,30 +49,41 @@ func (set SortedSet) String() string {
return strings.Join(elems, ",")
}

// Accepts reports whether s is a [list-based field value] whose elements are
// Accepts reports whether values is a sequence of [list-based field values]
// whose elements are
// - all members of set,
// - sorted in lexicographical order,
// - unique.
//
// This function's parameter is a slice of strings rather than just a string
// because, although [the Fetch standard] requires browsers to include at most
// one ACRH field line in CORS-preflight requests, some intermediaries may well
// (and [some reportedly do]) split it into multiple ACRH field lines.
// Note that, because [RFC 9110] (section 5.3) forbids intermediaries from
// changing the order of field lines of the same name, we can expect the
// overall sequence of elements to still be sorted in lexicographical order.
//
// Although [the Fetch standard] requires browsers to omit any whitespace
// in the value of the ACRH field, some intermediaries may well alter this
// list-based field's value by sprinkling optional whitespace (OWS) around
// the value's elements.
// [RFC 9110] does require recipients to tolerate arbitrary long OWS around
// elements of a list-based field value, but adherence to this requirement
// leads to non-negligible performance degradation in CORS middleware
// in the face of adversarial (spoofed) CORS-preflight requests.
// [RFC 9110] (section 5.6.1.2) requires recipients to tolerate arbitrary long
// OWS around elements of a list-based field value,
// but adherence to this requirement leads to non-negligible performance
// degradation in CORS middleware in the face of adversarial (spoofed)
// CORS-preflight requests.
// Therefore, this function only tolerates a small number (1) of OWS bytes
// before and/or after each element. This deviation from RFC 9110 is expected
// to strike a good balance between interoperability and performance.
//
// Moreover, this function tolerates a small number (16) of empty list elements,
// in accordance with [RFC 9110]'s recommendation.
// in accordance with [RFC 9110]'s recommendation (section 5.6.1.2).
//
// [RFC 9110]: https://httpwg.org/specs/rfc9110.html#abnf.extension.recipient
// [list-based field value]: https://httpwg.org/specs/rfc9110.html#abnf.extension
// [the Fetch standard]: https://fetch.spec.whatwg.org/#cors-preflight-fetch-0
func (set SortedSet) Accepts(s string) bool {
// [RFC 9110]: https://httpwg.org/specs/rfc9110.html
// [list-based field values]: https://httpwg.org/specs/rfc9110.html#abnf.extension
// [some reportedly do]: https://github.com/rs/cors/issues/184
// [the Fetch standard]: https://fetch.spec.whatwg.org
func (set SortedSet) Accepts(values []string) bool {
var ( // effectively constant
maxLen = MaxOWSBytes + set.maxLen + MaxOWSBytes + 1 // +1 for comma
)
Expand All @@ -83,44 +94,47 @@ func (set SortedSet) Accepts(s string) bool {
emptyElements int
ok bool
)
for {
// As a defense against maliciously long names in csv,
// we process only a small number of csv's leading bytes per iteration.
name, s, commaFound = cutAtComma(s, maxLen)
name, ok = TrimOWS(name, MaxOWSBytes)
if !ok {
return false
}
if name == "" {
// RFC 9110 requires recipients to tolerate
// "a reasonable number of empty list elements"; see
// https://httpwg.org/specs/rfc9110.html#abnf.extension.recipient.
emptyElements++
if emptyElements > MaxEmptyElements {
for _, s := range values {
for {
// As a defense against maliciously long names in csv,
// we process only a small number of csv's leading bytes per iteration.
name, s, commaFound = cutAtComma(s, maxLen)
name, ok = TrimOWS(name, MaxOWSBytes)
if !ok {
return false
}
if name == "" {
// RFC 9110 requires recipients to tolerate
// "a reasonable number of empty list elements"; see
// https://httpwg.org/specs/rfc9110.html#abnf.extension.recipient.
emptyElements++
if emptyElements > MaxEmptyElements {
return false
}
if !commaFound { // We have now exhausted the names in csv.
break
}
continue
}
pos, ok := set.m[name]
if !ok {
return false
}
// The names in csv are expected to be sorted in lexicographical order
// and to each appear at most once.
// Therefore, the positions (in set) of the names that
// appear in csv should form a strictly increasing sequence.
// If that's not actually the case, bail out.
if pos <= posOfLastNameSeen {
return false
}
posOfLastNameSeen = pos
if !commaFound { // We have now exhausted the names in csv.
return true
break
}
continue
}
pos, ok := set.m[name]
if !ok {
return false
}
// The names in csv are expected to be sorted in lexicographical order
// and to each appear at most once.
// Therefore, the positions (in set) of the names that
// appear in csv should form a strictly increasing sequence.
// If that's not actually the case, bail out.
if pos <= posOfLastNameSeen {
return false
}
posOfLastNameSeen = pos
if !commaFound { // We have now exhausted the names in csv.
return true
}
}
return true
}

const (
Expand Down
207 changes: 122 additions & 85 deletions internal/headers/sortedset_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,131 +16,168 @@ func TestSortedSet(t *testing.T) {
size int
combined string
slice []string
accepted []string
rejected []string
accepted [][]string
rejected [][]string
}{
{
desc: "empty set",
size: 0,
combined: "",
accepted: []string{
accepted: [][]string{
// some empty elements, possibly with OWS
"",
",",
"\t, , ",
{""},
{","},
{"\t, , "},
// multiple field lines, some empty elements
make([]string, headers.MaxEmptyElements),
},
rejected: []string{
"x-bar",
"x-bar,x-foo",
rejected: [][]string{
{"x-bar"},
{"x-bar,x-foo"},
// too many empty elements
{strings.Repeat(",", headers.MaxEmptyElements+1)},
// multiple field lines, too many empty elements
make([]string, headers.MaxEmptyElements+1),
},
}, {
desc: "singleton set",
elems: []string{"x-foo"},
size: 1,
combined: "x-foo",
slice: []string{"X-Foo"},
accepted: []string{
"x-foo",
accepted: [][]string{
{"x-foo"},
// some empty elements, possibly with OWS
"",
",",
"\t, , ",
"\tx-foo ,",
" x-foo\t,",
strings.Repeat(",", headers.MaxEmptyElements) + "x-foo",
{""},
{","},
{"\t, , "},
{"\tx-foo ,"},
{" x-foo\t,"},
{strings.Repeat(",", headers.MaxEmptyElements) + "x-foo"},
// multiple field lines, some empty elements
append(make([]string, headers.MaxEmptyElements), "x-foo"),
make([]string, headers.MaxEmptyElements),
},
rejected: []string{
"x-bar",
"x-bar,x-foo",
rejected: [][]string{
{"x-bar"},
{"x-bar,x-foo"},
// too much OWS
"x-foo ",
" x-foo ",
" x-foo ",
"x-foo\t\t",
"\tx-foo\t\t",
"\t\tx-foo\t\t",
{"x-foo "},
{" x-foo "},
{" x-foo "},
{"x-foo\t\t"},
{"\tx-foo\t\t"},
{"\t\tx-foo\t\t"},
// too many empty elements
strings.Repeat(",", headers.MaxEmptyElements+1) + "x-foo",
{strings.Repeat(",", headers.MaxEmptyElements+1) + "x-foo"},
// multiple field lines, too many empty elements
append(make([]string, headers.MaxEmptyElements+1), "x-foo"),
make([]string, headers.MaxEmptyElements+1),
},
}, {
desc: "no dupes",
elems: []string{"x-foo", "x-bar", "x-baz"},
size: 3,
combined: "x-bar,x-baz,x-foo",
slice: []string{"X-Bar", "X-Baz", "X-Foo"},
accepted: []string{
"x-bar",
"x-baz",
"x-foo",
"x-bar,x-baz",
"x-bar,x-foo",
"x-baz,x-foo",
"x-bar,x-baz,x-foo",
accepted: [][]string{
{"x-bar"},
{"x-baz"},
{"x-foo"},
{"x-bar,x-baz"},
{"x-bar,x-foo"},
{"x-baz,x-foo"},
{"x-bar,x-baz,x-foo"},
// some empty elements, possibly with OWS
"",
",",
"\t, , ",
"\tx-bar ,",
" x-baz\t,",
"x-foo,",
"\tx-bar ,\tx-baz ,",
" x-bar\t, x-foo\t,",
"x-baz,x-foo,",
" x-bar , x-baz , x-foo ,",
"x-bar" + strings.Repeat(",", headers.MaxEmptyElements+1) + "x-foo",
{""},
{","},
{"\t, , "},
{"\tx-bar ,"},
{" x-baz\t,"},
{"x-foo,"},
{"\tx-bar ,\tx-baz ,"},
{" x-bar\t, x-foo\t,"},
{"x-baz,x-foo,"},
{" x-bar , x-baz , x-foo ,"},
{"x-bar" + strings.Repeat(",", headers.MaxEmptyElements+1) + "x-foo"},
// multiple field lines
{"x-bar", "x-foo"},
{"x-bar", "x-baz,x-foo"},
// multiple field lines, some empty elements
append(make([]string, headers.MaxEmptyElements), "x-bar", "x-foo"),
make([]string, headers.MaxEmptyElements),
},
rejected: []string{
"x-qux",
"x-bar,x-baz,x-baz",
"x-qux,x-baz",
"x-qux,x-foo",
"x-quxbaz,x-foo",
rejected: [][]string{
{"x-qux"},
{"x-bar,x-baz,x-baz"},
{"x-qux,x-baz"},
{"x-qux,x-foo"},
{"x-quxbaz,x-foo"},
// too much OWS
"x-bar ",
" x-baz ",
" x-foo ",
"x-bar\t\t,x-baz",
"x-bar,\tx-foo\t\t",
"\t\tx-baz,x-foo\t\t",
" x-bar\t,\tx-baz\t ,x-foo",
{"x-bar "},
{" x-baz "},
{" x-foo "},
{"x-bar\t\t,x-baz"},
{"x-bar,\tx-foo\t\t"},
{"\t\tx-baz,x-foo\t\t"},
{" x-bar\t,\tx-baz\t ,x-foo"},
// too many empty elements
"x-bar" + strings.Repeat(",", headers.MaxEmptyElements+2) + "x-foo",
{"x-bar" + strings.Repeat(",", headers.MaxEmptyElements+2) + "x-foo"},
// multiple field lines, elements in the wrong order
{"x-foo", "x-bar"},
// multiple field lines, too many empty elements
append(make([]string, headers.MaxEmptyElements+1), "x-bar", "x-foo"),
make([]string, headers.MaxEmptyElements+1),
},
}, {
desc: "some dupes",
elems: []string{"x-foo", "x-bar", "x-foo"},
size: 2,
combined: "x-bar,x-foo",
slice: []string{"X-Bar", "X-Foo"},
accepted: []string{
"x-bar",
"x-foo",
"x-bar,x-foo",
accepted: [][]string{
{"x-bar"},
{"x-foo"},
{"x-bar,x-foo"},
// some empty elements, possibly with OWS
"",
",",
"\t, , ",
"\tx-bar ,",
" x-foo\t,",
"x-foo,",
"\tx-bar ,\tx-foo ,",
" x-bar\t, x-foo\t,",
"x-bar,x-foo,",
" x-bar , x-foo ,",
"x-bar" + strings.Repeat(",", headers.MaxEmptyElements+1) + "x-foo",
{""},
{","},
{"\t, , "},
{"\tx-bar ,"},
{" x-foo\t,"},
{"x-foo,"},
{"\tx-bar ,\tx-foo ,"},
{" x-bar\t, x-foo\t,"},
{"x-bar,x-foo,"},
{" x-bar , x-foo ,"},
{"x-bar" + strings.Repeat(",", headers.MaxEmptyElements+1) + "x-foo"},
// multiple field lines
{"x-bar", "x-foo"},
// multiple field lines, some empty elements
append(make([]string, headers.MaxEmptyElements), "x-bar", "x-foo"),
make([]string, headers.MaxEmptyElements),
},
rejected: []string{
"x-qux",
"x-qux,x-bar",
"x-qux,x-foo",
"x-qux,x-baz,x-foo",
rejected: [][]string{
{"x-qux"},
{"x-qux,x-bar"},
{"x-qux,x-foo"},
{"x-qux,x-baz,x-foo"},
// too much OWS
"x-qux ",
"x-qux,\t\tx-bar",
"x-qux,x-foo\t\t",
"\tx-qux , x-baz\t\t,x-foo",
{"x-qux "},
{"x-qux,\t\tx-bar"},
{"x-qux,x-foo\t\t"},
{"\tx-qux , x-baz\t\t,x-foo"},
// too many empty elements
"x-bar" + strings.Repeat(",", headers.MaxEmptyElements+2) + "x-foo",
{"x-bar" + strings.Repeat(",", headers.MaxEmptyElements+2) + "x-foo"},
// multiple field lines, elements in the wrong order
{"x-foo", "x-bar"},
// multiple field lines, too much whitespace
{"x-qux", "\t\tx-bar"},
{"x-qux", "x-foo\t\t"},
{"\tx-qux ", " x-baz\t\t,x-foo"},
// multiple field lines, too many empty elements
append(make([]string, headers.MaxEmptyElements+1), "x-bar", "x-foo"),
make([]string, headers.MaxEmptyElements+1),
},
},
}
Expand Down
Loading

0 comments on commit a72eb19

Please sign in to comment.