Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 58 additions & 3 deletions src/http_server/flb_http_server.c
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,44 @@ static int flb_http_server_session_write(struct flb_http_server_session *session
return 0;
}

/* Connection is a comma-separated list of tokens (RFC 9110). */
static int http_connection_header_has_token(const char *value, const char *token)
{
size_t token_len;
const char *cursor;
const char *start;

if (value == NULL || token == NULL) {
return FLB_FALSE;
}

token_len = strlen(token);
cursor = value;

while (*cursor != '\0') {
while (*cursor == ' ' || *cursor == '\t' || *cursor == ',') {
cursor++;
}

if (*cursor == '\0') {
break;
}

start = cursor;
while (*cursor != '\0' && *cursor != ',' &&
*cursor != ' ' && *cursor != '\t') {
cursor++;
}

if ((size_t) (cursor - start) == token_len &&
strncasecmp(start, token, token_len) == 0) {
return FLB_TRUE;
}
}

return FLB_FALSE;
}

static int flb_http_server_should_connection_be_closed(
struct flb_http_request *request)
{
Expand Down Expand Up @@ -417,11 +455,17 @@ static int flb_http_server_should_connection_be_closed(
keepalive = FLB_TRUE;
}

/* Override protocol defaults by checking connection header */
/* Override protocol defaults by checking connection header.
* HTTP/1.1 keep-alive is the default, so "Connection: close" must
* opt out. "close" takes precedence when both tokens are present.
*/
connection_header_value = flb_http_request_get_header(request,
"connection");
if (connection_header_value &&
strcasecmp(connection_header_value, "keep-alive") == 0) {
if (http_connection_header_has_token(connection_header_value, "close")) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve close across repeated Connection fields

On a valid HTTP/1.1 request with separate Connection lines, such as Connection: close followed by Connection: keep-alive, the HTTP/1 parser's request-header hash replaces the first value with the later one. This lookup then sees only keep-alive and leaves the socket open even though the combined field contains close. Aggregate repeated field instances before applying the close precedence and add a regression test for this representation.

AGENTS.md reference: AGENTS.md:L102

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked into this. Repeated Connection lines never reach this code as separate values: the bundled Monkey parser (lib/monkey/mk_server/mk_http_parser.c) keeps one slot per known header in p->headers[], so a later Connection: line overwrites the earlier one before http1_session_process_request() walks header_list. I tried aggregating in the HTTP/1 glue with a regression test for Connection: close + Connection: keep-alive, and the test still failed because only keep-alive is in the list.

So fixing that case means changing Monkey's parser, which I'd rather keep out of this PR. The single field and comma-separated forms (keep-alive, close) are handled and covered by tests here. Happy to follow up on the Monkey side if maintainers want it.

keepalive = FLB_FALSE;
}
else if (http_connection_header_has_token(connection_header_value,
"keep-alive")) {
keepalive = FLB_TRUE;
}

Expand Down Expand Up @@ -557,6 +601,17 @@ static int flb_http_server_client_activity_event_handler(void *data)
request->content_length = cfl_sds_len(request->body);
}

/* Announce the close before the handler commits the response,
* otherwise HTTP/1.1 clients that sent "Connection: close"
* get a keep-alive response and then wait for IO timeout.
*/
if (request->protocol_version < HTTP_PROTOCOL_VERSION_20 &&
flb_http_server_should_connection_be_closed(request)) {
flb_http_response_set_header(response,
"Connection", 0,
"close", 0);
}

callback_context.request = request;
callback_context.response = response;
if (server->authorization_callback != NULL) {
Expand Down
271 changes: 271 additions & 0 deletions tests/internal/http_server.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,12 @@
#include <fluent-bit/http_server/flb_hs.h>
#include <msgpack.h>

#include <fluent-bit/flb_time.h>
#include <string.h>
#include <cfl/cfl_atomic.h>
#ifndef _WIN32
#include <sys/select.h>
#endif

#include "flb_tests_internal.h"

Expand Down Expand Up @@ -1324,6 +1328,267 @@ static void test_http_request_compression_roundtrip(void)
}
}


