Headers: Unit tests and alignment between protocols

README.md

@@ -118,6 +118,40 @@ namespace impl {
 ```
 
 
+## Concurrent Requests
+
+HTTP/2 and HTTP/3 multiplex requests: each one is a stream of its own, and streams make progress independently. HTTP/1.1 has a single connection instead, which requests are written to and responses read from, one after the other. anyhttp treats HTTP/1.1 as a protocol with **"max concurrent streams = 1"**, and makes that explicit in the client API instead of hiding it.
+
+### Complete requests and responses
+
+A request is *complete* when all of it -- header and body -- has been written to the connection:
+
+* A request **without a body** is complete as soon as `async_submit()` has succeeded. Whether a request has a body is a matter of its framing only ([RFC 9112, section 6.3](https://www.rfc-editor.org/rfc/rfc9112#section-6.3)), never of its method: it has none without `Transfer-Encoding` and with a `Content-Length` of zero or none. As the HTTP/1.1 client sends every request without `Content-Length` chunked, that means `Content-Length: 0`.
+* **Any other request** is complete when `async_write_eof()` has succeeded -- even if all of a `Content-Length` has been written before. The body of a request without one is ended implicitly: `async_write_eof()` is an idempotent no-op on it, and writing data fails with `broken_pipe`.
+
+A response is *complete* when it has been read to its end.
+
+### Rules for HTTP/1.1
+
+1. `async_submit()` fails with `asio::error::would_block` while the previous request is not complete.
+2. `async_get_response()` fails with `asio::error::would_block` while the responses to earlier requests have not been read completely -- otherwise, it would read one of those.
+3. When a request can not be completed -- a write fails, or it is released before it is complete -- the connection is stuck in the middle of a message: every later `async_submit()` fails with `asio::error::connection_aborted`.
+4. When a response can not be read completely -- it is released before its end, or its request is released without asking for it -- every later `async_get_response()` fails with `asio::error::connection_aborted`.
+
+Pipelining is still possible: complete requests can be sent before any of their responses have been read.
+
+### Design decisions
+
+* **Fail instead of waiting.** An operation that has to wait for an earlier request or response does not wait. Very often, the caller waiting is the one who has to finish that earlier request or response, after the operation returns -- waiting would deadlock. Failing immediately with `would_block` turns a hang into an error that can be handled: finish the earlier one, then retry.
+* **No queueing of submitted requests.** An earlier version queued the header of a request submitted while the previous one was still incomplete, and sent it as soon as that was complete. That allows code written for HTTP/2 -- submit a couple of requests first, write their bodies later -- to work unchanged. But it needed a queue of pending requests, writes waiting for a header still on its way (and not cancellable while doing so), and failures cascading to queued requests. Refusing the submission keeps it at a single incomplete request per session, which is exactly what the protocol allows.
+* **Requests and responses may outlive their session.** The session keeps track of all of its readers and writers, and detaches them when it goes away. Everything a detached request or response is asked to do after that completes with an error (`connection_aborted` for HTTP/1.1 and HTTP/2, `connection_reset` for HTTP/3), without touching the connection that is gone.
+
+### Outlook: HTTP/2 and HTTP/3
+
+HTTP/2 and HTTP/3 have a limit of their own: the peer's `SETTINGS_MAX_CONCURRENT_STREAMS`, or the QUIC stream limit. With that limit reached, they should behave just like HTTP/1.1 -- fail with `would_block` instead of waiting. Currently, anyhttp does not check that limit itself, and leaves it to nghttp2 and ngtcp2; tests for that are still to be added.
+
+One difference remains to be decided: in HTTP/2 and HTTP/3, a stream counts against the limit until it is closed in *both* directions, that is, until its response has been received as well. Taken strictly, "max concurrent streams = 1" would forbid submitting the next request before the previous response has been read -- which is stricter than HTTP/1.1 pipelining as implemented.
+
 ## Links
 
 For now, this section contains just a set of random links collected during development.

include/anyhttp/client.hpp

@@ -22,6 +22,14 @@ struct Config
    // FIXME: the client does not connect to an URL, it connects to a host:port or endpoint
    boost::urls::url url{"localhost:8080"};
    Protocol protocol{Protocol::h2};
+
+   //
+   // The largest header section of a response the client accepts, in bytes, counted as for
+   // server::Config::max_header_size. For a response with more, async_get_response() fails with
+   // boost::beast::http::error::header_limit and the stream is reset -- with HTTP/1.1, which has no
+   // streams, the connection can not be used any more.
+   //
+   size_t max_header_size = default_max_header_size;
 };
 
 // =================================================================================================
@@ -104,6 +112,16 @@ class Request
    using GetResponse = void(boost::system::error_code, Response);
    using GetResponseHandler = asio::any_completion_handler<GetResponse>;
 
+   /**
+    * Waits for the response to this request, until its header has been received.
+    *
+    * With HTTP/1.1, responses arrive in the order the requests were sent, one after the other.
+    * Getting the response to a request whose predecessors' responses have not been read to their
+    * end does not wait for that to happen, but fails immediately with
+    * \c asio::error::would_block. After a response could not be read -- it was released before
+    * its end, or its request was released without asking for it -- getting any later response
+    * fails with \c asio::error::connection_aborted. See README.md, "Concurrent Requests".
+    */
    template <BOOST_ASIO_COMPLETION_TOKEN_FOR(GetResponse) CompletionToken = DefaultCompletionToken>
    auto async_get_response(CompletionToken&& token = CompletionToken())
    {

include/anyhttp/client_impl.hpp

@@ -48,9 +48,9 @@ class Client::Impl
    boost::asio::any_io_executor get_executor() const noexcept { return m_executor; }
 
    void async_connect(ConnectHandler handler);
+   const Config& config() const { return m_config; }
 
 private:
-   const Config& config() const { return m_config; }
    awaitable<Session> async_connect();
 
 private:

include/anyhttp/common.hpp

@@ -52,6 +52,19 @@ std::ostream& operator<<(std::ostream& str, Protocol protocol);
 using Fields = boost::beast::http::fields;
 static_assert(boost::beast::http::is_fields<Fields>::value);
 
+/// Default for \c server::Config::max_header_size and \c client::Config::max_header_size.
+inline constexpr size_t default_max_header_size = 64 * 1024;
+
+/**
+ * What a received header field counts against the limit on the size of a header section: its name
+ * and value plus 32 bytes of overhead, as for SETTINGS_MAX_HEADER_LIST_SIZE in HTTP/2 (RFC 9113,
+ * section 6.5.2) and SETTINGS_MAX_FIELD_SECTION_SIZE in HTTP/3 (RFC 9114, section 4.2.2).
+ */
+constexpr size_t header_field_size(std::string_view name, std::string_view value) noexcept
+{
+   return name.size() + value.size() + 32;
+}
+
 //
 // A header value as passed to fields() below: either something string-like, or anything
 // std::format can turn into a string, so sizes and counts need no conversion at the call site.

include/anyhttp/detail/h2_session_details.hpp

@@ -6,7 +6,9 @@
 //
 
 #include "anyhttp/any_async_stream.hpp"
+#include "anyhttp/h2_common.hpp"
 #include "anyhttp/h2_session.hpp"
+#include "anyhttp/literals.hpp"
 
 #include <boost/asio/basic_stream_socket.hpp>
 #include <boost/asio/buffer.hpp>
@@ -158,7 +160,7 @@ awaitable<void> NGHttp2SessionImpl<Stream>::send_loop()
 template <typename Stream>
 awaitable<void> NGHttp2SessionImpl<Stream>::recv_loop()
 {
-   m_buffer.reserve(64 * 1024);
+   m_buffer.reserve(64_k);
 
    unsigned int reason = NGHTTP2_NO_ERROR;
    while (nghttp2_session_want_read(session) || nghttp2_session_want_write(session))
@@ -190,6 +192,7 @@ ServerSession<Stream>::ServerSession(server::Server::Impl& parent, any_io_execut
                                      Stream&& stream)
    : ServerReference(parent), super("\x1b[1;31mserver\x1b[0m", executor, std::move(stream))
 {
+   m_max_header_size = parent.config().max_header_size;
 }
 
 // -------------------------------------------------------------------------------------------------
@@ -210,12 +213,19 @@ awaitable<void> ServerSession<Stream>::do_session(Buffer&& buffer)
    auto options = nghttp2_option_new();
    nghttp2_option_set_no_http_messaging(options.get(), 0); // h2spec: fails ~16 tests if 1
    nghttp2_option_set_no_auto_window_update(options.get(), 1);
+   nghttp2_option_set_max_send_header_block_length(options.get(), 1_m);
+   nghttp2_option_set_max_continuations(options.get(), max_continuations(m_max_header_size));
 
    if (auto rv = nghttp2_session_server_new2(&session, callbacks.get(), this, options.get()))
       throw std::runtime_error("nghttp2_session_server_new");
 
 #if 1
-   const uint32_t window_size = 1024 * 1024;
+   const uint32_t window_size = 1_m;
+   //
+   // No SETTINGS_MAX_HEADER_LIST_SIZE: it defaults to unlimited and is advisory anyway, as nghttp2
+   // enforces it in neither direction. Header sections beyond max_header_size are rejected where
+   // they arrive, see on_header_callback().
+   //
    std::array<nghttp2_settings_entry, 2> iv{{{NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS, 100},
                                              {NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE, window_size}}};
    nghttp2_submit_settings(session, NGHTTP2_FLAG_NONE, iv.data(), iv.size());
@@ -234,9 +244,9 @@ awaitable<void> ServerSession<Stream>::do_session(Buffer&& buffer)
    {
       const auto& settings = m_upgrade->settings;
       const bool head_request = m_upgrade->method == "HEAD";
-      if (auto rv = nghttp2_session_upgrade2(session,
-                                             reinterpret_cast<const uint8_t*>(settings.data()),
-                                             settings.size(), head_request, nullptr))
+      if (auto rv =
+             nghttp2_session_upgrade2(session, reinterpret_cast<const uint8_t*>(settings.data()),
+                                      settings.size(), head_request, nullptr))
       {
          mloge("nghttp2_session_upgrade2: {}", nghttp2_strerror(rv));
          nghttp2_session_terminate_session(session, NGHTTP2_PROTOCOL_ERROR);
@@ -279,6 +289,7 @@ ClientSession<Stream>::ClientSession(client::Client::Impl& parent, any_io_execut
                                      Stream&& stream)
    : ClientReference(parent), super("\x1b[1;32mclient\x1b[0m", executor, std::move(stream))
 {
+   m_max_header_size = parent.config().max_header_size;
 }
 
 // -------------------------------------------------------------------------------------------------
@@ -299,12 +310,19 @@ awaitable<void> ClientSession<Stream>::do_session(Buffer&& buffer)
    auto options = nghttp2_option_new();
    nghttp2_option_set_no_http_messaging(options.get(), 1);
    nghttp2_option_set_no_auto_window_update(options.get(), 1);
+   nghttp2_option_set_max_send_header_block_length(options.get(), 1_m);
+   nghttp2_option_set_max_continuations(options.get(), max_continuations(m_max_header_size));
 
    if (auto rv = nghttp2_session_client_new2(&session, callbacks.get(), this, options.get()))
       throw std::runtime_error("nghttp2_session_client_new");
 
 #if 1
-   const uint32_t window_size = 1024 * 1024;
+   const uint32_t window_size = 1_m;
+   //
+   // No SETTINGS_MAX_HEADER_LIST_SIZE: it defaults to unlimited and is advisory anyway, as nghttp2
+   // enforces it in neither direction. Header sections beyond max_header_size are rejected where
+   // they arrive, see on_header_callback().
+   //
    std::array<nghttp2_settings_entry, 2> iv{{{NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS, 100},
                                              {NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE, window_size}}};
    nghttp2_submit_settings(session, NGHTTP2_FLAG_NONE, iv.data(), iv.size());

include/anyhttp/formatter.hpp

@@ -10,8 +10,10 @@
 #include <boost/url/authority_view.hpp>
 #include <boost/url/pct_string_view.hpp>
 
-#include <thread>
+#include <cstddef>
 #include <format>
+#include <string_view>
+#include <thread>
 
 // =================================================================================================
 
@@ -80,6 +82,48 @@ struct std::formatter<boost::beast::http::field>
 
 // =================================================================================================
 
+namespace anyhttp
+{
+
+/// A string to be logged, cut short if it is longer than \c max_size bytes. See truncated().
+struct Truncated
+{
+   std::string_view text;
+   size_t max_size;
+};
+
+/// Default for truncated(): long enough for any regular header, short enough to keep the log readable.
+inline constexpr size_t max_logged_size = 80;
+
+/**
+ * Wraps \p text for logging, so that only its first \p max_size bytes are printed, followed by the
+ * total size. Meant for header names and values, which may be of almost any size:
+ * \code
+ * logd("{}: {}", truncated(name), truncated(value)); // x-large: aaaa... (30000 bytes, truncated)
+ * \endcode
+ */
+inline Truncated truncated(std::string_view text, size_t max_size = max_logged_size)
+{
+   return {text, max_size};
+}
+
+} // namespace anyhttp
+
+template <>
+struct std::formatter<anyhttp::Truncated> : std::formatter<std::string_view>
+{
+   auto format(const anyhttp::Truncated& value, std::format_context& ctx) const
+   {
+      if (value.text.size() <= value.max_size)
+         return std::formatter<std::string_view>::format(value.text, ctx);
+
+      return std::format_to(ctx.out(), "{}... ({} bytes, truncated)",
+                            value.text.substr(0, value.max_size), value.text.size());
+   }
+};
+
+// =================================================================================================
+
 template <>
 struct std::formatter<boost::asio::cancellation_type> : std::formatter<std::string_view>
 {