Skip to content

Commit

Permalink
fix: http matcher only signaling false if it is sure http is impossible
Browse files Browse the repository at this point in the history
  • Loading branch information
ydylla committed Jun 30, 2024
1 parent e170bbb commit 3e5b068
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 6 deletions.
15 changes: 10 additions & 5 deletions modules/l4http/httpmatcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,9 @@ func (m MatchHTTP) Match(cx *layer4.Connection) (bool, error) {
var err error

data := cx.MatchingBytes()
if !m.isHttp(data) {
return false, nil
match, err := m.isHttp(data)
if !match {
return match, err
}

// use bufio reader which exactly matches the size of prefetched data,
Expand Down Expand Up @@ -122,11 +123,15 @@ func (m MatchHTTP) Match(cx *layer4.Connection) (bool, error) {
return m.matcherSets.AnyMatch(req), nil
}

func (m MatchHTTP) isHttp(data []byte) bool {
func (m MatchHTTP) isHttp(data []byte) (bool, error) {
// try to find the end of a http request line, for example " HTTP/1.1\r\n"
i := bytes.IndexByte(data, 0x0a) // find first new line
if i == -1 {
// no line break found yet so we signal we need more data
return false, layer4.ErrConsumedAllPrefetchedBytes
}
if i < 10 {
return false
return false, nil
}
// assume only \n line ending
start := i - 9 // position of space in front of HTTP
Expand All @@ -136,7 +141,7 @@ func (m MatchHTTP) isHttp(data []byte) bool {
start -= 1
end -= 1
}
return bytes.Compare(data[start:end], []byte(" HTTP/")) == 0
return bytes.Compare(data[start:end], []byte(" HTTP/")) == 0, nil
}

// Parses information from a http2 request with prior knowledge (RFC 7540 Section 3.4)
Expand Down
19 changes: 18 additions & 1 deletion modules/l4http/httpmatcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,48 +252,65 @@ func TestMatchHTTP_isHttp(t *testing.T) {
name string
data []byte
shouldMatch bool
expectedErr error
}{
{
name: "http/1.1-only-lf",
data: []byte("GET /foo/bar?aaa=bbb HTTP/1.1\nHost: localhost:10443\n\n"),
shouldMatch: true,
expectedErr: nil,
},
{
name: "http/1.1-cr-lf",
data: []byte("GET /foo/bar?aaa=bbb HTTP/1.1\r\nHost: localhost:10443\r\n\r\n"),
shouldMatch: true,
expectedErr: nil,
},
{
name: "http/1.0-cr-lf",
data: []byte("GET /foo/bar?aaa=bbb HTTP/1.0\r\nHost: localhost:10443\r\n\r\n"),
shouldMatch: true,
expectedErr: nil,
},
{
name: "http/2.0-cr-lf",
data: []byte("PRI * HTTP/2.0\r\n\r\n"),
shouldMatch: true,
expectedErr: nil,
},
{
name: "dummy-short",
data: []byte("dum\n"),
shouldMatch: false,
expectedErr: nil,
},
{
name: "dummy-long",
data: []byte("dummydummydummy\n"),
shouldMatch: false,
expectedErr: nil,
},
{
name: "http/1.1-without-space-in-front",
data: []byte("HTTP/1.1\n"),
shouldMatch: false,
expectedErr: nil,
},
{
name: "get-with-incomplete-request-line",
data: []byte("GET /with/incomplete/request-line"),
shouldMatch: false,
expectedErr: layer4.ErrConsumedAllPrefetchedBytes,
},
} {
t.Run(tc.name, func(t *testing.T) {
matched := MatchHTTP{}.isHttp(tc.data)
matched, err := MatchHTTP{}.isHttp(tc.data)
if matched != tc.shouldMatch {
t.Fatalf("test %v | matched: %v != shouldMatch: %v", tc.name, matched, tc.shouldMatch)
}
if err != tc.expectedErr {
t.Fatalf("test %v | err: %v != expectedErr: %v", tc.name, err, tc.expectedErr)
}
})
}
}

0 comments on commit 3e5b068

Please sign in to comment.