http: support broken status line (#6631) (#6740)

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
elastic · Apr 3, 2018 · 9a93ee7 · 9a93ee7
1 parent bf5e16f
commit 9a93ee7
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 47 deletions.
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
@@ -369,6 +369,8 @@ https://github.com/elastic/beats/compare/v6.0.1...v6.1.0[View commits]
 *Packetbeat*
 
 - Add support for decoding the TLS envelopes. {pull}5476[5476]
+- HTTP parses successfully on empty status phrase. {issue}6176[6176]
+- HTTP parser supports broken status line. {pull}6631[6631]
 
 [[release-notes-6.0.1]]
 === Beats version 6.0.1

diff --git 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
@@ -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" +
-		"<!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))
+	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) {