Skip to content

Commit

Permalink
[POA-1519] Allow HTTP response without reason phrase (#214)
Browse files Browse the repository at this point in the history
Technically, the RFC requires a space even if the reason phrase is
empty, but at least one service has been observed to violate this. Add
unit tests for both correct and incorrect blank reason phrase, and a one
reproducing the original error to verify that Go's http parser is fine
with this.
  • Loading branch information
mgritter authored Aug 5, 2024
1 parent ea7eb1c commit 6896f31
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 2 deletions.
8 changes: 6 additions & 2 deletions akinet/http/parser_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,13 +149,17 @@ func hasValidHTTPRequestLine(input memview.MemView) akinet.AcceptDecision {
// RFC 2616 Section 6.1. The input should start right after the HTTP version.
func hasValidHTTPResponseStatusLine(input memview.MemView) akinet.AcceptDecision {
if input.Len() < 5 {
// Need a 2 spaces plus 3 bytes for status code.
// Need a space, 3 bytes for status code, and a space or CR.
return akinet.NeedMoreData
}

// A space separates the HTTP version from status code.
// The format is SP Status-Code SP Reason-Phrase CR LF
if input.GetByte(0) != ' ' || input.GetByte(4) != ' ' {
// Some servers do not include the second 'SP" though and skip straight to 'CR'.
if input.GetByte(0) != ' ' {
return akinet.Reject
}
if input.GetByte(4) != ' ' && input.GetByte(4) != '\r' {
return akinet.Reject
}

Expand Down
12 changes: 12 additions & 0 deletions akinet/http/parser_factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,18 @@ func TestHTTPResponseParserFactoryAccepts(t *testing.T) {
expectedDecision: akinet.Accept,
expectedDF: int64(len("OK")),
},
// This is technically illegal but we observed some Postman services sending it.
{
name: "no reason message",
input: "HTTP/1.1 200\r\n",
expectedDecision: akinet.Accept,
},
// This is technically legal, as the reason string can be zero bytes long.
{
name: "no reason message",
input: "HTTP/1.1 200 \r\n",
expectedDecision: akinet.Accept,
},
/*
// Currently failing -- does not look for response that spans the end of
// what is available.
Expand Down
22 changes: 22 additions & 0 deletions akinet/http/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,28 @@ func TestHTTPResponseParser(t *testing.T) {
Body: memview.New([]byte("foobarbaz")),
},
},
{
name: "example response from Postman health check missing reason phrase",
input: "HTTP/1.1 200\r\nServer: nginx/1.25.3\r\nDate: Fri, 02 Aug 2024 03:04:14 GMT\r\n" +
"Content-Type: application/json; charset=utf-8\r\nContent-Length: 28\r\n" +
"Connection: close\r\nuWebSockets: 20\r\n\r\n{\"response\":\"Who is there?\"}",
expected: akinet.HTTPResponse{
StreamID: uuid.UUID(testBidiID),
Seq: 522,
StatusCode: 200,
ProtoMajor: 1,
ProtoMinor: 1,
Header: map[string][]string{
"Server": {"nginx/1.25.3"},
// "Connection": {"close"}, // ignored by parser
"Date": {"Fri, 02 Aug 2024 03:04:14 GMT"},
"Uwebsockets": {"20"}, // normalized
"Content-Length": {"28"},
"Content-Type": {"application/json; charset=utf-8"},
},
Body: memview.New([]byte("{\"response\":\"Who is there?\"}")),
},
},
{
name: "ignore trailing bytes",
verbatimInput: []memview.MemView{
Expand Down

0 comments on commit 6896f31

Please sign in to comment.