fix: Write a status line for every HTTP status a reply can report

`httpReply` names each status in a switch with no `default:` arm, and the
error table names two statuses that switch does not spell out: 402 for
`highFee` and 502 for `dbDeserialization`. A reply reporting either writes
no status line at all, so its first line is a header and the whole thing
is not an HTTP response. A client parsing it reads a protocol error rather
than the error the server meant to report.

Add the arm, taking the reason phrase from beast, which knows the whole
status registry, and drop the `bugprone-switch-missing-default-case`
suppression the omission needed. Eight of the arms above it then spell out
exactly what that arm produces, so they go. Three stay: 401 and 503 report
a phrase of this server's own, and 200 is what every successful reply
carries, so its line stays a compile-time literal rather than a
`std::format` call on the server's most common path.

Both statuses are reachable today through the `ripplerpc: "3.0"` envelope,
which derives the status from the error code. A gtest pins the status line
for every status this server sends, walks the error table, and reads the
placeholder line for a number the registry does not know, so a row added
with a new status cannot reintroduce the defect. A `sign` request whose
fee ceiling is zero reports `highFee` through that envelope, so the server
suite reads the 402 line end to end.

API-CHANGELOG.md

@@ -31,6 +31,7 @@ This version is supported by all `xrpld` versions. For WebSocket and HTTP JSON-R
 - A WebSocket frame that does not parse, or exceeds the request size limit, is answered `{"type": "error", "error": "jsonInvalid", "size": <bytes>}`. The frame's body is reported by size rather than echoed back in a `value` member, since a body that does not parse has no fields to mask. A client that read `value` gets `size` instead.
 - Four error codes that named no HTTP status of their own, and so answered 200 on a reply reporting an error, now name one: `actMalformed`, `alreadyMultisig` and `alreadySingleSig` answer 400, and `actNotFound` answers 404. **No shipped envelope reports these four.** A request sending `ripplerpc: "3.0"` still receives 200 for all four, as it always has, so `account_info` on a malformed account or one the ledger does not hold answers 200 exactly as before.
 - `submit`, `simulate`, `transaction_entry`, `ledger_entry` and `ledger_accept`: Errors from these methods now include `error_code` and `error_message` alongside the `error` token, as every other method already did. Each error now answers the status its code names: 400 for a malformed request, 404 for `transactionNotFound`, 500 for an internal failure, and 501 for `notYetImplemented` and `notStandAlone`. That status change reaches only a request sending `ripplerpc: "3.0"`, which is the envelope that derives the status from the error. With `ripplerpc` `"1.0"` the status stays 200 and the two new members appear beside `error`; with `"2.0"` the status stays 200, `error_code` appears, and the `code` and `message` members carry the code and the message rather than null, since that envelope copies them from `error_code` and `error_message` and drops `error_message`.
+- A reply reporting HTTP 402 or 502 now carries a status line. Those two statuses named no case in the switch that writes one, so such a reply began with a header instead and did not parse as an HTTP response at all. Both are reachable at any API version with `ripplerpc: "3.0"`, which derives the status from the error code: 402 through `highFee` from `sign`, `sign_for` or `submit` with a low `fee_mult_max`, and 502 through `dbDeserialization` from `tx`. The eleven statuses that already named a case report the same phrase they always have.
 
 ## XRP Ledger server version 3.5.0
 

include/xrpl/server/detail/JSONRPCUtil.h

