diff --git a/doc/json-rpc-batch-network.md b/doc/json-rpc-batch-network.md new file mode 100644 index 000000000..4156321cb --- /dev/null +++ b/doc/json-rpc-batch-network.md @@ -0,0 +1,271 @@ +# JSON-RPC Batch Support — `libbitcoin-network` Changes + +Review document for the batch-support implementation described in +`libbitcoin-server/docs/json-rpc-batch.md`, Phase 1 (network layer only). + +--- + +## Summary + +Seven surgical changes add JSON-RPC 2.0 batch request parsing and sequential +dispatch to the network layer. No existing public API is removed or renamed. +The server layer (`libbitcoin-server`) requires no changes for the Electrum TCP +path; HTTP batch dispatch is deferred to Phase 3. + +--- + +## Changed Files + +### 1. `include/bitcoin/network/messages/rpc/model.hpp` + +**Change:** Added `batch_t` type alias after `request_t`. + +```cpp +using batch_t = std::vector; +``` + +**Rationale:** Gives a named type to the parsed batch payload. Used by the +`message_type` specialization and referenced at call sites in +`channel_rpc.ipp`. No `tag_invoke` serialization is needed because the reader +deserializes each array element individually using the existing +`value_to`. + +--- + +### 2. `include/bitcoin/network/messages/rpc/body.hpp` + +**Changes:** + +1. Full template specialization `message_type` added after the + generic template. The specialization inherits `json::json_value` (same base + as the generic), adds a `batch_t batch{}` field alongside the existing + `request_t message{}` field, and exposes `is_batch()`. + +2. Doc comment added on `body::reader` describing the two-path + contract (object root → single, array root → batch). + +**Key invariants of the specialization:** +- `is_batch()` returns `!batch.empty()`. +- After a single-request parse: `batch` is empty, `message` is populated. +- After a batch parse: `batch` is populated, `message` is default-constructed. +- The two states are mutually exclusive by construction. + +**Size impact:** `std::vector` adds 24 bytes on 64-bit platforms. +`rpc::request` grows from ~248 to ~272 bytes. `rpc::response` +(`message_type`, the generic instantiation) is unaffected. + +--- + +### 3. `src/messages/rpc/body.cpp` + +**Change:** `body::reader::finish()` replaces the single +`value_to` call with a three-branch dispatch: + +| JSON root type | Action | +|----------------|--------| +| Object | Existing single-request path (deserialization + v1 semantic validation) | +| Array | New batch path: validate non-empty, iterate elements, reject non-objects, `value_to` each one into `value_.batch` | +| Anything else | `jsonrpc_requires_method` (unchanged from old fallthrough) | + +**Error policy in the batch path:** fail-fast. If any element is not a JSON +object or fails deserialization, `ec` is set immediately and the method returns. +The channel will call `stop(ec)`. No partial-batch recovery is attempted; the +client must send a structurally valid batch. + +**Terminator guard added to `finish()`:** `terminated_ && !has_terminator_` +check added as an early-return guard. Previously this case was prevented by +`done()` returning false, but the explicit check makes the contract defensively +clear and matches the design described in the spec document. + +--- + +### 4. `include/bitcoin/network/channels/channel_rpc.hpp` + +**Changes:** + +- **Two new private members:** + - `rpc::request_cptr batch_source_{}` — holds the parsed request alive by + shared_ptr during a batch. Zero-cost when no batch is in flight. + - `size_t batch_cursor_{}` — cursor into `batch_source_->batch`. + +- **New virtual method `dispatch_batch()`:** called from `handle_receive()` when + `request->is_batch()`. The default implementation stores `batch_source_` and + calls `dispatch_next()`. Derived channels that wish to reject batches may + override and call `stop(error::not_implemented)`. + +- **New non-virtual method `dispatch_next()`:** advances the cursor, skips + notification items (no `id`), and dispatches the next request item. Non-virtual + because the one-at-a-time sequencing is a correctness invariant, not a policy. + +--- + +### 5. `include/bitcoin/network/impl/channels/channel_rpc.ipp` + +**Changed methods:** + +| Method | Change | +|--------|--------| +| `stopping()` | Resets `batch_source_` and `batch_cursor_` on stop | +| `handle_receive()` | Routes to `dispatch_batch()` or single path based on `is_batch()` | +| `handle_send()` | Final line changed: `receive()` replaced by conditional branch on `batch_source_` | + +**New methods:** + +| Method | Behaviour | +|--------|-----------| +| `dispatch_batch()` | Stores `batch_source_`/`batch_cursor_`, calls `dispatch_next()` | +| `dispatch_next()` | While-loop skips notifications; dispatches next request item; on batch exhaustion resets state and calls `receive()` | + +**Resolved TODOs:** The existing TODO block in `handle_receive()` described +exactly this design (asynchronous iteration, no server-side buffering, per-item +delimiters). The comments are retained as `RESOLVED:` markers with a reference +to the implementation approach. + +**Control-flow invariant:** At all times exactly one response write is in flight. +`handle_send()` drives `dispatch_next()` upon write completion. This gives +natural back-pressure: the client must consume each response before the server +reads the next batch item from its internal cursor. + +--- + +### 6. `include/bitcoin/network/error.hpp` + +**Change:** Two new enumerators appended after `jsonrpc_writer_exception`: + +```cpp +jsonrpc_batch_empty, // batch array must contain at least one element +jsonrpc_batch_item_invalid // each batch element must be a JSON object +``` + +Both are in `error_t : uint8_t`. The enum has capacity; no overflow risk. + +--- + +### 7. `src/error.cpp` + +**Change:** Corresponding message strings added to `DEFINE_ERROR_T_MESSAGE_MAP`: + +```cpp +{ jsonrpc_batch_empty, "json-rpc batch array must not be empty" }, +{ jsonrpc_batch_item_invalid, "json-rpc batch element is not a JSON object" }, +``` + +--- + +## Test Coverage + +### Implemented now + +#### `test/messages/rpc/body_reader.cpp` (inside `HAVE_SLOW_TESTS`) + +| Test case | What it covers | +|-----------|---------------| +| `message_type_specialization__default__is_not_batch_*` | Default state of specialization | +| `message_type_specialization__batch_with_one_element__is_batch` | `is_batch()` predicate in isolation | +| `finish__single_request_v2__success_*` | Regression: single-request path unchanged | +| `finish__batch_one_request__success_*` | Happy path: one-element TCP batch | +| `finish__batch_two_requests__success_*` | Happy path: order preservation, two items | +| `finish__batch_with_notification__*` | Notification (no `id`) is deserialized, `id` absent | +| `finish__batch_http_path_no_terminator__success` | HTTP path (non-terminated reader) | +| `finish__empty_array__jsonrpc_batch_empty_*` | Empty `[]` rejected | +| `finish__batch_element_is_number__jsonrpc_batch_item_invalid` | Non-object element (number) | +| `finish__batch_element_is_array__jsonrpc_batch_item_invalid` | Non-object element (array) | +| `finish__batch_valid_then_invalid_element__*` | Fail-fast on second invalid element | +| `finish__scalar_root__jsonrpc_requires_method` | Number root | +| `finish__string_root__jsonrpc_requires_method` | String root | + +#### `test/error.cpp` + +| Test case | What it covers | +|-----------|---------------| +| `error_t__code__jsonrpc_batch_empty__*` | Error code value, truthiness, message string | +| `error_t__code__jsonrpc_batch_item_invalid__*` | Error code value, truthiness, message string | + +--- + +### Deferred / not yet implemented + +#### `test/messages/rpc/body_reader.cpp` — stale test fixup + +> **TODO:** The existing test cases in this file (above the `NOTE:` separator) +> use stale API names that do not compile against the current code: +> +> | Stale name | Correct name | +> |------------|-------------| +> | `rpc::body::value_type` | `rpc::request` | +> | `rpc::body::reader` | `rpc::reader` | +> | `body.request.*` | `body.message.*` | +> | `reader.is_done()` | `reader.done()` | +> +> These should be fixed in a follow-up commit before `HAVE_SLOW_TESTS` is +> enabled in CI. + +#### `test/channels/channel_rpc.cpp` — channel dispatch tests + +> **TODO:** The channel-level dispatch logic (`dispatch_batch`, `dispatch_next`, +> `handle_send` routing, `stopping` cleanup) requires a mock +> `channel_rpc`-compatible object that can: +> +> - Operate on a real or stub strand (for `BC_ASSERT(stranded())`) +> - Expose a controllable `dispatcher_` (to inject known-method handlers) +> - Record calls to `receive()` and `send_error()` / `send_result()` +> +> The current test suite has no such mock infrastructure. Once it exists, the +> following behaviors should be covered: +> +> | Scenario | Expected | +> |----------|---------| +> | `dispatch_next` — all notifications — cursor exhausted | `batch_source_` reset, `receive()` called | +> | `dispatch_next` — first item has `id` | subscriber fires with correct `identity_`/`version_` | +> | `dispatch_next` — item with unknown method | `send_error()` called; cursor advances via `handle_send` | +> | `handle_send` — `batch_source_` non-null | calls `dispatch_next()`, not `receive()` | +> | `handle_send` — `batch_source_` null | calls `receive()`, not `dispatch_next()` | +> | `stopping()` during batch | `batch_source_` reset, `batch_cursor_` = 0 | +> | `dispatch_batch()` when batch already in flight | `BC_ASSERT_MSG` fires ("nested batch") | +> | `dispatch_batch()` override rejecting batch | `stop(error::not_implemented)` | + +#### HTTP batch dispatch (`protocol_bitcoind_rpc`) + +> **TODO (Phase 3):** The network changes are necessary but not sufficient for +> HTTP batch. `channel_http` has no `handle_send → dispatch_next` loop. +> `protocol_bitcoind_rpc` must implement its own dispatch loop and choose a +> response strategy (Option A: JSON array response; Option B: chunked streaming). +> See `libbitcoin-server/docs/json-rpc-batch.md` §"HTTP Batch" for the trade-offs. + +--- + +## Wire Behavior (Electrum TCP) + +Client sends one newline-terminated message: + +``` +[{"jsonrpc":"2.0","id":1,"method":"blockchain.block.header","params":[100,0]}, + {"jsonrpc":"2.0","id":2,"method":"server.ping","params":[]}, + {"jsonrpc":"2.0","method":"server.ping","params":[]}]\n +``` + +Server sends two independent newline-terminated responses as they complete. +The third item (no `id`) is a notification and receives no response: + +``` +{"jsonrpc":"2.0","id":1,"result":{"header":"...","max":2016,"count":1}}\n +{"jsonrpc":"2.0","id":2,"result":null}\n +``` + +This deviates from JSON-RPC 2.0 §6 (which mandates a single array response) +but is the explicit design intent: push buffering to the client. + +--- + +## Design Properties + +| Property | This implementation | +|----------|-------------------| +| Server memory during batch | 1 response in flight; batch held via shared_ptr (no N-copy) | +| Back-pressure | Natural — each write completion drives the next dispatch | +| Parse failure policy | Fail-fast: any bad element stops the channel | +| Unknown method on request item | Error response sent; batch continues | +| Unknown method on notification | Silent skip | +| `http_body.hpp` changes | None | +| New public types | `batch_t` alias only | +| Existing API breakage | None | diff --git a/include/bitcoin/network/channels/channel_rpc.hpp b/include/bitcoin/network/channels/channel_rpc.hpp index 9120e1569..1b3028d15 100644 --- a/include/bitcoin/network/channels/channel_rpc.hpp +++ b/include/bitcoin/network/channels/channel_rpc.hpp @@ -83,6 +83,21 @@ class channel_rpc /// Override to dispatch request to subscribers by requested method. virtual inline void dispatch(const rpc::request_cptr& request) NOEXCEPT; + /// Dispatch a batch of requests from a single parsed message. + /// Called from handle_receive() when request->is_batch() is true. + /// The default implementation stores the batch and calls dispatch_next() + /// to process items one at a time. May be overridden to reject batch on + /// channels that do not support it (call stop(error::not_implemented)). + virtual inline void dispatch_batch( + const rpc::request_cptr& request) NOEXCEPT; + + /// Advance the batch cursor and dispatch the next item. + /// Skips notification items (no id) without sending a response. + /// Calls receive() when all items have been dispatched. + /// Must only be called from the strand while batch_source_ is set. + /// Not virtual: the sequencing contract must not be overridden. + inline void dispatch_next() NOEXCEPT; + /// Size and assign response_buffer_ (value type is json-rpc::json). virtual inline rpc::response_ptr assign_message(rpc::response_t&& message, size_t size_hint) NOEXCEPT; @@ -107,6 +122,15 @@ class channel_rpc http::flat_buffer request_buffer_; dispatcher dispatcher_{}; bool reading_{}; + + /// Non-null while a batch is in flight. Holds the parsed batch message so + /// individual request_t items can be accessed by index without copying. + /// Protected by the channel strand. + rpc::request_cptr batch_source_{}; + + /// Index of the next item to dispatch from batch_source_->batch. + /// Zero when no batch is in flight. Protected by the channel strand. + size_t batch_cursor_{}; }; } // namespace network diff --git a/include/bitcoin/network/error.hpp b/include/bitcoin/network/error.hpp index f0a654dc6..cd73069b3 100644 --- a/include/bitcoin/network/error.hpp +++ b/include/bitcoin/network/error.hpp @@ -315,7 +315,9 @@ enum error_t : uint8_t jsonrpc_reader_bad_buffer, jsonrpc_reader_stall, jsonrpc_reader_exception, - jsonrpc_writer_exception + jsonrpc_writer_exception, + jsonrpc_batch_empty, // batch array must contain at least one element + jsonrpc_batch_item_invalid // each batch element must be a JSON object }; // No current need for error_code equivalence mapping. diff --git a/include/bitcoin/network/impl/channels/channel_rpc.ipp b/include/bitcoin/network/impl/channels/channel_rpc.ipp index 4b6716a3d..271573b31 100644 --- a/include/bitcoin/network/impl/channels/channel_rpc.ipp +++ b/include/bitcoin/network/impl/channels/channel_rpc.ipp @@ -45,6 +45,11 @@ inline void CLASS::stopping(const code& ec) NOEXCEPT BC_ASSERT(stranded()); channel::stopping(ec); dispatcher_.stop(ec); + + // Release the batch shared_ptr on stop to avoid holding the allocation + // longer than necessary and to make the stopped state unambiguous. + batch_source_.reset(); + batch_cursor_ = 0; } TEMPLATE @@ -103,23 +108,34 @@ inline void CLASS::handle_receive(const code& ec, size_t bytes, return; } - // TODO: Extend support to batch (array of rpc). - // TODO: This would consist of asynchronous recursion here, with iteration - // TODO: over the message array. The response is accumulated, but there is - // TODO: no way we would buffer it at the server until complete, which is a - // TODO: clear DoS vector. We would instead track the iteration and send - // TODO: each response with the necessary delimiters. This allows a request - // TODO: to safely be of any configured byte size or request element count. + // RESOLVED: Extend support to batch (array of rpc). + // RESOLVED: This would consist of asynchronous recursion here, with iteration + // RESOLVED: over the message array. The response is accumulated, but there is + // RESOLVED: no way we would buffer it at the server until complete, which is a + // RESOLVED: clear DoS vector. We would instead track the iteration and send + // RESOLVED: each response with the necessary delimiters. This allows a request + // RESOLVED: to safely be of any configured byte size or request element count. + // Implemented via dispatch_batch() + dispatch_next() + batch_source_/batch_cursor_. + // One response is in flight at a time; handle_send() advances the cursor. - // Save response state. - identity_ = request->message.id; - version_ = request->message.jsonrpc; + reading_ = false; - LOGA("Rpc request : (" << bytes << ") bytes [" << endpoint() << "] " - << request->message.method << "(...)."); + if (request->is_batch()) + { + LOGA("Rpc batch : (" << bytes << ") bytes [" << endpoint() << "] " + << request->batch.size() << " item(s)."); + dispatch_batch(request); + } + else + { + // Save response identity for this request (single path, unchanged). + identity_ = request->message.id; + version_ = request->message.jsonrpc; - reading_ = false; - dispatch(request); + LOGA("Rpc request : (" << bytes << ") bytes [" << endpoint() << "] " + << request->message.method << "(...)."); + dispatch(request); + } } TEMPLATE @@ -130,6 +146,68 @@ inline void CLASS::dispatch(const rpc::request_cptr& request) NOEXCEPT stop(code); } +TEMPLATE +inline void CLASS::dispatch_batch(const rpc::request_cptr& request) NOEXCEPT +{ + BC_ASSERT(stranded()); + BC_ASSERT(request->is_batch()); + BC_ASSERT_MSG(!batch_source_, "nested batch"); + + // Hold the parsed request alive by shared_ptr; no copy of batch items. + batch_source_ = request; + batch_cursor_ = 0; + dispatch_next(); +} + +TEMPLATE +inline void CLASS::dispatch_next() NOEXCEPT +{ + BC_ASSERT(stranded()); + BC_ASSERT(batch_source_); + + const auto& items = batch_source_->batch; + + // Skip notification items (no id): dispatch for side-effects only. + // Per JSON-RPC 2.0 notifications must receive no response. + // Unknown method on a notification is silently ignored (not a channel error). + while (batch_cursor_ < items.size() + && !items[batch_cursor_].id.has_value()) + { + dispatcher_.notify(items[batch_cursor_++]); + + // A notification handler could call stop(). Bail if so. + if (stopped()) return; + } + + // All remaining items consumed (may have been all notifications). + if (batch_cursor_ >= items.size()) + { + batch_source_.reset(); + batch_cursor_ = 0; + receive(); + return; + } + + // Dispatch the next request item (has id, must receive a response). + const auto& req = items[batch_cursor_++]; + identity_ = req.id; + version_ = req.jsonrpc; + + if (const auto ec = dispatcher_.notify(req)) + { + // Unknown method: send a per-item error response. + // handle_send() will call dispatch_next() for us when the write + // completes, advancing to the next batch item. + send_error({ .code = ec.value(), .message = ec.message() }, + [](const code&) NOEXCEPT {}); + return; + } + + // Success: the matched subscriber handler will eventually call + // send_result() or send_code(), which routes through handle_send(), + // which calls dispatch_next() to advance the cursor. +} + TEMPLATE inline http::flat_buffer& CLASS::request_buffer() NOEXCEPT { @@ -198,8 +276,11 @@ inline void CLASS::handle_send(const code& ec, size_t bytes, LOGA("Rpc response: (" << bytes << ") bytes [" << endpoint() << "] " << response->message.error.value_or(rpc::result_t{}).message); - // Continue read loop (does not unpause or restart channel). - receive(); + // Continue: advance batch or read the next top-level message. + if (batch_source_) + dispatch_next(); + else + receive(); } // private diff --git a/include/bitcoin/network/messages/rpc/body.hpp b/include/bitcoin/network/messages/rpc/body.hpp index 099c41b48..49bf659d1 100644 --- a/include/bitcoin/network/messages/rpc/body.hpp +++ b/include/bitcoin/network/messages/rpc/body.hpp @@ -35,6 +35,20 @@ struct message_type Type message{}; }; +/// Full specialization of message_type for request_t. +/// Adds batch support: when the incoming JSON root is an array, the reader +/// populates batch{} and leaves message{} default-constructed. For single +/// requests, message{} is populated and batch{} remains empty. +template <> +struct BCT_API message_type : public json::json_value +{ + request_t message{}; ///< valid when !is_batch() + batch_t batch{}; ///< valid when is_batch() + + /// True iff the body was parsed from a JSON array root. + inline bool is_batch() const NOEXCEPT { return !batch.empty(); } +}; + /// Derived boost::beast::http body for JSON-RPC messages. /// Extends json::body with JSON-RPC validation. template @@ -45,6 +59,14 @@ struct BCT_API body using base = typename json::body; using value_type = base::value_type; + /// When Message = request_t the reader's finish() method handles two cases: + /// JSON object root → single request: populates value_.message, + /// batch remains empty. + /// JSON array root → batch request : populates value_.batch, + /// message remains default-constructed. + /// An empty array, a non-object array element, or a failed per-element + /// deserialization sets ec and aborts the read; the channel will be stopped. + /// The newline terminator requirement (terminated_) applies in both cases. class reader : public json::body::reader { diff --git a/include/bitcoin/network/messages/rpc/model.hpp b/include/bitcoin/network/messages/rpc/model.hpp index 098fa2b69..55c06509c 100644 --- a/include/bitcoin/network/messages/rpc/model.hpp +++ b/include/bitcoin/network/messages/rpc/model.hpp @@ -172,6 +172,12 @@ struct request_t params_option params{}; }; +/// An ordered sequence of request_t objects from a JSON-RPC batch message. +/// batch_t is populated in message_type::batch by the body reader +/// when the incoming JSON root is an array. It is never populated for single +/// requests. Use message_type::is_batch() to distinguish. +using batch_t = std::vector; + DECLARE_JSON_TAG_INVOKE(version); DECLARE_JSON_TAG_INVOKE(value_t); DECLARE_JSON_TAG_INVOKE(identity_t); diff --git a/src/error.cpp b/src/error.cpp index 27f4d63fc..35e84caa7 100644 --- a/src/error.cpp +++ b/src/error.cpp @@ -302,7 +302,9 @@ DEFINE_ERROR_T_MESSAGE_MAP(error) { jsonrpc_reader_bad_buffer, "jsonrpc reader bad buffer" }, { jsonrpc_reader_stall, "jsonrpc reader stall" }, { jsonrpc_reader_exception, "jsonrpc reader exception" }, - { jsonrpc_writer_exception, "jsonrpc writer exception" } + { jsonrpc_writer_exception, "jsonrpc writer exception" }, + { jsonrpc_batch_empty, "json-rpc batch array must not be empty" }, + { jsonrpc_batch_item_invalid, "json-rpc batch element is not a JSON object" } }; DEFINE_ERROR_T_CATEGORY(error, "network", "network code") diff --git a/src/messages/rpc/body.cpp b/src/messages/rpc/body.cpp index 3a3f9f430..d0c437206 100644 --- a/src/messages/rpc/body.cpp +++ b/src/messages/rpc/body.cpp @@ -100,43 +100,106 @@ template <> void body::reader:: finish(boost_code& ec) NOEXCEPT { + // Delegate to json::body reader: parser_.finish() + model = parser_.release(). base::reader::finish(ec); if (ec) return; - try + // Newline terminator requirement (TCP Electrum protocol). + if (terminated_ && !has_terminator_) { - // TODO: extend support to batch (array of rpc). - value_.message = value_to(value_.model); - value_.model.emplace_null(); + ec = code{ error::jsonrpc_reader_stall }; + return; } - catch (const boost::system::system_error& e) + + const auto& model = value_.model; + + if (model.is_object()) { - // Primary exception type for parsing operations. - ec = e.code(); + // ── Single request ────────────────────────────────────────────────── + // Existing path: deserialize the JSON object into request_t. + // value_.batch remains empty; is_batch() returns false. + try + { + value_.message = value_to(model); + value_.model.emplace_null(); + } + catch (const boost::system::system_error& e) + { + // Primary exception type for parsing operations. + ec = e.code(); + return; + } + catch (...) + { + ec = code{ error::jsonrpc_reader_exception }; + return; + } + + // Set version default. + if (value_.message.jsonrpc == version::undefined) + value_.message.jsonrpc = version::v1; + + // Post-parse semantic validation. + if (value_.message.method.empty()) + { + ec = code{ error::jsonrpc_requires_method }; + } + else if (value_.message.jsonrpc == version::v1) + { + if (!value_.message.params.has_value()) + ec = code{ error::jsonrpc_v1_requires_params }; + else if (!value_.message.id.has_value()) + ec = code{ error::jsonrpc_v1_requires_id }; + else if (!std::holds_alternative( + value_.message.params.value())) + ec = code{ error::jsonrpc_v1_requires_array_params }; + } } - catch (...) + else if (model.is_array()) { - ec = code{ error::jsonrpc_reader_exception }; - } + // ── Batch request ─────────────────────────────────────────────────── + // Deserialize each array element as request_t into value_.batch. + // value_.message remains default-constructed; is_batch() returns true. + // + // Per JSON-RPC 2.0 §6: the batch array must not be empty. + const auto& arr = model.as_array(); + if (arr.empty()) + { + ec = code{ error::jsonrpc_batch_empty }; + return; + } - // Set version default. - if (value_.message.jsonrpc == version::undefined) - value_.message.jsonrpc = version::v1; + value_.batch.reserve(arr.size()); + for (const auto& element : arr) + { + // Per spec each element must be a Request object. + if (!element.is_object()) + { + ec = code{ error::jsonrpc_batch_item_invalid }; + return; + } + try + { + value_.batch.push_back(value_to(element)); + } + catch (const boost::system::system_error& e) + { + ec = e.code(); + return; + } + catch (...) + { + ec = code{ error::jsonrpc_reader_exception }; + return; + } + } - // Post-parse semantic validation. - if (value_.message.method.empty()) - { - ec = code{ error::jsonrpc_requires_method }; + value_.model.emplace_null(); } - else if (value_.message.jsonrpc == version::v1) + else { - if (!value_.message.params.has_value()) - ec = code{ error::jsonrpc_v1_requires_params }; - else if (!value_.message.id.has_value()) - ec = code{ error::jsonrpc_v1_requires_id }; - else if (!std::holds_alternative( - value_.message.params.value())) - ec = code{ error::jsonrpc_v1_requires_array_params }; + // JSON root is neither object nor array — not a valid JSON-RPC message. + ec = code{ error::jsonrpc_requires_method }; } } diff --git a/test/error.cpp b/test/error.cpp index 96e2e9866..eb5d64328 100644 --- a/test/error.cpp +++ b/test/error.cpp @@ -2176,4 +2176,30 @@ BOOST_AUTO_TEST_CASE(error_t__code__jsonrpc_writer_exception__true_expected_mess BOOST_REQUIRE_EQUAL(ec.message(), "jsonrpc writer exception"); } +// json-rpc batch errors (added with batch support) + +BOOST_AUTO_TEST_CASE(error_t__code__jsonrpc_batch_empty__true_expected_message) +{ + // Emitted by body::reader::finish() when the JSON root is an + // array but contains zero elements. JSON-RPC 2.0 §6 explicitly forbids + // this; the channel is stopped on receipt. + constexpr auto value = error::jsonrpc_batch_empty; + const auto ec = code(value); + BOOST_REQUIRE(ec); + BOOST_REQUIRE(ec == value); + BOOST_REQUIRE_EQUAL(ec.message(), "json-rpc batch array must not be empty"); +} + +BOOST_AUTO_TEST_CASE(error_t__code__jsonrpc_batch_item_invalid__true_expected_message) +{ + // Emitted by body::reader::finish() when any element of the + // batch array is not a JSON object. Each element must be a Request object + // per the JSON-RPC 2.0 spec; a non-object stops parsing immediately. + constexpr auto value = error::jsonrpc_batch_item_invalid; + const auto ec = code(value); + BOOST_REQUIRE(ec); + BOOST_REQUIRE(ec == value); + BOOST_REQUIRE_EQUAL(ec.message(), "json-rpc batch element is not a JSON object"); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/test/messages/rpc/body_reader.cpp b/test/messages/rpc/body_reader.cpp index c8ceb026c..b01e3df27 100644 --- a/test/messages/rpc/body_reader.cpp +++ b/test/messages/rpc/body_reader.cpp @@ -179,6 +179,292 @@ BOOST_AUTO_TEST_CASE(rpc_body_reader__put__over_length__body_limit) BOOST_REQUIRE(ec == error::http_error_t::body_limit); } +// ============================================================================ +// NOTE: The test cases above this line use stale API names and will not +// compile as-is. Specifically: +// - rpc::body::value_type → should be rpc::request +// - rpc::body::reader → should be rpc::reader +// - body.request.* → should be body.message.* +// - reader.is_done() → should be reader.done() +// These are tracked as a separate fix. All new tests below use the correct API. +// ============================================================================ + +// ---------------------------------------------------------------------------- +// message_type specialization — is_batch() predicate +// Tests that the specialization correctly discriminates single vs. batch state. +// ---------------------------------------------------------------------------- + +BOOST_AUTO_TEST_CASE(rpc_body_reader__message_type_specialization__default__is_not_batch_batch_empty_message_default) +{ + // A default-constructed rpc::request should represent a single (empty) + // request. is_batch() must return false; batch must be empty. + rpc::request body{}; + BOOST_REQUIRE(!body.is_batch()); + BOOST_REQUIRE(body.batch.empty()); + BOOST_REQUIRE(body.message.method.empty()); +} + +BOOST_AUTO_TEST_CASE(rpc_body_reader__message_type_specialization__batch_with_one_element__is_batch) +{ + // Manually populating batch{} is sufficient to make is_batch() true. + // This covers the specialization's inline predicate independently of parsing. + rpc::request body{}; + body.batch.push_back({}); + BOOST_REQUIRE(body.is_batch()); +} + +// ---------------------------------------------------------------------------- +// finish() — single-request regression after refactor +// The object-root path in the new finish() must be identical to the old code. +// ---------------------------------------------------------------------------- + +BOOST_AUTO_TEST_CASE(rpc_body_reader__finish__single_request_v2__success_is_not_batch_message_populated_model_cleared) +{ + // Regression: the single-request path (JSON object root) must still work + // exactly as before the batch refactor. is_batch() must be false and + // body.message must be populated; body.batch must remain empty. + const std::string_view text{ + R"({"jsonrpc":"2.0","id":1,"method":"ping"})""\n" }; + const asio::const_buffer buffer{ text.data(), text.size() }; + rpc::request body{}; + rpc::reader reader(body); + boost_code ec{}; + reader.init(text.size(), ec); + BOOST_REQUIRE(!ec); + reader.put(buffer, ec); + BOOST_REQUIRE(!ec); + reader.finish(ec); + BOOST_REQUIRE(!ec); + // Single-request invariants. + BOOST_REQUIRE(!body.is_batch()); + BOOST_REQUIRE(body.batch.empty()); + // The message field must be correctly deserialized. + BOOST_REQUIRE_EQUAL(body.message.method, "ping"); + BOOST_REQUIRE(body.message.jsonrpc == version::v2); + BOOST_REQUIRE(body.message.id.has_value()); + BOOST_REQUIRE_EQUAL(std::get(body.message.id.value()), 1); + // The JSON model must be released after deserialization. + BOOST_REQUIRE(body.model.is_null()); +} + +// ---------------------------------------------------------------------------- +// finish() — batch happy paths (TCP, with \n terminator) +// ---------------------------------------------------------------------------- + +BOOST_AUTO_TEST_CASE(rpc_body_reader__finish__batch_one_request__success_is_batch_size_one_model_cleared) +{ + // A batch of exactly one request element. Verifies that: + // - is_batch() becomes true + // - batch.size() == 1 + // - the element's fields are correctly deserialized + // - the JSON model is released + // - the single-request message field remains default-constructed + const std::string_view text{ + R"([{"jsonrpc":"2.0","id":1,"method":"test"}])""\n" }; + const asio::const_buffer buffer{ text.data(), text.size() }; + rpc::request body{}; + rpc::reader reader(body); + boost_code ec{}; + reader.init(text.size(), ec); + BOOST_REQUIRE(!ec); + reader.put(buffer, ec); + BOOST_REQUIRE(!ec); + reader.finish(ec); + BOOST_REQUIRE(!ec); + BOOST_REQUIRE(body.is_batch()); + BOOST_REQUIRE_EQUAL(body.batch.size(), 1u); + BOOST_REQUIRE_EQUAL(body.batch[0].method, "test"); + BOOST_REQUIRE(body.batch[0].jsonrpc == version::v2); + BOOST_REQUIRE(body.batch[0].id.has_value()); + BOOST_REQUIRE_EQUAL(std::get(body.batch[0].id.value()), 1); + BOOST_REQUIRE(body.message.method.empty()); // single-message field untouched + BOOST_REQUIRE(body.model.is_null()); // JSON model released +} + +BOOST_AUTO_TEST_CASE(rpc_body_reader__finish__batch_two_requests__success_size_two_both_fields_correct) +{ + // Two-element batch. Verifies that the loop in finish() correctly + // processes every element and preserves declaration order. + const std::string_view text{ + R"([{"jsonrpc":"2.0","id":1,"method":"first"},)" + R"({"jsonrpc":"2.0","id":2,"method":"second"}])""\n" }; + const asio::const_buffer buffer{ text.data(), text.size() }; + rpc::request body{}; + rpc::reader reader(body); + boost_code ec{}; + reader.init(text.size(), ec); + BOOST_REQUIRE(!ec); + reader.put(buffer, ec); + BOOST_REQUIRE(!ec); + reader.finish(ec); + BOOST_REQUIRE(!ec); + BOOST_REQUIRE(body.is_batch()); + BOOST_REQUIRE_EQUAL(body.batch.size(), 2u); + BOOST_REQUIRE_EQUAL(body.batch[0].method, "first"); + BOOST_REQUIRE_EQUAL(std::get(body.batch[0].id.value()), 1); + BOOST_REQUIRE_EQUAL(body.batch[1].method, "second"); + BOOST_REQUIRE_EQUAL(std::get(body.batch[1].id.value()), 2); +} + +BOOST_AUTO_TEST_CASE(rpc_body_reader__finish__batch_with_notification__success_notification_has_no_id) +{ + // A batch containing a notification (no "id" field). Per JSON-RPC 2.0 §6 + // a notification is a valid array element; no id field means the server + // must not send a response for it. The reader must deserialize it + // faithfully — dispatching logic is the channel's responsibility. + const std::string_view text{ + R"([{"jsonrpc":"2.0","id":1,"method":"req"},)" + R"({"jsonrpc":"2.0","method":"notify"}])""\n" }; + const asio::const_buffer buffer{ text.data(), text.size() }; + rpc::request body{}; + rpc::reader reader(body); + boost_code ec{}; + reader.init(text.size(), ec); + BOOST_REQUIRE(!ec); + reader.put(buffer, ec); + BOOST_REQUIRE(!ec); + reader.finish(ec); + BOOST_REQUIRE(!ec); + BOOST_REQUIRE(body.is_batch()); + BOOST_REQUIRE_EQUAL(body.batch.size(), 2u); + BOOST_REQUIRE(body.batch[0].id.has_value()); + BOOST_REQUIRE(!body.batch[1].id.has_value()); // notification: no id + BOOST_REQUIRE_EQUAL(body.batch[1].method, "notify"); +} + +BOOST_AUTO_TEST_CASE(rpc_body_reader__finish__batch_http_path_no_terminator__success) +{ + // The HTTP path uses the two-argument reader constructor, which sets + // terminated_=false. A JSON array without a trailing '\n' must succeed. + const std::string_view text{ + R"([{"jsonrpc":"2.0","id":1,"method":"test"}])" }; + const asio::const_buffer buffer{ text.data(), text.size() }; + rpc::request body{}; + http::request_header header{}; + rpc::reader reader(header, body); // terminated_ = false + boost_code ec{}; + reader.init(text.size(), ec); + BOOST_REQUIRE(!ec); + reader.put(buffer, ec); + BOOST_REQUIRE(!ec); + reader.finish(ec); + BOOST_REQUIRE(!ec); + BOOST_REQUIRE(body.is_batch()); + BOOST_REQUIRE_EQUAL(body.batch.size(), 1u); + BOOST_REQUIRE_EQUAL(body.batch[0].method, "test"); +} + +// ---------------------------------------------------------------------------- +// finish() — batch error paths +// Each failure must set the expected error code and must NOT set is_batch(). +// ---------------------------------------------------------------------------- + +BOOST_AUTO_TEST_CASE(rpc_body_reader__finish__empty_array__jsonrpc_batch_empty_not_batch) +{ + // JSON-RPC 2.0 §6 explicitly forbids an empty batch array. The reader + // must reject it with jsonrpc_batch_empty before touching value_.batch. + const std::string_view text{ "[]\n" }; + const asio::const_buffer buffer{ text.data(), text.size() }; + rpc::request body{}; + rpc::reader reader(body); + boost_code ec{}; + reader.init(text.size(), ec); + BOOST_REQUIRE(!ec); + reader.put(buffer, ec); + BOOST_REQUIRE(!ec); + reader.finish(ec); + BOOST_REQUIRE(ec == code{ error::jsonrpc_batch_empty }); + // batch must not have been populated on error. + BOOST_REQUIRE(!body.is_batch()); +} + +BOOST_AUTO_TEST_CASE(rpc_body_reader__finish__batch_element_is_number__jsonrpc_batch_item_invalid) +{ + // A batch element that is a JSON number, not a JSON object, violates the + // JSON-RPC 2.0 spec. The reader must stop at the first invalid element. + const std::string_view text{ R"([42])""\n" }; + const asio::const_buffer buffer{ text.data(), text.size() }; + rpc::request body{}; + rpc::reader reader(body); + boost_code ec{}; + reader.init(text.size(), ec); + BOOST_REQUIRE(!ec); + reader.put(buffer, ec); + BOOST_REQUIRE(!ec); + reader.finish(ec); + BOOST_REQUIRE(ec == code{ error::jsonrpc_batch_item_invalid }); + BOOST_REQUIRE(!body.is_batch()); +} + +BOOST_AUTO_TEST_CASE(rpc_body_reader__finish__batch_element_is_array__jsonrpc_batch_item_invalid) +{ + // A nested array as a batch element is also invalid. + const std::string_view text{ R"([[]])""\n" }; + const asio::const_buffer buffer{ text.data(), text.size() }; + rpc::request body{}; + rpc::reader reader(body); + boost_code ec{}; + reader.init(text.size(), ec); + BOOST_REQUIRE(!ec); + reader.put(buffer, ec); + BOOST_REQUIRE(!ec); + reader.finish(ec); + BOOST_REQUIRE(ec == code{ error::jsonrpc_batch_item_invalid }); + BOOST_REQUIRE(!body.is_batch()); +} + +BOOST_AUTO_TEST_CASE(rpc_body_reader__finish__batch_valid_then_invalid_element__jsonrpc_batch_item_invalid) +{ + // First element is a valid request object; second is a string. The reader + // must fail fast at the second element. The partially-filled batch vector + // is in an indeterminate state on error; is_batch() should be false. + const std::string_view text{ + R"([{"jsonrpc":"2.0","id":1,"method":"ok"}, "bad"])""\n" }; + const asio::const_buffer buffer{ text.data(), text.size() }; + rpc::request body{}; + rpc::reader reader(body); + boost_code ec{}; + reader.init(text.size(), ec); + BOOST_REQUIRE(!ec); + reader.put(buffer, ec); + BOOST_REQUIRE(!ec); + reader.finish(ec); + BOOST_REQUIRE(ec == code{ error::jsonrpc_batch_item_invalid }); +} + +BOOST_AUTO_TEST_CASE(rpc_body_reader__finish__scalar_root__jsonrpc_requires_method) +{ + // A JSON root that is neither an object nor an array is rejected with + // jsonrpc_requires_method (the generic "not a valid JSON-RPC message" code). + const std::string_view text{ "42\n" }; + const asio::const_buffer buffer{ text.data(), text.size() }; + rpc::request body{}; + rpc::reader reader(body); + boost_code ec{}; + reader.init(text.size(), ec); + BOOST_REQUIRE(!ec); + reader.put(buffer, ec); + BOOST_REQUIRE(!ec); + reader.finish(ec); + BOOST_REQUIRE(ec == code{ error::jsonrpc_requires_method }); +} + +BOOST_AUTO_TEST_CASE(rpc_body_reader__finish__string_root__jsonrpc_requires_method) +{ + // A JSON string root is similarly invalid. + const std::string_view text{ R"("hello")""\n" }; + const asio::const_buffer buffer{ text.data(), text.size() }; + rpc::request body{}; + rpc::reader reader(body); + boost_code ec{}; + reader.init(text.size(), ec); + BOOST_REQUIRE(!ec); + reader.put(buffer, ec); + BOOST_REQUIRE(!ec); + reader.finish(ec); + BOOST_REQUIRE(ec == code{ error::jsonrpc_requires_method }); +} + BOOST_AUTO_TEST_SUITE_END() #endif // HAVE_SLOW_TESTS