michaelh/zephyr
1
0
Fork 0
Code Releases Activity

net: http_server: fix header capture on concurrent http2 streams

Browse source 
Concurrent HTTP POST requests on different HTTP2 concurrent streams
require that the client's header_capture_context is re-used to capture
headers on a second stream before all of the body data has been received
(and sent to the application) on the first stream.

As a result, any captured headers must be sent to the application
callback before any headers can be received on a different stream. In
practice this means that for HTTP2 the application callback is called
for the first time on receiving a headers frame, before any data frames
are received. All subsequent application callbacks will not include the
request header data.

While this mechanism is not necessary for HTTP1, it is also updated to
only send headers in the first application callback for consistency.

Fixes #82273

Signed-off-by: Matt Rodgers <mrodgers@witekio.com>
This commit is contained in:
Matt Rodgers 2024-11-29 18:04:30 +00:00 • committed by Anas Nashif
commit ddaeb1379a
9 changed files with 354 additions and 139 deletions
Download patch file Download diff file Expand all files Collapse all files

232
tests/net/lib/http_server/core/src/main.c
View file

@ -44,7 +44,18 @@ BUILD_ASSERT(sizeof(long_payload) - 1 > CONFIG_HTTP_SERVER_CLIENT_BUFFER_SIZE,
"long_payload should be longer than client buffer to test payload being sent to "
"application across multiple calls to dynamic resource callback");
/* Individual HTTP2 frames, used to compose requests. */
/* Individual HTTP2 frames, used to compose requests.
*
* Headers and data frames can be composed based on a "real" request by copying the frame from a
* wireshark capture (Copy --> ...as a hex stream) and formatting into a C array initializer using
* xxd:
*
* echo "<frame_as_hex_stream>" | xxd -r -p | xxd -i
*
* For example:
* $ echo "01234567" | xxd -r -p | xxd -i
* 0x01, 0x23, 0x45, 0x67
*/
#define TEST_HTTP2_MAGIC \
0x50, 0x52, 0x49, 0x20, 0x2a, 0x20, 0x48, 0x54, 0x54, 0x50, 0x2f, 0x32, \
0x2e, 0x30, 0x0d, 0x0a, 0x0d, 0x0a, 0x53, 0x4d, 0x0d, 0x0a, 0x0d, 0x0a
@ -106,6 +117,22 @@ BUILD_ASSERT(sizeof(long_payload) - 1 > CONFIG_HTTP_SERVER_CLIENT_BUFFER_SIZE,
0x9f, 0x87, 0x49, 0x50, 0x98, 0xbb, 0x8e, 0x8b, 0x4b, 0x40, 0x88, 0x49, \
0x50, 0x95, 0xa7, 0x28, 0xe4, 0x2d, 0x82, 0x88, 0x49, 0x50, 0x98, 0xbb, \
0x8e, 0x8b, 0x4a, 0x2f
#define TEST_HTTP2_HEADERS_POST_HEADER_CAPTURE_WITH_TESTHEADER_STREAM_1 \
0x00, 0x00, 0x4b, 0x01, 0x04, 0x00, 0x00, 0x00, TEST_STREAM_ID_1, \
0x83, 0x04, 0x8b, 0x62, 0x72, 0x8e, 0x42, 0xd9, 0x11, 0x07, 0x5a, 0x6d, \
0xb0, 0xbf, 0x86, 0x41, 0x87, 0x0b, 0xe2, 0x5c, 0x0b, 0x89, 0x70, 0xff, \
0x7a, 0x88, 0x25, 0xb6, 0x50, 0xc3, 0xab, 0xbc, 0x15, 0xc1, 0x53, 0x03, \
0x2a, 0x2f, 0x2a, 0x40, 0x88, 0x49, 0x50, 0x95, 0xa7, 0x28, 0xe4, 0x2d, \
0x9f, 0x87, 0x49, 0x50, 0x98, 0xbb, 0x8e, 0x8b, 0x4b, 0x5f, 0x8b, 0x1d, \
0x75, 0xd0, 0x62, 0x0d, 0x26, 0x3d, 0x4c, 0x74, 0x41, 0xea, 0x0f, 0x0d, \
0x02, 0x31, 0x30
#define TEST_HTTP2_HEADERS_POST_HEADER_CAPTURE2_NO_TESTHEADER_STREAM_2 \
0x00, 0x00, 0x39, 0x01, 0x04, 0x00, 0x00, 0x00, TEST_STREAM_ID_2, \
0x83, 0x04, 0x8b, 0x62, 0x72, 0x8e, 0x42, 0xd9, 0x11, 0x07, 0x5a, 0x6d, \
0xb0, 0xa2, 0x86, 0x41, 0x87, 0x0b, 0xe2, 0x5c, 0x0b, 0x89, 0x70, 0xff, \
0x7a, 0x88, 0x25, 0xb6, 0x50, 0xc3, 0xab, 0xbc, 0x15, 0xc1, 0x53, 0x03, \
0x2a, 0x2f, 0x2a, 0x5f, 0x8b, 0x1d, 0x75, 0xd0, 0x62, 0x0d, 0x26, 0x3d, \
0x4c, 0x74, 0x41, 0xea, 0x0f, 0x0d, 0x02, 0x31, 0x30
#define TEST_HTTP2_HEADERS_GET_RESPONSE_HEADERS_STREAM_1 \
0x00, 0x00, 0x28, 0x01, 0x05, 0x00, 0x00, 0x00, TEST_STREAM_ID_1, \
0x82, 0x04, 0x8c, 0x62, 0xc2, 0xa2, 0xb3, 0xd4, 0x82, 0xc5, 0x39, 0x47, \
@ -163,6 +190,12 @@ BUILD_ASSERT(sizeof(long_payload) - 1 > CONFIG_HTTP_SERVER_CLIENT_BUFFER_SIZE,
0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0x00, 0x00, TEST_STREAM_ID_1, \
0x54, 0x65, 0x73, 0x74, 0x20, 0x64, 0x79, 0x6e, 0x61, 0x6d, 0x69, 0x63, \
0x20, 0x50, 0x4f, 0x53, 0x54
#define TEST_HTTP2_DATA_POST_HEADER_CAPTURE_STREAM_1 \
0x00, 0x00, 0x0a, 0x00, 0x01, 0x00, 0x00, 0x00, TEST_STREAM_ID_1, \
0x7b, 0x22, 0x74, 0x65, 0x73, 0x74, 0x22, 0x3a, 0x31, 0x7d
#define TEST_HTTP2_DATA_POST_HEADER_CAPTURE_STREAM_2 \
0x00, 0x00, 0x0a, 0x00, 0x01, 0x00, 0x00, 0x00, TEST_STREAM_ID_2, \
0x7b, 0x22, 0x74, 0x65, 0x73, 0x74, 0x22, 0x3a, 0x31, 0x7d
#define TEST_HTTP2_TRAILING_HEADER_STREAM_1 \
0x00, 0x00, 0x0c, 0x01, 0x05, 0x00, 0x00, 0x00, TEST_STREAM_ID_1, \
0x40, 0x84, 0x92, 0xda, 0x69, 0xf5, 0x85, 0x9c, 0xa3, 0x90, 0xb6, 0x7f
@ -190,8 +223,9 @@ HTTP_RESOURCE_DEFINE(static_resource, test_http_service, "/",
static uint8_t dynamic_payload[32];
static size_t dynamic_payload_len = sizeof(dynamic_payload);
static int dynamic_cb(struct http_client_ctx *client, enum http_data_status status, uint8_t *buffer,
size_t len, struct http_response_ctx *response_ctx, void *user_data)
static int dynamic_cb(struct http_client_ctx *client, enum http_data_status status,
const struct http_request_ctx *request_ctx,
struct http_response_ctx *response_ctx, void *user_data)
{
static size_t offset;
@ -207,13 +241,13 @@ static int dynamic_cb(struct http_client_ctx *client, enum http_data_status stat
response_ctx->final_chunk = true;
break;
case HTTP_POST:
if (len + offset > sizeof(dynamic_payload)) {
if (request_ctx->data_len + offset > sizeof(dynamic_payload)) {
return -ENOMEM;
}
if (len > 0) {
memcpy(dynamic_payload + offset, buffer, len);
offset += len;
if (request_ctx->data_len > 0) {
memcpy(dynamic_payload + offset, request_ctx->data, request_ctx->data_len);
offset += request_ctx->data_len;
}
if (status == HTTP_SERVER_DATA_FINAL) {
@ -244,36 +278,43 @@ struct http_resource_detail_dynamic dynamic_detail = {
HTTP_RESOURCE_DEFINE(dynamic_resource, test_http_service, "/dynamic",
&dynamic_detail);
static struct http_header_capture_ctx header_capture_ctx_clone;
struct test_headers_clone {
uint8_t buffer[CONFIG_HTTP_SERVER_CAPTURE_HEADER_BUFFER_SIZE];
struct http_header headers[CONFIG_HTTP_SERVER_CAPTURE_HEADER_COUNT];
size_t count;
enum http_header_status status;
};
static int dynamic_request_headers_cb(struct http_client_ctx *client, enum http_data_status status,
uint8_t *buffer, size_t len,
const struct http_request_ctx *request_ctx,
struct http_response_ctx *response_ctx, void *user_data)
{
ptrdiff_t offset;
struct http_header *hdrs_src;
struct http_header *hdrs_dst;
struct test_headers_clone *clone = (struct test_headers_clone *)user_data;
if (status == HTTP_SERVER_DATA_FINAL) {
if (request_ctx->header_count != 0) {
/* Copy the captured header info to static buffer for later assertions in testcase.
* Don't assume that the buffer inside client context remains valid after return
* from the callback - this is currently the case at the time of writing but could
* change. Also need to update pointers within structure with an offset to point at
* new buffer.
* from the callback. Also need to update pointers within structure with an offset
* to point at new buffer.
*/
memcpy(&header_capture_ctx_clone, &client->header_capture_ctx,
sizeof(header_capture_ctx_clone));
memcpy(clone->buffer, &client->header_capture_ctx, sizeof(clone->buffer));
hdrs_src = client->header_capture_ctx.headers;
hdrs_dst = header_capture_ctx_clone.headers;
offset = header_capture_ctx_clone.buffer - client->header_capture_ctx.buffer;
clone->count = request_ctx->header_count;
clone->status = request_ctx->headers_status;
for (int i = 0; i < CONFIG_HTTP_SERVER_CAPTURE_HEADER_COUNT; i++) {
hdrs_src = request_ctx->headers;
hdrs_dst = clone->headers;
offset = clone->buffer - client->header_capture_ctx.buffer;
for (int i = 0; i < request_ctx->header_count; i++) {
if (hdrs_src[i].name != NULL) {
hdrs_dst[i].name = hdrs_src[i].name + offset;
}
if (hdrs_dst[i].value != NULL) {
if (hdrs_src[i].value != NULL) {
hdrs_dst[i].value = hdrs_src[i].value + offset;
}
}
@ -282,6 +323,10 @@ static int dynamic_request_headers_cb(struct http_client_ctx *client, enum http_
return 0;
}
/* Define two resources for testing header capture, so that we can check concurrent streams */
static struct test_headers_clone request_headers_clone;
static struct test_headers_clone request_headers_clone2;
struct http_resource_detail_dynamic dynamic_request_headers_detail = {
.common = {
.type = HTTP_RESOURCE_TYPE_DYNAMIC,
@ -289,12 +334,25 @@ struct http_resource_detail_dynamic dynamic_request_headers_detail = {
.content_type = "text/plain",
},
.cb = dynamic_request_headers_cb,
.user_data = NULL
.user_data = &request_headers_clone,
};
struct http_resource_detail_dynamic dynamic_request_headers_detail2 = {
.common = {
.type = HTTP_RESOURCE_TYPE_DYNAMIC,
.bitmask_of_supported_http_methods = BIT(HTTP_GET) | BIT(HTTP_POST),
.content_type = "text/plain",
},
.cb = dynamic_request_headers_cb,
.user_data = &request_headers_clone2,
};
HTTP_RESOURCE_DEFINE(dynamic_request_headers_resource, test_http_service, "/header_capture",
&dynamic_request_headers_detail);
HTTP_RESOURCE_DEFINE(dynamic_request_headers_resource2, test_http_service, "/header_capture2",
&dynamic_request_headers_detail2);
HTTP_SERVER_REGISTER_HEADER_CAPTURE(capture_user_agent, "User-Agent");
HTTP_SERVER_REGISTER_HEADER_CAPTURE(capture_test_header, "Test-Header");
HTTP_SERVER_REGISTER_HEADER_CAPTURE(capture_test_header2, "Test-Header2");
@ -326,7 +384,7 @@ static uint8_t dynamic_response_headers_variant;
static uint8_t dynamic_response_headers_buffer[sizeof(long_payload)];
static int dynamic_response_headers_cb(struct http_client_ctx *client, enum http_data_status status,
uint8_t *buffer, size_t len,
const struct http_request_ctx *request_ctx,
struct http_response_ctx *response_ctx, void *user_data)
{
static bool request_continuation;
@ -340,6 +398,14 @@ static int dynamic_response_headers_cb(struct http_client_ctx *client, enum http
{.name = "Content-Type", .value = "application/json"},
};
if (status != HTTP_SERVER_DATA_FINAL &&
dynamic_response_headers_variant != DYNAMIC_RESPONSE_HEADERS_VARIANT_BODY_LONG) {
/* Long body variant is the only one which needs to take some action before final
* data has been received from server
*/
return 0;
}
switch (dynamic_response_headers_variant) {
case DYNAMIC_RESPONSE_HEADERS_VARIANT_NONE:
break;
@ -400,10 +466,12 @@ static int dynamic_response_headers_cb(struct http_client_ctx *client, enum http
}
} else if (client->method == HTTP_POST) {
/* Copy POST payload into buffer for later comparison */
zassert(offset + len <= sizeof(dynamic_response_headers_buffer),
zassert(offset + request_ctx->data_len <=
sizeof(dynamic_response_headers_buffer),
"POST data too long for buffer");
memcpy(dynamic_response_headers_buffer + offset, buffer, len);
offset += len;
memcpy(dynamic_response_headers_buffer + offset, request_ctx->data,
request_ctx->data_len);
offset += request_ctx->data_len;
if (status == HTTP_SERVER_DATA_FINAL) {
offset = 0;
@ -544,10 +612,12 @@ static void expect_http2_headers_frame(size_t *offset, int stream_id, uint8_t fl
test_get_frame_header(offset, &frame);
zassert_equal(frame.type, HTTP2_HEADERS_FRAME, "Expected headers frame");
zassert_equal(frame.type, HTTP2_HEADERS_FRAME, "Expected headers frame, got frame type %u",
frame.type);
zassert_equal(frame.stream_identifier, stream_id,
"Invalid headers frame stream ID");
zassert_equal(frame.flags, flags, "Unexpected flags received");
zassert_equal(frame.flags, flags, "Unexpected flags received (expected %x got %x)", flags,
frame.flags);
/* Consume headers payload */
test_read_data(offset, frame.length);
@ -588,7 +658,8 @@ static void expect_http2_window_update_frame(size_t *offset, int stream_id)
zassert_equal(frame.type, HTTP2_WINDOW_UPDATE_FRAME,
"Expected window update frame");
zassert_equal(frame.stream_identifier, stream_id,
"Invalid window update frame stream ID");
"Invalid window update frame stream ID (expected %d got %d)", stream_id,
frame.stream_identifier);
zassert_equal(frame.flags, 0, "Unexpected flags received");
zassert_equal(frame.length, sizeof(uint32_t),
"Unexpected window update frame length");
@ -1173,14 +1244,14 @@ ZTEST(server_function_tests, test_http1_header_capture)
"Accept: */*\r\n"
"Accept-Encoding: deflate, gzip, br\r\n"
"\r\n";
struct http_header *hdrs = header_capture_ctx_clone.headers;
struct http_header *hdrs = request_headers_clone.headers;
int ret;
test_http1_header_capture_common(request);
zassert_equal(header_capture_ctx_clone.count, 2,
zassert_equal(request_headers_clone.count, 2,
"Didn't capture the expected number of headers");
zassert_equal(header_capture_ctx_clone.status, HTTP_HEADER_STATUS_OK,
zassert_equal(request_headers_clone.status, HTTP_HEADER_STATUS_OK,
"Header capture status was not OK");
zassert_not_equal(hdrs[0].name, NULL, "First header name is NULL");
@ -1207,14 +1278,14 @@ ZTEST(server_function_tests, test_http1_header_too_long)
"Accept: */*\r\n"
"Accept-Encoding: deflate, gzip, br\r\n"
"\r\n";
struct http_header *hdrs = header_capture_ctx_clone.headers;
struct http_header *hdrs = request_headers_clone.headers;
int ret;
test_http1_header_capture_common(request);
zassert_equal(header_capture_ctx_clone.count, 1,
zassert_equal(request_headers_clone.count, 1,
"Didn't capture the expected number of headers");
zassert_equal(header_capture_ctx_clone.status, HTTP_HEADER_STATUS_DROPPED,
zassert_equal(request_headers_clone.status, HTTP_HEADER_STATUS_DROPPED,
"Header capture status was OK, but should not have been");
/* First header too long should not stop second header being captured into first slot */
@ -1236,14 +1307,14 @@ ZTEST(server_function_tests, test_http1_header_too_many)
"Accept: */*\r\n"
"Accept-Encoding: deflate, gzip, br\r\n"
"\r\n";
struct http_header *hdrs = header_capture_ctx_clone.headers;
struct http_header *hdrs = request_headers_clone.headers;
int ret;
test_http1_header_capture_common(request);
zassert_equal(header_capture_ctx_clone.count, 2,
zassert_equal(request_headers_clone.count, 2,
"Didn't capture the expected number of headers");
zassert_equal(header_capture_ctx_clone.status, HTTP_HEADER_STATUS_DROPPED,
zassert_equal(request_headers_clone.status, HTTP_HEADER_STATUS_DROPPED,
"Header capture status OK, but should not have been");
zassert_not_equal(hdrs[0].name, NULL, "First header name is NULL");
@ -1290,14 +1361,14 @@ ZTEST(server_function_tests, test_http2_header_capture)
TEST_HTTP2_HEADERS_GET_HEADER_CAPTURE1_STREAM_1,
TEST_HTTP2_GOAWAY,
};
struct http_header *hdrs = header_capture_ctx_clone.headers;
struct http_header *hdrs = request_headers_clone.headers;
int ret;
common_verify_http2_get_header_capture_request(request, sizeof(request));
zassert_equal(header_capture_ctx_clone.count, 2,
zassert_equal(request_headers_clone.count, 2,
"Didn't capture the expected number of headers");
zassert_equal(header_capture_ctx_clone.status, HTTP_HEADER_STATUS_OK,
zassert_equal(request_headers_clone.status, HTTP_HEADER_STATUS_OK,
"Header capture status was not OK");
zassert_not_equal(hdrs[0].name, NULL, "First header name is NULL");
@ -1324,14 +1395,14 @@ ZTEST(server_function_tests, test_http2_header_too_long)
TEST_HTTP2_HEADERS_GET_HEADER_CAPTURE2_STREAM_1,
TEST_HTTP2_GOAWAY,
};
struct http_header *hdrs = header_capture_ctx_clone.headers;
struct http_header *hdrs = request_headers_clone.headers;
int ret;
common_verify_http2_get_header_capture_request(request, sizeof(request));
zassert_equal(header_capture_ctx_clone.count, 1,
zassert_equal(request_headers_clone.count, 1,
"Didn't capture the expected number of headers");
zassert_equal(header_capture_ctx_clone.status, HTTP_HEADER_STATUS_DROPPED,
zassert_equal(request_headers_clone.status, HTTP_HEADER_STATUS_DROPPED,
"Header capture status was OK, but should not have been");
/* First header too long should not stop second header being captured into first slot */
@ -1353,14 +1424,14 @@ ZTEST(server_function_tests, test_http2_header_too_many)
TEST_HTTP2_HEADERS_GET_HEADER_CAPTURE3_STREAM_1,
TEST_HTTP2_GOAWAY,
};
struct http_header *hdrs = header_capture_ctx_clone.headers;
struct http_header *hdrs = request_headers_clone.headers;
int ret;
common_verify_http2_get_header_capture_request(request, sizeof(request));
zassert_equal(header_capture_ctx_clone.count, 2,
zassert_equal(request_headers_clone.count, 2,
"Didn't capture the expected number of headers");
zassert_equal(header_capture_ctx_clone.status, HTTP_HEADER_STATUS_DROPPED,
zassert_equal(request_headers_clone.status, HTTP_HEADER_STATUS_DROPPED,
"Header capture status OK, but should not have been");
zassert_not_equal(hdrs[0].name, NULL, "First header name is NULL");
@ -1378,6 +1449,73 @@ ZTEST(server_function_tests, test_http2_header_too_many)
zassert_equal(0, ret, "Header strings did not match");
}
ZTEST(server_function_tests, test_http2_header_concurrent)
{
/* Two POST requests which are concurrent, ie. headers1, headers2, data1, data2 */
static const uint8_t request[] = {
TEST_HTTP2_MAGIC,
TEST_HTTP2_SETTINGS,
TEST_HTTP2_SETTINGS_ACK,
TEST_HTTP2_HEADERS_POST_HEADER_CAPTURE_WITH_TESTHEADER_STREAM_1,
TEST_HTTP2_HEADERS_POST_HEADER_CAPTURE2_NO_TESTHEADER_STREAM_2,
TEST_HTTP2_DATA_POST_HEADER_CAPTURE_STREAM_1,
TEST_HTTP2_DATA_POST_HEADER_CAPTURE_STREAM_2,
TEST_HTTP2_GOAWAY,
};
struct http_header *hdrs = request_headers_clone.headers;
struct http_header *hdrs2 = request_headers_clone2.headers;
int ret;
size_t offset = 0;
ret = zsock_send(client_fd, request, sizeof(request), 0);
zassert_not_equal(ret, -1, "send() failed (%d)", errno);
/* Wait for response on both resources before checking captured headers */
expect_http2_settings_frame(&offset, false);
expect_http2_settings_frame(&offset, true);
expect_http2_headers_frame(&offset, TEST_STREAM_ID_1,
HTTP2_FLAG_END_HEADERS | HTTP2_FLAG_END_STREAM, NULL, 0);
expect_http2_window_update_frame(&offset, TEST_STREAM_ID_1);
expect_http2_window_update_frame(&offset, 0);
expect_http2_headers_frame(&offset, TEST_STREAM_ID_2,
HTTP2_FLAG_END_HEADERS | HTTP2_FLAG_END_STREAM, NULL, 0);
/* Headers captured on /header_capture path should have two headers including the
* Test-Header
*/
zassert_equal(request_headers_clone.count, 2,
"Didn't capture the expected number of headers");
zassert_not_equal(hdrs[0].name, NULL, "First header name is NULL");
zassert_not_equal(hdrs[0].value, NULL, "First header value is NULL");
zassert_not_equal(hdrs[1].name, NULL, "Second header name is NULL");
zassert_not_equal(hdrs[1].value, NULL, "Second header value is NULL");
ret = strcmp(hdrs[0].name, "User-Agent");
zassert_equal(0, ret, "Header strings did not match");
ret = strcmp(hdrs[0].value, "curl/7.81.0");
zassert_equal(0, ret, "Header strings did not match");
ret = strcmp(hdrs[1].name, "Test-Header");
zassert_equal(0, ret, "Header strings did not match");
ret = strcmp(hdrs[1].value, "test_value");
zassert_equal(0, ret, "Header strings did not match");
/* Headers captured on the /header_capture2 path should have only one header, not including
* the Test-Header
*/
zassert_equal(request_headers_clone2.count, 1,
"Didn't capture the expected number of headers");
zassert_not_equal(hdrs2[0].name, NULL, "First header name is NULL");
zassert_not_equal(hdrs2[0].value, NULL, "First header value is NULL");
ret = strcmp(hdrs2[0].name, "User-Agent");
zassert_equal(0, ret, "Header strings did not match");
ret = strcmp(hdrs2[0].value, "curl/7.81.0");
zassert_equal(0, ret, "Header strings did not match");
}
static void test_http1_dynamic_response_headers(const char *request, const char *expected_response)
{
int ret;
@ -1982,6 +2120,8 @@ static void http_server_tests_before(void *fixture)
memset(dynamic_payload, 0, sizeof(dynamic_payload));
memset(dynamic_response_headers_buffer, 0, sizeof(dynamic_response_headers_buffer));
memset(&request_headers_clone, 0, sizeof(request_headers_clone));
memset(&request_headers_clone2, 0, sizeof(request_headers_clone2));
dynamic_payload_len = 0;
ret = http_server_start();