From e17151ad2a45599e8cad90552ffcd979730b7a32 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 6 Aug 2014 13:01:49 +0200 Subject: [PATCH 1/2] Avoid a copy in RPC output Split up HTTPReply into HTTPReply and HTTPReplyHeader, so that the message data can be streamed directly. Also removes a c_str(), which would have prevented binary output with NUL characters in it. --- src/rpcprotocol.cpp | 23 +++++++++++++++-------- src/rpcprotocol.h | 2 ++ src/rpcserver.cpp | 2 +- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/rpcprotocol.cpp b/src/rpcprotocol.cpp index 9e18ca847..48c6c68eb 100644 --- a/src/rpcprotocol.cpp +++ b/src/rpcprotocol.cpp @@ -93,8 +93,7 @@ string HTTPError(int nStatus, bool keepalive, bool headersOnly) headersOnly, "text/plain"); } -string HTTPReply(int nStatus, const string& strMsg, bool keepalive, - bool headersOnly, const char *contentType) +string HTTPReplyHeader(int nStatus, bool keepalive, size_t contentLength, const char *contentType) { return strprintf( "HTTP/1.1 %d %s\r\n" @@ -103,17 +102,25 @@ string HTTPReply(int nStatus, const string& strMsg, bool keepalive, "Content-Length: %u\r\n" "Content-Type: %s\r\n" "Server: bitcoin-json-rpc/%s\r\n" - "\r\n" - "%s", + "\r\n", nStatus, httpStatusDescription(nStatus), rfc1123Time(), keepalive ? "keep-alive" : "close", - (headersOnly ? 0 : strMsg.size()), + contentLength, contentType, - FormatFullVersion(), - (headersOnly ? "" : strMsg.c_str()) - ); + FormatFullVersion()); +} + +string HTTPReply(int nStatus, const string& strMsg, bool keepalive, + bool headersOnly, const char *contentType) +{ + if (headersOnly) + { + return HTTPReplyHeader(nStatus, keepalive, 0, contentType); + } else { + return HTTPReplyHeader(nStatus, keepalive, strMsg.size(), contentType) + strMsg; + } } bool ReadHTTPRequestLine(std::basic_istream& stream, int &proto, diff --git a/src/rpcprotocol.h b/src/rpcprotocol.h index 5627077bf..a088c379d 100644 --- a/src/rpcprotocol.h +++ b/src/rpcprotocol.h @@ -143,6 +143,8 @@ private: std::string HTTPPost(const std::string& strMsg, const std::map& mapRequestHeaders); std::string HTTPError(int nStatus, bool keepalive, bool headerOnly = false); +std::string HTTPReplyHeader(int nStatus, bool keepalive, size_t contentLength, + const char *contentType = "application/json"); std::string HTTPReply(int nStatus, const std::string& strMsg, bool keepalive, bool headerOnly = false, const char *contentType = "application/json"); diff --git a/src/rpcserver.cpp b/src/rpcserver.cpp index 716a7fba6..d9a664dc5 100644 --- a/src/rpcserver.cpp +++ b/src/rpcserver.cpp @@ -862,7 +862,7 @@ static bool HTTPReq_JSONRPC(AcceptedConnection *conn, else throw JSONRPCError(RPC_PARSE_ERROR, "Top-level object parse error"); - conn->stream() << HTTPReply(HTTP_OK, strReply, fRun) << std::flush; + conn->stream() << HTTPReplyHeader(HTTP_OK, fRun, strReply.size()) << strReply << std::flush; } catch (Object& objError) { From 733177ebd3ecf3a03c2acb6b244c8b3d1b4a3981 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 6 Aug 2014 13:03:58 +0200 Subject: [PATCH 2/2] Remove size limit in RPC client, keep it in server The size limit makes a lot of sense for the server, as it never has to accept very large data. The client, however, can request arbitrary amounts of data with `listtransactions` on a large wallet. Fixes #4604. --- src/bitcoin-cli.cpp | 2 +- src/rpcprotocol.cpp | 4 ++-- src/rpcprotocol.h | 2 +- src/rpcserver.cpp | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 3b991f927..0609adcab 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -126,7 +126,7 @@ Object CallRPC(const string& strMethod, const Array& params) // Receive HTTP reply message headers and body map mapHeaders; string strReply; - ReadHTTPMessage(stream, mapHeaders, strReply, nProto); + ReadHTTPMessage(stream, mapHeaders, strReply, nProto, std::numeric_limits::max()); if (nStatus == HTTP_UNAUTHORIZED) throw runtime_error("incorrect rpcuser or rpcpassword (authorization failed)"); diff --git a/src/rpcprotocol.cpp b/src/rpcprotocol.cpp index 48c6c68eb..643208b3b 100644 --- a/src/rpcprotocol.cpp +++ b/src/rpcprotocol.cpp @@ -201,14 +201,14 @@ int ReadHTTPHeaders(std::basic_istream& stream, map& mapHe int ReadHTTPMessage(std::basic_istream& stream, map& mapHeadersRet, string& strMessageRet, - int nProto) + int nProto, size_t max_size) { mapHeadersRet.clear(); strMessageRet = ""; // Read header int nLen = ReadHTTPHeaders(stream, mapHeadersRet); - if (nLen < 0 || nLen > (int)MAX_SIZE) + if (nLen < 0 || (size_t)nLen > max_size) return HTTP_INTERNAL_SERVER_ERROR; // Read message diff --git a/src/rpcprotocol.h b/src/rpcprotocol.h index a088c379d..8f05c0848 100644 --- a/src/rpcprotocol.h +++ b/src/rpcprotocol.h @@ -153,7 +153,7 @@ bool ReadHTTPRequestLine(std::basic_istream& stream, int &proto, int ReadHTTPStatus(std::basic_istream& stream, int &proto); int ReadHTTPHeaders(std::basic_istream& stream, std::map& mapHeadersRet); int ReadHTTPMessage(std::basic_istream& stream, std::map& mapHeadersRet, - std::string& strMessageRet, int nProto); + std::string& strMessageRet, int nProto, size_t max_size); std::string JSONRPCRequest(const std::string& strMethod, const json_spirit::Array& params, const json_spirit::Value& id); json_spirit::Object JSONRPCReplyObj(const json_spirit::Value& result, const json_spirit::Value& error, const json_spirit::Value& id); std::string JSONRPCReply(const json_spirit::Value& result, const json_spirit::Value& error, const json_spirit::Value& id); diff --git a/src/rpcserver.cpp b/src/rpcserver.cpp index d9a664dc5..e7ed73310 100644 --- a/src/rpcserver.cpp +++ b/src/rpcserver.cpp @@ -891,7 +891,7 @@ void ServiceConnection(AcceptedConnection *conn) break; // Read HTTP message headers and body - ReadHTTPMessage(conn->stream(), mapHeaders, strRequest, nProto); + ReadHTTPMessage(conn->stream(), mapHeaders, strRequest, nProto, MAX_SIZE); // HTTP Keep-Alive is false; close connection immediately if (mapHeaders["connection"] == "close")