Packetbeat HTTP parsing fails on empty status phrase.

- The function [parseResponseStatus](https://github.com/elastic/beats/blob/81f55f89fd1db748d0063e4e3c0564913e23ee46/packetbeat/protos/http/http_parser.go#L210) now returns the phrase, even if empty, instead of an error. - Added tests cases to http_test.go
elastic · Apr 6, 2018 · f8cec4d · f8cec4d
1 parent 42081b9
commit f8cec4d
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 5 deletions.
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
@@ -59,6 +59,8 @@ https://github.com/elastic/beats/compare/v5.6.8...5.6[Check the HEAD diff]
 
 *Packetbeat*
 
+- HTTP parses successfully on empty status phrase. {issue}6176[6176]
+
 *Winlogbeat*
 
 ==== Deprecated

diff --git a/packetbeat/protos/http/http_parser.go b/packetbeat/protos/http/http_parser.go
@@ -221,9 +221,6 @@ func parseResponseStatus(s []byte) (uint16, []byte, error) {
 		return 0, nil, fmt.Errorf("Unable to parse status code from [%s]", s)
 	}
 	phrase := s[p+1:]
-	if len(phrase) == 0 {
-		return 0, nil, fmt.Errorf("Unable to parse status phrase from [%s]", s)
-	}
 	return uint16(statusCode), phrase, nil
 }
 

diff --git a/packetbeat/protos/http/http_test.go b/packetbeat/protos/http/http_test.go
@@ -380,6 +380,54 @@ func TestHttpParser_Response_HTTP_10_without_content_length(t *testing.T) {
 	assert.Equal(t, 4, message.contentLength)
 }
 
+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" +
+		"<!DOCTYPE HTML PUBLIC \"-//IETF//DTD HTML 2.0//EN\">\r\n" +
+		"<html><head>\r\n" +
+		"<title>301 Moved Permanently</title>\r\n" +
+		"</head><body>\r\n" +
+		"<h1>Moved Permanently</h1>\r\n" +
+		"<p>The document has moved <a href=\"http://www.hotnews.ro/\">here</a>.</p>\r\n" +
+		"<hr>\r\n" +
+		"<address>Apache Server at hotnews.ro Port 80</address>\r\n" +
+		"</body></html>"
+	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))
+}
+
 func TestHttpParser_splitResponse_midBody(t *testing.T) {
 	data1 := "HTTP/1.1 200 OK\r\n" +
 		"Date: Tue, 14 Aug 2012 22:31:45 GMT\r\n" +
@@ -566,8 +614,11 @@ func TestHttpParser_PhraseContainsSpaces(t *testing.T) {
 		"\r\n" +
 		"xx"
 	r, ok, complete = testParse(nil, broken)
-	assert.False(t, ok)
-	assert.False(t, complete)
+	assert.True(t, ok)
+	assert.True(t, complete)
+	assert.Equal(t, 2, r.contentLength)
+	assert.Equal(t, "", string(r.statusPhrase))
+	assert.Equal(t, 500, int(r.statusCode))
 }
 
 func TestEatBodyChunked(t *testing.T) {