static int test_http_server_response_has_connection_close(const char *response)
{
const char *cursor;
const char *headers_end;
const char *line_end;
size_t name_len;

headers_end = strstr(response, "\r\n\r\n");
if (headers_end == NULL) {
return FLB_FALSE;
}

cursor = response;
name_len = strlen("Connection:");
while (cursor < headers_end) {
line_end = strstr(cursor, "\r\n");
if (line_end == NULL || line_end > headers_end) {
break;
}
if ((size_t) (line_end - cursor) > name_len &&
strncasecmp(cursor, "Connection:", name_len) == 0) {
cursor += name_len;
while (cursor < line_end && (*cursor == ' ' || *cursor == '\t')) {
cursor++;
}
if ((size_t) (line_end - cursor) >= 5 &&
strncasecmp(cursor, "close", 5) == 0) {
return FLB_TRUE;
}
return FLB_FALSE;
}
cursor = line_end + 2;
}

return FLB_FALSE;
}

/* Start a one-worker server, send one request, and check whether the
* response advertises Connection: close and whether the server actually
* closes (EOF) without waiting for the idle timeout.
*/
static void test_http1_connection_header(const char *request, int expect_close)
{
char response[1024];
struct sockaddr_in address;
struct test_http_server_context context;
struct flb_http_server_options options;
struct flb_http_server server;
struct flb_net_setup net_setup;
struct flb_config *config;
struct timeval timeout;
fd_set read_fds;
flb_sockfd_t fd;
size_t request_length;
size_t request_offset;
size_t response_length;
int got_eof;
int attempt;
int port;
int ret;
int written;
int received;

fd = FLB_INVALID_SOCKET;
got_eof = FLB_FALSE;
response_length = 0;
memset(response, 0, sizeof(response));

ret = test_http_server_network_init();
if (ret != 0) {
return;
}

config = flb_config_init();
if (!TEST_CHECK(config != NULL)) {
test_http_server_network_cleanup();
return;
}

test_http_server_context_init(&context);
context.expected_idle_timeout = HTTP_SERVER_DEFAULT_IDLE_TIMEOUT;

port = test_http_server_reserve_port();
if (!TEST_CHECK(port > 0)) {
test_http_server_context_destroy(&context);
flb_config_exit(config);
test_http_server_network_cleanup();
return;
}

flb_net_setup_init(&net_setup);
flb_http_server_options_init(&options);

options.protocol_version = HTTP_PROTOCOL_VERSION_AUTODETECT;
options.request_callback = test_http_server_request_handler;
options.user_data = &context;
options.address = (char *) TEST_HTTP_SERVER_HOST;
options.port = port;
options.networking_flags = FLB_IO_TCP;
options.networking_setup = &net_setup;
options.system_context = config;
options.workers = 1;
options.use_caller_event_loop = FLB_FALSE;

ret = flb_http_server_init_with_options(&server, &options);
if (!TEST_CHECK(ret == 0)) {
test_http_server_context_destroy(&context);
flb_config_exit(config);
test_http_server_network_cleanup();
return;
}

ret = flb_http_server_start(&server);
if (!TEST_CHECK(ret == 0)) {
flb_http_server_destroy(&server);
test_http_server_context_destroy(&context);
flb_config_exit(config);
test_http_server_network_cleanup();
return;
}

memset(&address, 0, sizeof(address));
address.sin_family = AF_INET;
address.sin_port = htons(port);
address.sin_addr.s_addr = htonl(INADDR_LOOPBACK);

for (attempt = 0; attempt < 50; attempt++) {
fd = flb_net_socket_create(AF_INET, FLB_FALSE);
if (fd == FLB_INVALID_SOCKET) {
break;
}
ret = connect(fd, (struct sockaddr *) &address, sizeof(address));
if (ret == 0) {
break;
}
flb_socket_close(fd);
fd = FLB_INVALID_SOCKET;
flb_time_msleep(20);
}

if (!TEST_CHECK(fd != FLB_INVALID_SOCKET)) {
flb_http_server_destroy(&server);
test_http_server_context_destroy(&context);
flb_config_exit(config);
test_http_server_network_cleanup();
return;
}

request_length = strlen(request);
request_offset = 0;
while (request_offset < request_length) {
written = send(fd, request + request_offset,
(int) (request_length - request_offset), 0);
if (written <= 0) {
break;
}
request_offset += (size_t) written;
}
TEST_CHECK(request_offset == request_length);

while (response_length < sizeof(response) - 1) {
FD_ZERO(&read_fds);
FD_SET(fd, &read_fds);
timeout.tv_sec = 2;
timeout.tv_usec = 0;
ret = select((int) (fd + 1), &read_fds, NULL, NULL, &timeout);
if (ret <= 0) {
break;
}
received = recv(fd, response + response_length,
(int) (sizeof(response) - 1 - response_length), 0);
if (received < 0) {
break;
}
if (received == 0) {
got_eof = FLB_TRUE;
break;
}
response_length += (size_t) received;
response[response_length] = '\0';
if (strstr(response, "\r\n\r\nok") != NULL) {
break;
}
}

if (got_eof == FLB_FALSE) {
FD_ZERO(&read_fds);
FD_SET(fd, &read_fds);
timeout.tv_sec = 0;
timeout.tv_usec = 300000;
ret = select((int) (fd + 1), &read_fds, NULL, NULL, &timeout);
if (ret > 0) {
received = recv(fd, response + response_length,
(int) (sizeof(response) - 1 - response_length), 0);
if (received == 0) {
got_eof = FLB_TRUE;
}
else if (received > 0) {
response_length += (size_t) received;
response[response_length] = '\0';
}
}
}

TEST_CHECK(strstr(response, "200") != NULL);
TEST_CHECK(strstr(response, "\r\n\r\nok") != NULL);
if (expect_close) {
TEST_CHECK_(test_http_server_response_has_connection_close(response),
"expected Connection: close in: %s", response);
TEST_CHECK_(got_eof == FLB_TRUE,
"expected server close after Connection: close, got: %s",
response);
}
else {
TEST_CHECK_(test_http_server_response_has_connection_close(response) == FLB_FALSE,
"unexpected Connection: close in: %s", response);
TEST_CHECK_(got_eof == FLB_FALSE,
"server closed a keep-alive HTTP/1.1 connection: %s",
response);
}

flb_socket_close(fd);
flb_http_server_destroy(&server);
test_http_server_context_destroy(&context);
flb_config_exit(config);
test_http_server_network_cleanup();
}

