mhd_h2_conn_streamid_window_incr() added the stream id to the stream
send window instead of the increment from the WINDOW_UPDATE frame. So a
reply larger than the window the client announced stalled after the
first window. The overflow check used the stream id too, in signed
arithmetic, which overflows for a large stream id.
The increment is now added, and the sum is checked in unsigned
arithmetic, as the connection window check does.
Added three cases to test_h2_raw: a reply larger than the announced
window, a stream window raised to exactly 2147483647 (accepted), and
one raised above it (refused).
---
src/mhd2/h2/h2_conn_streams.c | 4 +-
src/tests/raw/test_h2_raw.c | 298 ++++++++++++++++++++++++++++++++--
2 files changed, 283 insertions(+), 19 deletions(-)
diff --git a/src/mhd2/h2/h2_conn_streams.c b/src/mhd2/h2/h2_conn_streams.c
index b190dd9..f472959 100644
--- a/src/mhd2/h2/h2_conn_streams.c
+++ b/src/mhd2/h2/h2_conn_streams.c
@@ -533,10 +533,10 @@ mhd_h2_conn_streamid_window_incr (struct
MHD_Connection *restrict c,
return true; /* Just ignore the frame */
}
if ((0 < s->state.send_window)
- && (0 > s->state.send_window + (int_least32_t)stream_id))
+ && ((incr + (uint_least32_t)s->state.send_window) > 0x7FFFFFFFu))
return mhd_h2_stream_req_problem (s,
mhd_H2_REQ_PRBLM_FLOW_CONTROL);
- s->state.send_window += (int_least32_t)stream_id;
+ s->state.send_window += (int_least32_t)incr;
return true;
}
diff --git a/src/tests/raw/test_h2_raw.c b/src/tests/raw/test_h2_raw.c
index a5f1e49..4ad0372 100644
--- a/src/tests/raw/test_h2_raw.c
+++ b/src/tests/raw/test_h2_raw.c
@@ -66,6 +66,11 @@
*/
#define H2_FRAME_HDR_SIZE 9
+/**
+ * The type of the DATA frame, RFC 9113, Section 6.1.
+ */
+#define H2_FRAME_DATA 0x0
+
/**
* The type of the HEADERS frame, RFC 9113, Section 6.2.
*/
@@ -86,13 +91,19 @@
*/
#define H2_FRAME_GOAWAY 0x7
+/**
+ * The type of the WINDOW_UPDATE frame, RFC 9113, Section 6.9.
+ */
+#define H2_FRAME_WINDOW_UPDATE 0x8
+
/**
* The PROTOCOL_ERROR code, RFC 9113, Section 7.
*/
#define H2_ERR_PROTOCOL_ERROR 0x1
/**
- * The END_STREAM flag of the HEADERS frame, RFC 9113, Section 6.2.
+ * The END_STREAM flag of the DATA and HEADERS frames, RFC 9113,
+ * Sections 6.1 and 6.2.
*/
#define H2_FLAG_END_STREAM 0x1
@@ -101,6 +112,11 @@
*/
#define H2_FLAG_END_HEADERS 0x4
+/**
+ * The SETTINGS_INITIAL_WINDOW_SIZE identifier, RFC 9113, Section 6.5.2.
+ */
+#define H2_SETTINGS_INITIAL_WINDOW_SIZE 0x4
+
/**
* Defines a test.
*/
@@ -206,6 +222,90 @@ static const struct Test tests[] = {
};
+/**
+ * The request of the window tests.
+ */
+static const struct Test body_request = {
+ .method = "GET",
+ .scheme = "http",
+ .path = "/body",
+ .authority = "localhost",
+ .expect_processed = true,
+};
+
+
+/**
+ * The reply body of the window tests, larger than the initial
+ * connection window of 65535 bytes, RFC 9113, Section 6.9.2.
+ */
+static const char body[70000] = { 0 };
+
+
+/**
+ * Defines a window test: the client requests the body, and opens the
+ * windows with one WINDOW_UPDATE frame for the stream and one for the
+ * connection when the server has used them.
+ */
+struct WindowTest
+{
+ /**
+ * Human-readable name of the test. NULL to end test array.
+ */
+ const char *name;
+
+ /**
+ * The initial stream window of the client, sent as its only setting.
+ */
+ uint32_t init_window;
+
+ /**
+ * The increment of the stream window.
+ */
+ uint32_t stream_incr;
+
+ /**
+ * The increment of the connection window.
+ */
+ uint32_t conn_incr;
+
+ /**
+ * True if the stream window would exceed 2^31-1, so the server must
+ * reset the stream or end the connection instead of sending the rest.
+ */
+ bool expect_rejected;
+};
+
+
+static const struct WindowTest window_tests[] = {
+ {
+ /* RFC 9113, Section 6.5: a SETTINGS frame may carry one setting */
+ .name = "HTTP/2 GET with a stream window update",
+ .init_window = 1000,
+ .stream_incr = sizeof (body) - 1000,
+ .conn_incr = sizeof (body) - 65535,
+ },
+ {
+ /* RFC 9113, Section 6.9.1: the stream window may reach 2^31-1 */
+ .name = "HTTP/2 GET with a stream window update to 2^31-1",
+ .init_window = 0x7fffffffu,
+ .stream_incr = 65535,
+ .conn_incr = sizeof (body) - 65535,
+ },
+ {
+ /* RFC 9113, Section 6.9.1: a stream window above 2^31-1 is a
+ flow-control error */
+ .name = "HTTP/2 GET with a stream window update above 2^31-1",
+ .init_window = 0x7fffffffu,
+ .stream_incr = 65536,
+ .conn_incr = sizeof (body) - 65535,
+ .expect_rejected = true,
+ },
+ {
+ .name = NULL,
+ }
+};
+
+
/**
* True if the request has been given to the application.
* Set by a daemon thread, read by the main thread only after the reply
@@ -221,6 +321,7 @@ static uint16_t port;
/**
* Records that the request reached the application and replies.
+ * The reply for the path of the window tests is the body.
*
* @param cls closure
* @param request the request object
@@ -237,11 +338,17 @@ server_req_cb (void *cls,
uint_fast64_t upload_size)
{
(void)cls; /* Unused */
- (void)path; /* Unused */
(void)method; /* Unused */
(void)upload_size; /* Unused */
processed = true;
+ if (0 == strcmp (path->cstr,
+ body_request.path))
+ return MHD_action_from_response (
+ request,
+ MHD_response_from_buffer_static (MHD_HTTP_STATUS_OK,
+ sizeof (body),
+ body));
return MHD_action_from_response (
request,
MHD_response_from_empty (MHD_HTTP_STATUS_NO_CONTENT));
@@ -407,14 +514,13 @@ put_field (unsigned char *buf,
/**
- * Send the connection preface, the SETTINGS frame required to complete
- * it, and the request of the test as a single HEADERS frame on stream 1.
+ * Send the request of the test as a single HEADERS frame on stream 1.
*
* @param s the socket to send to
* @param t the test to send the request of
*/
static void
-send_request (int s,
+send_headers (int s,
const struct Test *t)
{
/* Five fields, each within the limits of put_field(), plus the frame
@@ -422,17 +528,6 @@ send_request (int s,
unsigned char buf[5 * 256 + H2_FRAME_HDR_SIZE];
size_t size;
- write_all (s,
- H2_PREFACE,
- strlen (H2_PREFACE));
- put_frame_header (buf,
- H2_FRAME_SETTINGS,
- 0,
- 0,
- 0);
- write_all (s,
- buf,
- H2_FRAME_HDR_SIZE);
size = put_field (buf + H2_FRAME_HDR_SIZE,
2 /* ":method" */,
NULL,
@@ -465,6 +560,43 @@ send_request (int s,
}
+/**
+ * Send the connection preface and the SETTINGS frame required to
+ * complete it.
+ *
+ * @param s the socket to send to
+ * @param init_window the initial stream window to send as the only
+ * setting, zero to send no setting
+ */
+static void
+send_preface (int s,
+ uint32_t init_window)
+{
+ unsigned char buf[H2_FRAME_HDR_SIZE + 6];
+ const size_t size = (0 == init_window) ? 0 : 6;
+
+ write_all (s,
+ H2_PREFACE,
+ strlen (H2_PREFACE));
+ put_frame_header (buf,
+ H2_FRAME_SETTINGS,
+ 0,
+ 0,
+ size);
+ if (0 != size)
+ {
+ /* Exactly one setting: a frame of six bytes must be processed too */
+ buf[H2_FRAME_HDR_SIZE] = 0;
+ buf[H2_FRAME_HDR_SIZE + 1] = H2_SETTINGS_INITIAL_WINDOW_SIZE;
+ put_u32 (buf + H2_FRAME_HDR_SIZE + 2,
+ init_window);
+ }
+ write_all (s,
+ buf,
+ H2_FRAME_HDR_SIZE + size);
+}
+
+
/**
* A connection to our server and the data read from it.
*/
@@ -663,7 +795,9 @@ run_test (const struct Test *t)
if (0 != connect_to_server (&c))
return 1;
processed = false;
- send_request (c.s,
+ send_preface (c.s,
+ 0);
+ send_headers (c.s,
t);
ret = read_reply (&c,
&answer,
@@ -705,6 +839,124 @@ run_test (const struct Test *t)
}
+/**
+ * Request the body with the initial window of the test and check that
+ * the server stops when the window is used, then send the window updates
+ * of the test in a single write. A stream window above 2^31-1 is a
+ * flow-control error (RFC 9113, Section 6.9.1).
+ *
+ * @param t the test to run
+ * @return 0 if succeed, 1 on failure
+ */
+static int
+run_window_test (const struct WindowTest *t)
+{
+ /* The smaller of the stream window and the initial connection window,
+ RFC 9113, Section 6.9.2 */
+ const uint32_t window = (65535u < t->init_window) ? 65535u :
+ t->init_window;
+ struct Conn c;
+ const unsigned char *f;
+ size_t data_got = 0;
+ bool window_sent = false;
+
+ if (0 != connect_to_server (&c))
+ return 1;
+ send_preface (c.s,
+ t->init_window);
+ send_headers (c.s,
+ &body_request);
+ while (NULL != (f = read_frame (&c)))
+ {
+ if ((H2_FRAME_RST_STREAM == f[3])
+ || (H2_FRAME_GOAWAY == f[3]))
+ {
+ close (c.s);
+ if (t->expect_rejected && window_sent)
+ return 0;
+ if (!window_sent)
+ {
+ fprintf (stderr,
+ "The server ended the stream before the window update\n");
+ return 1;
+ }
+ fprintf (stderr,
+ "The server rejected a stream window increment of %u\n",
+ (unsigned int)t->stream_incr);
+ return 1;
+ }
+ if ((H2_FRAME_DATA == f[3])
+ && (1 == (get_u32 (f + 5) & 0x7fffffffu)))
+ {
+ data_got += get_u24 (f);
+ if ((!window_sent) && (window < data_got))
+ {
+ fprintf (stderr,
+ "The server sent %u bytes, more than the window of %u\n",
+ (unsigned int)data_got,
+ (unsigned int)window);
+ close (c.s);
+ return 1;
+ }
+ if (0 != (f[4] & H2_FLAG_END_STREAM))
+ {
+ close (c.s);
+ if (t->expect_rejected)
+ {
+ fprintf (stderr,
+ "The server accepted a stream window above 2^31-1\n");
+ return 1;
+ }
+ if (sizeof (body) != data_got)
+ {
+ fprintf (stderr,
+ "The server sent %u bytes of the body, expected %u\n",
+ (unsigned int)data_got,
+ (unsigned int)sizeof (body));
+ return 1;
+ }
+ return 0;
+ }
+ }
+ if ((!window_sent) && (window <= data_got))
+ {
+ unsigned char wu[2 * (H2_FRAME_HDR_SIZE + 4)];
+
+ put_frame_header (wu,
+ H2_FRAME_WINDOW_UPDATE,
+ 0,
+ 1,
+ 4);
+ put_u32 (wu + H2_FRAME_HDR_SIZE,
+ t->stream_incr);
+ put_frame_header (wu + H2_FRAME_HDR_SIZE + 4,
+ H2_FRAME_WINDOW_UPDATE,
+ 0,
+ 0,
+ 4);
+ put_u32 (wu + 2 * H2_FRAME_HDR_SIZE + 4,
+ t->conn_incr);
+ write_all (c.s,
+ wu,
+ sizeof (wu));
+ window_sent = true;
+ }
+ }
+ if (t->expect_rejected && window_sent && (0 == errno))
+ {
+ close (c.s);
+ return 0; /* The server closed the connection without a frame */
+ }
+ fprintf (stderr,
+ "The server sent only %u bytes of the body: %s\n",
+ (unsigned int)data_got,
+ (0 == errno) ? "the connection has been closed" :
+ strerror (errno));
+ close (c.s);
+ return 1;
+}
+
+
static int
run_tests (void)
{
@@ -720,6 +972,18 @@ run_tests (void)
return 1;
}
}
+ for (unsigned int i = 0;
+ NULL != window_tests[i].name;
+ i++)
+ {
+ if (0 != run_window_test (&window_tests[i]))
+ {
+ fprintf (stderr,
+ "Test `%s' failed\n",
+ window_tests[i].name);
+ return 1;
+ }
+ }
return 0;
}
--
2.43.0
