From ba1c3772d012460e7d9ce1b1c89afd3b2e549e7c Mon Sep 17 00:00:00 2001 From: Adrian Serrano Date: Thu, 22 Mar 2018 16:34:17 +0100 Subject: [PATCH 1/2] http: support broken status line User reports some HTTP servers may respond with a broken status line that's missing a space between the status code and the optional status phrase. HTTP clients tested already support this behavior. This patch adjusts the http parser so that this deviation from the standard is accepted. Fixes #6176 --- packetbeat/protos/http/http_parser.go | 6 ++- packetbeat/protos/http/http_test.go | 61 +++++++-------------------- 2 files changed, 20 insertions(+), 47 deletions(-) diff --git a/packetbeat/protos/http/http_parser.go b/packetbeat/protos/http/http_parser.go index 1526c47e985a..1f8938110c72 100644 --- a/packetbeat/protos/http/http_parser.go +++ b/packetbeat/protos/http/http_parser.go @@ -212,15 +212,17 @@ func parseResponseStatus(s []byte) (uint16, []byte, error) { debugf("parseResponseStatus: %s", s) } + var phrase []byte p := bytes.IndexByte(s, ' ') if p == -1 { - return 0, nil, errors.New("Not able to identify status code") + p = len(s) + } else { + phrase = s[p+1:] } statusCode, err := parseInt(s[0:p]) if err != nil { return 0, nil, fmt.Errorf("Unable to parse status code from [%s]", s) } - phrase := s[p+1:] return uint16(statusCode), phrase, nil } diff --git a/packetbeat/protos/http/http_test.go b/packetbeat/protos/http/http_test.go index 3aee21531b85..980230a62c0e 100644 --- a/packetbeat/protos/http/http_test.go +++ b/packetbeat/protos/http/http_test.go @@ -4,6 +4,7 @@ package http import ( "bytes" + "fmt" "net" "regexp" "strings" @@ -394,51 +395,21 @@ func TestHttpParser_Response_HTTP_10_without_content_length(t *testing.T) { } func TestHttpParser_Response_without_phrase(t *testing.T) { - data := "HTTP/1.1 200 \r\n" + - "Date: Tue, 14 Aug 2012 22:31:45 GMT\r\n" + - "Expires: -1\r\n" + - "Cache-Control: private, max-age=0\r\n" + - "Content-Type: text/html; charset=UTF-8\r\n" + - "Content-Encoding: gzip\r\n" + - "Server: gws\r\n" + - "Content-Length: 0\r\n" + - "X-XSS-Protection: 1; mode=block\r\n" + - "X-Frame-Options: SAMEORIGIN\r\n" + - "\r\n" - r, ok, complete := testParse(nil, data) - - assert.True(t, ok) - assert.True(t, complete) - assert.False(t, r.isRequest) - assert.Equal(t, 200, int(r.statusCode)) - assert.Equal(t, "", string(r.statusPhrase)) - assert.Equal(t, 0, r.contentLength) - - broken := "HTTP/1.1 301 \r\n" + - "Date: Sun, 29 Sep 2013 16:53:59 GMT\r\n" + - "Server: Apache\r\n" + - "Location: http://www.hotnews.ro/\r\n" + - "Vary: Accept-Encoding\r\n" + - "Content-Length: 290\r\n" + - "Connection: close\r\n" + - "Content-Type: text/html; charset=iso-8859-1\r\n" + - "\r\n" + - "\r\n" + - "\r\n" + - "301 Moved Permanently\r\n" + - "\r\n" + - "

Moved Permanently

\r\n" + - "

The document has moved here.

\r\n" + - "
\r\n" + - "
Apache Server at hotnews.ro Port 80
\r\n" + - "" - r, ok, complete = testParse(nil, broken) - - assert.True(t, ok) - assert.True(t, complete) - assert.Equal(t, 290, r.contentLength) - assert.Equal(t, "", string(r.statusPhrase)) - assert.Equal(t, 301, int(r.statusCode)) + for idx, testCase := range []struct { + ok, complete bool + code int + request string + }{ + {true, true, 200, "HTTP/1.1 200 \r\nContent-Length: 0\r\n\r\n"}, + {true, true, 301, "HTTP/1.1 301\r\nContent-Length: 0\r\n\r\n"}, + } { + msg := fmt.Sprintf("failed test case[%d]: \"%s\"", idx, testCase.request) + r, ok, complete := testParse(nil, testCase.request) + assert.Equal(t, testCase.ok, ok, msg) + assert.Equal(t, testCase.complete, complete, msg) + assert.Equal(t, testCase.code, int(r.statusCode), msg) + assert.Equal(t, "", string(r.statusPhrase), msg) + } } func TestHttpParser_splitResponse_midBody(t *testing.T) { From 4173efd54a8fe8d58e62a5551afd6de0e8e09546 Mon Sep 17 00:00:00 2001 From: Adrian Serrano Date: Fri, 23 Mar 2018 16:44:32 +0100 Subject: [PATCH 2/2] Updated CHANGELOG for #6631 Added new feature to packetbeat: - Support broken status line --- CHANGELOG.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index be6a8289be48..1d42af320517 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -263,6 +263,7 @@ https://github.com/elastic/beats/compare/v6.0.0-beta2...master[Check the HEAD di - Configure good defaults for `add_kubernetes_metadata`. {pull}5707[5707] - Add support for condition on bool type {issue}5659[5659] {pull}5954[5954] - HTTP parses successfully on empty status phrase. {issue}6176[6176] +- HTTP parser supports broken status line. {pull}6631[6631] *Winlogbeat*