@@ -13,6 +13,8 @@ namespace xrpl {
  * credential this library cannot mask, so the caller logs it masked.
  *
  * A 401 with an empty body is answered with the fixed authentication page.
+ * The status line carries the phrase Beast's registry gives @p nStatus, except
+ * for 401 and 503, which carry a phrase of this server's own.
  *
  * @param nStatus The HTTP status code.
  * @param strMsg The body.

src/libxrpl/server/JSONRPCUtil.cpp

@@ -6,7 +6,10 @@
 #include <xrpl/protocol/BuildInfo.h>
 #include <xrpl/protocol/SystemParameters.h>
 
+#include <boost/beast/http/status.hpp>
+
 #include <ctime>
+#include <format>
 #include <string>
 
 namespace xrpl {
@@ -72,42 +75,29 @@ httpReply(int nStatus, std::string const& content, json::Output const& output, b
         return;
     }
 
-    // NOLINTNEXTLINE(bugprone-switch-missing-default-case)
     switch (nStatus)
     {
+        // The status every successful reply carries, so a literal rather than a format call.
         case 200:
             output("HTTP/1.1 200 OK\r\n");
             break;
-        case 202:
-            output("HTTP/1.1 202 Accepted\r\n");
-            break;
-        case 400:
-            output("HTTP/1.1 400 Bad Request\r\n");
-            break;
+        // Two statuses this server phrases itself rather than taking from the registry.
         case 401:
             output("HTTP/1.1 401 Authorization Required\r\n");
             break;
-        case 403:
-            output("HTTP/1.1 403 Forbidden\r\n");
-            break;
-        case 404:
-            output("HTTP/1.1 404 Not Found\r\n");
-            break;
-        case 405:
-            output("HTTP/1.1 405 Method Not Allowed\r\n");
-            break;
-        case 429:
-            output("HTTP/1.1 429 Too Many Requests\r\n");
-            break;
-        case 500:
-            output("HTTP/1.1 500 Internal Server Error\r\n");
-            break;
-        case 501:
-            output("HTTP/1.1 501 Not Implemented\r\n");
-            break;
         case 503:
             output("HTTP/1.1 503 Server is overloaded\r\n");
             break;
+        default:
+            // A reply whose first line is a header is not an HTTP response. Beast knows the whole
+            // registry, so a status the error table gains needs no case here.
+            output(
+                std::format(
+                    "HTTP/1.1 {} {}\r\n",
+                    nStatus,
+                    boost::beast::http::obsolete_reason(
+                        static_cast<boost::beast::http::status>(nStatus))));
+            break;
     }
 
     output(getHTTPHeaderTimestamp());

src/test/server/ServerStatus_test.cpp

@@ -5,6 +5,7 @@
 #include <test/jtx/WSClient.h>
 #include <test/jtx/amount.h>
 #include <test/jtx/envconfig.h>
+#include <test/jtx/pay.h>
 
 #include <xrpld/app/ledger/LedgerMaster.h>
 #include <xrpld/rpc/detail/MaskSecrets.h>
@@ -1497,6 +1498,54 @@ class ServerStatus_test : public beast::unit_test::Suite, public beast::test::En
         }
     }
 
