From c912e22db08d0a44ad6fd027c09bbdf79c34dbbc Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Wed, 4 Jun 2014 11:24:43 -0400 Subject: [PATCH 1/3] RPC cleanup: Improve HTTP server replies 1) support varying content types 2) support only sending the header 3) properly deliver error message as content, if HTTP error 4) move AcceptedConnection class to header, for wider use --- src/rpcprotocol.cpp | 17 ++++++++++++++--- src/rpcprotocol.h | 14 +++++++++++++- src/rpcserver.cpp | 10 ---------- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/src/rpcprotocol.cpp b/src/rpcprotocol.cpp index 2718f8178..bfa799f84 100644 --- a/src/rpcprotocol.cpp +++ b/src/rpcprotocol.cpp @@ -54,7 +54,8 @@ static string rfc1123Time() return DateTimeStrFormat("%a, %d %b %Y %H:%M:%S +0000", GetTime()); } -string HTTPReply(int nStatus, const string& strMsg, bool keepalive) +string HTTPReply(int nStatus, const string& strMsg, bool keepalive, + bool headersOnly, const char *contentType) { if (nStatus == HTTP_UNAUTHORIZED) return strprintf("HTTP/1.0 401 Authorization Required\r\n" @@ -73,6 +74,7 @@ string HTTPReply(int nStatus, const string& strMsg, bool keepalive) "\r\n" "

401 Unauthorized.

\r\n" "\r\n", rfc1123Time(), FormatFullVersion()); + const char *cStatus; if (nStatus == HTTP_OK) cStatus = "OK"; else if (nStatus == HTTP_BAD_REQUEST) cStatus = "Bad Request"; @@ -80,12 +82,19 @@ string HTTPReply(int nStatus, const string& strMsg, bool keepalive) else if (nStatus == HTTP_NOT_FOUND) cStatus = "Not Found"; else if (nStatus == HTTP_INTERNAL_SERVER_ERROR) cStatus = "Internal Server Error"; else cStatus = ""; + + bool useInternalContent = false; + if (nStatus != HTTP_OK) { + contentType = "text/plain"; + useInternalContent = true; + } + return strprintf( "HTTP/1.1 %d %s\r\n" "Date: %s\r\n" "Connection: %s\r\n" "Content-Length: %u\r\n" - "Content-Type: application/json\r\n" + "Content-Type: %s\r\n" "Server: bitcoin-json-rpc/%s\r\n" "\r\n" "%s", @@ -94,8 +103,10 @@ string HTTPReply(int nStatus, const string& strMsg, bool keepalive) rfc1123Time(), keepalive ? "keep-alive" : "close", strMsg.size(), + contentType, FormatFullVersion(), - strMsg); + headersOnly ? "" : + useInternalContent ? cStatus : strMsg.c_str()); } bool ReadHTTPRequestLine(std::basic_istream& stream, int &proto, diff --git a/src/rpcprotocol.h b/src/rpcprotocol.h index 11bdd171d..8d415efb1 100644 --- a/src/rpcprotocol.h +++ b/src/rpcprotocol.h @@ -71,6 +71,16 @@ enum RPCErrorCode RPC_WALLET_ALREADY_UNLOCKED = -17, // Wallet is already unlocked }; +class AcceptedConnection +{ +public: + virtual ~AcceptedConnection() {} + + virtual std::iostream& stream() = 0; + virtual std::string peer_address_to_string() const = 0; + virtual void close() = 0; +}; + // // IOStream device that speaks SSL but can also speak non-SSL // @@ -141,7 +151,9 @@ private: }; std::string HTTPPost(const std::string& strMsg, const std::map& mapRequestHeaders); -std::string HTTPReply(int nStatus, const std::string& strMsg, bool keepalive); +std::string HTTPReply(int nStatus, const std::string& strMsg, bool keepalive, + bool headerOnly = false, + const char *contentType = "application/json"); bool ReadHTTPRequestLine(std::basic_istream& stream, int &proto, std::string& http_method, std::string& http_uri); int ReadHTTPStatus(std::basic_istream& stream, int &proto); diff --git a/src/rpcserver.cpp b/src/rpcserver.cpp index 6552de8c4..b85b17c81 100644 --- a/src/rpcserver.cpp +++ b/src/rpcserver.cpp @@ -393,16 +393,6 @@ bool ClientAllowed(const boost::asio::ip::address& address) return false; } -class AcceptedConnection -{ -public: - virtual ~AcceptedConnection() {} - - virtual std::iostream& stream() = 0; - virtual std::string peer_address_to_string() const = 0; - virtual void close() = 0; -}; - template class AcceptedConnectionImpl : public AcceptedConnection { From 854d013012c2d457d5296227d212b053cbea5239 Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Wed, 4 Jun 2014 11:38:33 -0400 Subject: [PATCH 2/3] RPC code movement: separate out JSON-RPC execution logic from HTTP server logic --- src/rpcserver.cpp | 129 ++++++++++++++++++++++++++-------------------- 1 file changed, 72 insertions(+), 57 deletions(-) diff --git a/src/rpcserver.cpp b/src/rpcserver.cpp index b85b17c81..49bc05e5d 100644 --- a/src/rpcserver.cpp +++ b/src/rpcserver.cpp @@ -809,6 +809,71 @@ static string JSONRPCExecBatch(const Array& vReq) return write_string(Value(ret), false) + "\n"; } +static bool HTTPReq_JSONRPC(AcceptedConnection *conn, + string& strRequest, + map& mapHeaders, + bool fRun) +{ + // Check authorization + if (mapHeaders.count("authorization") == 0) + { + conn->stream() << HTTPReply(HTTP_UNAUTHORIZED, "", false) << std::flush; + return false; + } + + if (!HTTPAuthorized(mapHeaders)) + { + LogPrintf("ThreadRPCServer incorrect password attempt from %s\n", conn->peer_address_to_string()); + /* Deter brute-forcing short passwords. + If this results in a DoS the user really + shouldn't have their RPC port exposed. */ + if (mapArgs["-rpcpassword"].size() < 20) + MilliSleep(250); + + conn->stream() << HTTPReply(HTTP_UNAUTHORIZED, "", false) << std::flush; + return false; + } + + JSONRequest jreq; + try + { + // Parse request + Value valRequest; + if (!read_string(strRequest, valRequest)) + throw JSONRPCError(RPC_PARSE_ERROR, "Parse error"); + + string strReply; + + // singleton request + if (valRequest.type() == obj_type) { + jreq.parse(valRequest); + + Value result = tableRPC.execute(jreq.strMethod, jreq.params); + + // Send reply + strReply = JSONRPCReply(result, Value::null, jreq.id); + + // array of requests + } else if (valRequest.type() == array_type) + strReply = JSONRPCExecBatch(valRequest.get_array()); + else + throw JSONRPCError(RPC_PARSE_ERROR, "Top-level object parse error"); + + conn->stream() << HTTPReply(HTTP_OK, strReply, fRun) << std::flush; + } + catch (Object& objError) + { + ErrorReply(conn->stream(), objError, jreq.id); + return false; + } + catch (std::exception& e) + { + ErrorReply(conn->stream(), JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id); + return false; + } + return true; +} + void ServiceConnection(AcceptedConnection *conn) { bool fRun = true; @@ -825,67 +890,17 @@ void ServiceConnection(AcceptedConnection *conn) // Read HTTP message headers and body ReadHTTPMessage(conn->stream(), mapHeaders, strRequest, nProto); - if (strURI != "/") { - conn->stream() << HTTPReply(HTTP_NOT_FOUND, "", false) << std::flush; - break; - } - - // Check authorization - if (mapHeaders.count("authorization") == 0) - { - conn->stream() << HTTPReply(HTTP_UNAUTHORIZED, "", false) << std::flush; - break; - } - if (!HTTPAuthorized(mapHeaders)) - { - LogPrintf("ThreadRPCServer incorrect password attempt from %s\n", conn->peer_address_to_string()); - /* Deter brute-forcing short passwords. - If this results in a DoS the user really - shouldn't have their RPC port exposed. */ - if (mapArgs["-rpcpassword"].size() < 20) - MilliSleep(250); - - conn->stream() << HTTPReply(HTTP_UNAUTHORIZED, "", false) << std::flush; - break; - } + // HTTP Keep-Alive is false; close connection immediately if (mapHeaders["connection"] == "close") fRun = false; - JSONRequest jreq; - try - { - // Parse request - Value valRequest; - if (!read_string(strRequest, valRequest)) - throw JSONRPCError(RPC_PARSE_ERROR, "Parse error"); - - string strReply; - - // singleton request - if (valRequest.type() == obj_type) { - jreq.parse(valRequest); - - Value result = tableRPC.execute(jreq.strMethod, jreq.params); - - // Send reply - strReply = JSONRPCReply(result, Value::null, jreq.id); - - // array of requests - } else if (valRequest.type() == array_type) - strReply = JSONRPCExecBatch(valRequest.get_array()); - else - throw JSONRPCError(RPC_PARSE_ERROR, "Top-level object parse error"); - - conn->stream() << HTTPReply(HTTP_OK, strReply, fRun) << std::flush; - } - catch (Object& objError) - { - ErrorReply(conn->stream(), objError, jreq.id); - break; + if (strURI == "/") { + if (!HTTPReq_JSONRPC(conn, strRequest, mapHeaders, fRun)) + break; } - catch (std::exception& e) - { - ErrorReply(conn->stream(), JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id); + + else { + conn->stream() << HTTPReply(HTTP_NOT_FOUND, "", false) << std::flush; break; } } From ed5769f536f663a8deb8e8b7a68681cebaa52bdd Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Fri, 27 Jun 2014 00:10:53 -0400 Subject: [PATCH 3/3] Move AcceptedConnection class to rpcserver.h. Also, add parens to HTTPReply() to assist readability. --- src/rpcprotocol.cpp | 4 ++-- src/rpcprotocol.h | 10 ---------- src/rpcserver.h | 10 ++++++++++ 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/rpcprotocol.cpp b/src/rpcprotocol.cpp index bfa799f84..2cb4a35c4 100644 --- a/src/rpcprotocol.cpp +++ b/src/rpcprotocol.cpp @@ -105,8 +105,8 @@ string HTTPReply(int nStatus, const string& strMsg, bool keepalive, strMsg.size(), contentType, FormatFullVersion(), - headersOnly ? "" : - useInternalContent ? cStatus : strMsg.c_str()); + (headersOnly ? "" : + (useInternalContent ? cStatus : strMsg.c_str()))); } bool ReadHTTPRequestLine(std::basic_istream& stream, int &proto, diff --git a/src/rpcprotocol.h b/src/rpcprotocol.h index 8d415efb1..f1317e9c2 100644 --- a/src/rpcprotocol.h +++ b/src/rpcprotocol.h @@ -71,16 +71,6 @@ enum RPCErrorCode RPC_WALLET_ALREADY_UNLOCKED = -17, // Wallet is already unlocked }; -class AcceptedConnection -{ -public: - virtual ~AcceptedConnection() {} - - virtual std::iostream& stream() = 0; - virtual std::string peer_address_to_string() const = 0; - virtual void close() = 0; -}; - // // IOStream device that speaks SSL but can also speak non-SSL // diff --git a/src/rpcserver.h b/src/rpcserver.h index 1966a65b5..fcd293663 100644 --- a/src/rpcserver.h +++ b/src/rpcserver.h @@ -21,6 +21,16 @@ class CBlockIndex; class CNetAddr; +class AcceptedConnection +{ +public: + virtual ~AcceptedConnection() {} + + virtual std::iostream& stream() = 0; + virtual std::string peer_address_to_string() const = 0; + virtual void close() = 0; +}; + /* Start RPC threads */ void StartRPCThreads(); /* Alternative to StartRPCThreads for the GUI, when no server is