Packetbeat HTTP parsing fails on empty status phrase

[adriansr]

An HTTP response whose status phrase is empty causes the following error:

2018-01-25T10:50:49Z WARN Failed to understand HTTP response status: 302

For example :

HTTP/1.1 302
Location: http://172.17.0.12:8080/resources/userinfo
Content-Length: 0
Date: Thu, 25 Jan 2018 10:50:48 GMT
Connection: close

The response parser in https://github.com/elastic/beats/blob/master/packetbeat/protos/http/http_parser.go#L224 is expecting the status-phrase to be non-empty.

From this question on discuss.

[mbudnick]

The warning tells status:302 so the status seems to be parsed?

See my full log file for more examples: packetbeat.log

Packetbeat and tomcat running together in a docker-container. Maybe there is a problem with docker network because of logged messages "Ignore empty non-FIN packet"?

[adriansr]

@mbudnick It is failing because it expects something else after the status code. Your server status response is in the form:

HTTP/1.1 200
HTTP/1.1 302

When packetbeat is expecting a non-empty status phrase:

HTTP/1.1 200 OK
HTTP/1.1 302 Found

[jreyeshdez]

Hello @adriansr

I was checking and testing this issue and about to commit it to my fork but it seemed to obvious to me where the issue might be that I'd rather leave the question in this issue before actually make a PR.

In the function parseResponseStatus I would only do the following:
→ return uint16(statusCode), []byte(http.StatusText(statusCode)), nil
And delete the phrase := s[p+1:] as well as the if-statement check

Let me know what you think about this change.

[adriansr]

Hi @jreyeshdez

I think it's better to just return the phrase, even if empty, instead of an error.
Also, it will be cool if you can add a test case in http_test.go

[xkoomy]

Hi @adriansr
I see an implementation HTTP 1.1 where in status line is no space after status code (i.e. HTTP/1.1 200\r\n). In this case parseResponseStatus still doesn't work even after the patch. We can debate that HTTP standard says the space is mandatory but life looks different. For instance I got this case now during my job and have to workaround it.
Could you consider change parseResponseStatus to support such status lines too?

xkoomy

[adriansr]

@xkoomy I've submitted a patch for the issue: #6631