+    /**
+     * Every reply begins with a status line, including one whose status the
+     * status-line switch does not spell out. `highFee` reports 402, one of
+     * the two statuses the switch names no case for, and a reply that begins
+     * with a header instead is not an HTTP response at all.
+     *
+     * @param yield The coroutine the request runs on.
+     */
+    void
+    testUncommonHttpStatus(boost::asio::yield_context& yield)
+    {
+        testcase("A reply names an HTTP status the status-line switch does not spell out");
+
+        using namespace test::jtx;
+        Env env{*this, envconfig([](std::unique_ptr<Config> cfg) {
+                    cfg->loadFromString(std::string("[") + Sections::kSigningSupport + "]\ntrue");
+                    return cfg;
+                })};
+
+        Account const alice{"alice"};
+        Account const bob{"bob"};
+        env.fund(XRP(10000), alice, bob);
+        env.close();
+
+        // A fee ceiling of zero covers no fee at all, which is what `highFee` reports. The
+        // `ripplerpc: "3.0"` envelope derives the HTTP status from the code.
+        json::Value params(json::ValueType::Object);
+        params[jss::ripplerpc] = rpc::kRippleRpcVersion3;
+        params[jss::secret] = toBase58(generateSeed("alice"));
+        params[jss::fee_mult_max] = 0;
+        params[jss::tx_json] = pay(alice, bob, XRP(1));
+
+        json::Value jv;
+        jv[jss::method] = "sign";
+        jv[jss::params] = json::ValueType::Array;
+        jv[jss::params][0u] = params;
+
+        Response resp;
+        boost::system::error_code ec;
+        auto const reply = postAndParse(env, yield, resp, ec, to_string(jv));
+
+        // The reply parsed as a response, which is what a missing status line breaks.
+        BEAST_EXPECT(!ec);
+        BEAST_EXPECT(resp.result_int() == rpc::errorCodeHttpStatus(RpcHighFee));
+        BEAST_EXPECT(resp.result() == boost::beast::http::status::payment_required);
+        BEAST_EXPECT(reply[jss::error][jss::error] == "highFee");
+    }
+
     /**
      * A credential the server echoes back is masked, on every path that echoes.
      *
@@ -1991,6 +2040,7 @@ public:
             testNoCredentialReachesTheLogAtTrace(yield);
             testHandlerErrorsCarryCodes(yield);
             testGainedStatusesStayOffLegacyEnvelope(yield);
+            testUncommonHttpStatus(yield);
             testStatusNotOkay(yield);
         });
 

src/tests/libxrpl/server/JSONRPCUtil.cpp

@@ -0,0 +1,105 @@
+#include <xrpl/server/detail/JSONRPCUtil.h>
+
+#include <xrpl/beast/utility/Journal.h>
+#include <xrpl/protocol/ErrorCodes.h>
+
+#include <gtest/gtest.h>
+
+#include <set>
+#include <string>
+#include <string_view>
+
+using namespace xrpl;
+
+namespace {
+
+/**
+ * The reply httpReply writes, collected as one string.
+ *
+ * @param status The HTTP status to write a reply for.
+ * @return The whole reply, headers and body.
+ */
+std::string
+reply(int status)
+{
+    std::string out;
+    beast::Journal const journal{beast::Journal::getNullSink()};
+    httpReply(status, "{}", [&out](std::string_view s) { out += s; }, journal);
+    return out;
+}
+
+/**
+ * The reply's status line, without the trailing CRLF.
+ *
+ * @param reply A whole HTTP reply.
+ * @return The first line, or the whole reply when it holds no CRLF.
+ */
+std::string_view
+statusLine(std::string const& reply)
+{
+    auto const end = reply.find("\r\n");
+    return std::string_view{reply}.substr(0, end == std::string::npos ? reply.size() : end);
+}
+
+}  // namespace
+
+TEST(JSONRPCUtil, status_line_names_the_status)
+{
+    // Every status this server sends but the two the last test names. The phrase comes from the
+    // registry but for the two below.
+    EXPECT_EQ(statusLine(reply(200)), "HTTP/1.1 200 OK");
+    EXPECT_EQ(statusLine(reply(202)), "HTTP/1.1 202 Accepted");
+    EXPECT_EQ(statusLine(reply(400)), "HTTP/1.1 400 Bad Request");
+    EXPECT_EQ(statusLine(reply(403)), "HTTP/1.1 403 Forbidden");
+    EXPECT_EQ(statusLine(reply(404)), "HTTP/1.1 404 Not Found");
+    EXPECT_EQ(statusLine(reply(405)), "HTTP/1.1 405 Method Not Allowed");
+    EXPECT_EQ(statusLine(reply(429)), "HTTP/1.1 429 Too Many Requests");
+    EXPECT_EQ(statusLine(reply(500)), "HTTP/1.1 500 Internal Server Error");
+    EXPECT_EQ(statusLine(reply(501)), "HTTP/1.1 501 Not Implemented");
+
+    // The two whose phrase is this server's own, not the registry's.
+    EXPECT_EQ(statusLine(reply(401)), "HTTP/1.1 401 Authorization Required");
+    EXPECT_EQ(statusLine(reply(503)), "HTTP/1.1 503 Server is overloaded");
+}
+
+TEST(JSONRPCUtil, every_status_the_error_table_names_gets_a_status_line)
+{
+    // httpReply sees a status, not the table, so only this can check every status the table names.
+    std::set<int> statuses;
+    for (int i = RpcBadSyntax; i <= RpcLast; ++i)
+    {
+        auto const& info = rpc::getErrorInfo(static_cast<ErrorCodeI>(i));
+
+        // Gaps in the table name no error, so they report no status of their own.
+        if (info.code == RpcUnknown)
+            continue;
+
+        statuses.insert(info.httpStatus);
+    }
+    EXPECT_FALSE(statuses.empty());
+
+    for (int const status : statuses)
+    {
+        auto const line = std::string{statusLine(reply(status))};
+        auto const expectedPrefix = "HTTP/1.1 " + std::to_string(status) + " ";
+        EXPECT_TRUE(line.starts_with(expectedPrefix)) << "status: " << status << ", line: " << line;
+
+        // A placeholder phrase means the table names something that is not a status.
+        EXPECT_GT(line.size(), expectedPrefix.size()) << "status: " << status;
+        EXPECT_EQ(line.find("unknown-status"), std::string::npos) << "status: " << status;
+    }
+}
+
+TEST(JSONRPCUtil, statuses_without_a_case_take_the_registry_phrase)
+{
+    // The two statuses the error table names and the switch spells no case for.
+    EXPECT_EQ(rpc::errorCodeHttpStatus(RpcHighFee), 402);
+    EXPECT_EQ(statusLine(reply(402)), "HTTP/1.1 402 Payment Required");
+
+    EXPECT_EQ(rpc::errorCodeHttpStatus(RpcDbDeserialization), 502);
+    EXPECT_EQ(statusLine(reply(502)), "HTTP/1.1 502 Bad Gateway");
+
+    // A number the registry does not know still gets a line, with the placeholder phrase the table
+    // test above refuses.
+    EXPECT_EQ(statusLine(reply(999)), "HTTP/1.1 999 <unknown-status>");
+}