GitLab

Describe the bug

The request parser of Kea HTTP library fails to parse empty POST requests with the header Content-Length: 0.

Impact of the bug

I'm trying to build a RESTful style API for Kea CA, where some endpoints accept empty POST requests to conduct non-idempotent operations. Using the popular command-line utility cURL, clients can do this in two ways:

1. curl -iv -H "Content-Type: application/json" -X POST http://127.0.0.1:5000 (no Content-Length header)
2. curl -iv -H "Content-Type: application/json" -d '' http://127.0.0.1:5000 (contains the header Content-Length: 0)

The HTTP 1.1 spec recommends the second one, but does not explicitly prohibit the first one. So I think Kea should support both to avoid surprising unknowing clients.

A user agent SHOULD send a Content-Length in a request message when no Transfer-Encoding is sent and the request method defines a meaning for an enclosed payload body. For example, a Content-Length header field is normally sent in a POST request even when the value is 0 (indicating an empty payload body).

For now, this bug has no impact on clients. And it can be circumvented as stated above. Therefore, it is not a priority.

To Reproduce

Pull Kea source code from Git. Add the following unit test to src/lib/http/tests/request_parser_unittests.cc. Compile with Google Test enabled. Then run the test with ./libhttp_unittests --gtest_filter="HttpRequestParserTest.parseEmptyPostRequest"

TEST_F(HttpRequestParserTest, parseEmptyPostRequest) {
    std::string http_req = "POST / HTTP/1.1\r\n"
        "Content-Type: application/json\r\n"
        "Content-Length: 0\r\n\r\n";

    ASSERT_NO_FATAL_FAILURE(doParse(http_req));

    EXPECT_EQ(HttpRequest::Method::HTTP_POST, request_.getMethod());
    EXPECT_EQ("/", request_.getUri());
    EXPECT_EQ("", request_.getBody());
    EXPECT_EQ(1, request_.getHttpVersion().major_);
    EXPECT_EQ(1, request_.getHttpVersion().minor_);
}

Expected behavior

The test should pass without problems. Instead, it fails and says

request_parser_unittests.cc:50: Failure
Value of: parser.needData()
  Actual: true
Expected: false
request_parser_unittests.cc:376: Failure
Expected: doParse(http_req) doesn't generate new fatal failures in the current thread.
  Actual: it does.

Additional Information

This slightly modified test, whose Content-Length value (2) is larger than the actual body length (1), produces an identical error message.

TEST_F(HttpRequestParserTest, parseEmptyPostRequest) {
    std::string http_req = "POST / HTTP/1.1\r\n"
        "Content-Type: application/json\r\n"
        "Content-Length: 2\r\n\r\n1";

    ASSERT_NO_FATAL_FAILURE(doParse(http_req));

    EXPECT_EQ(HttpRequest::Method::HTTP_POST, request_.getMethod());
    EXPECT_EQ("/", request_.getUri());
    EXPECT_EQ("1", request_.getBody());
    EXPECT_EQ(1, request_.getHttpVersion().major_);
    EXPECT_EQ(1, request_.getHttpVersion().minor_);
}