From 6896f31886a1f5fb04b915391ee36b3f6da66167 Mon Sep 17 00:00:00 2001 From: Mark Gritter Date: Mon, 5 Aug 2024 12:47:48 -0500 Subject: [PATCH] [POA-1519] Allow HTTP response without reason phrase (#214) 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. --- akinet/http/parser_factory.go | 8 ++++++-- akinet/http/parser_factory_test.go | 12 ++++++++++++ akinet/http/parser_test.go | 22 ++++++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/akinet/http/parser_factory.go b/akinet/http/parser_factory.go index 2475cd9..3e8c71e 100644 --- a/akinet/http/parser_factory.go +++ b/akinet/http/parser_factory.go @@ -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 } diff --git a/akinet/http/parser_factory_test.go b/akinet/http/parser_factory_test.go index 72245bb..8e23fb4 100644 --- a/akinet/http/parser_factory_test.go +++ b/akinet/http/parser_factory_test.go @@ -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. diff --git a/akinet/http/parser_test.go b/akinet/http/parser_test.go index 7691874..7c0be88 100644 --- a/akinet/http/parser_test.go +++ b/akinet/http/parser_test.go @@ -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{