void test_http1_connection_close_closes_and_echoes_header(void)
{
const char *request =
"GET / HTTP/1.1\r\n"
"Host: localhost\r\n"
"Connection: close\r\n"
"\r\n";

test_http1_connection_header(request, FLB_TRUE);
}

void test_http1_default_keepalive_leaves_connection_open(void)
{
const char *request =
"GET / HTTP/1.1\r\n"
"Host: localhost\r\n"
"\r\n";

test_http1_connection_header(request, FLB_FALSE);
}

void test_http1_connection_close_token_list(void)
{
const char *request =
"GET / HTTP/1.1\r\n"
"Host: localhost\r\n"
"Connection: keep-alive, close\r\n"
"\r\n";

test_http1_connection_header(request, FLB_TRUE);
}

TEST_LIST = {
{ "http_request_compression_roundtrip", test_http_request_compression_roundtrip },
{ "http_server_rejects_invalid_decoded_body_limit",
Expand Down Expand Up @@ -1364,5 +1629,11 @@ TEST_LIST = {
test_http_server_session_destroy_is_reentrant_safe },
{ "http_server_hs_buf_push_during_read_keeps_payload",
test_http_server_hs_buf_push_during_read_keeps_payload },
{ "http1_connection_close_closes_and_echoes_header",
test_http1_connection_close_closes_and_echoes_header },
{ "http1_default_keepalive_leaves_connection_open",
test_http1_default_keepalive_leaves_connection_open },
{ "http1_connection_close_token_list",
test_http1_connection_close_token_list },
{ 0 }
};
Loading