From 2306dc4b760df577130337be0a32d5a4014981a1 Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Sun, 4 Nov 2012 16:06:38 -0500 Subject: [PATCH 1/2] RPC, cosmetic: push down ReadHTTPStatus() calls into ReadHTTP() callers ReadHTTPStatus() is currently overloaded: In client mode, it properly parses and receives an HTTP status line. In server mode, it incorrectly parses the HTTP request line as an HTTP status line. This server mode bug has never mattered, because the RPC server never cared about the URI (path) provided in the HTTP request. That will change in the future, so go ahead and begin fixing the problem. This patch is cosmetic, and should result in NO behavior changes. Further renames: ReadHTTPHeader -> ReadHTTPHeaders ReadHTTP -> ReadHTTPMessage --- src/bitcoinrpc.cpp | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/bitcoinrpc.cpp b/src/bitcoinrpc.cpp index 21e37c75e..84f93c433 100644 --- a/src/bitcoinrpc.cpp +++ b/src/bitcoinrpc.cpp @@ -377,7 +377,7 @@ int ReadHTTPStatus(std::basic_istream& stream, int &proto) return atoi(vWords[1].c_str()); } -int ReadHTTPHeader(std::basic_istream& stream, map& mapHeadersRet) +int ReadHTTPHeaders(std::basic_istream& stream, map& mapHeadersRet) { int nLen = 0; loop @@ -402,17 +402,15 @@ int ReadHTTPHeader(std::basic_istream& stream, map& mapHea return nLen; } -int ReadHTTP(std::basic_istream& stream, map& mapHeadersRet, string& strMessageRet) +int ReadHTTPMessage(std::basic_istream& stream, map& mapHeadersRet, string& strMessageRet, + int nProto) { mapHeadersRet.clear(); strMessageRet = ""; - // Read status - int nProto = 0; - int nStatus = ReadHTTPStatus(stream, nProto); - // Read header - int nLen = ReadHTTPHeader(stream, mapHeadersRet); + int nLen = ReadHTTPHeaders(stream, mapHeadersRet); if (nLen < 0 || nLen > (int)MAX_SIZE) return HTTP_INTERNAL_SERVER_ERROR; @@ -434,7 +432,7 @@ int ReadHTTP(std::basic_istream& stream, map& mapHeadersRe mapHeadersRet["connection"] = "close"; } - return nStatus; + return HTTP_OK; } bool HTTPAuthorized(map& mapHeaders) @@ -944,7 +942,12 @@ void ThreadRPCServer3(void* parg) map mapHeaders; string strRequest; - ReadHTTP(conn->stream(), mapHeaders, strRequest); + // Read HTTP status (um, we mean, HTTP request line) + int nProto = 0; + ReadHTTPStatus(conn->stream(), nProto); + + // Read HTTP message headers and body + ReadHTTPMessage(conn->stream(), mapHeaders, strRequest, nProto); // Check authorization if (mapHeaders.count("authorization") == 0) @@ -1076,10 +1079,15 @@ Object CallRPC(const string& strMethod, const Array& params) string strPost = HTTPPost(strRequest, mapRequestHeaders); stream << strPost << std::flush; - // Receive reply + // Receive HTTP reply status + int nProto = 0; + int nStatus = ReadHTTPStatus(stream, nProto); + + // Receive HTTP reply message headers and body map mapHeaders; string strReply; - int nStatus = ReadHTTP(stream, mapHeaders, strReply); + ReadHTTPMessage(stream, mapHeaders, strReply, nProto); + if (nStatus == HTTP_UNAUTHORIZED) throw runtime_error("incorrect rpcuser or rpcpassword (authorization failed)"); else if (nStatus >= 400 && nStatus != HTTP_BAD_REQUEST && nStatus != HTTP_NOT_FOUND && nStatus != HTTP_INTERNAL_SERVER_ERROR) From fcf234fc08e445e2e5b4dfd1a35b10e929940244 Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Sun, 4 Nov 2012 17:16:46 -0500 Subject: [PATCH 2/2] RPC: HTTP server uses its own ReadHTTPRequestLine() rather than reusing ReadHTTPStatus() from the client mode. The following additional HTTP request validations are added, both in line with existing HTTP client practice: 1) HTTP method must be GET or POST. Most clients use POST, some use GET. Either way, this continues to work. 2) HTTP URI must start with "/" character. Normal URI is "/" (a 1-char string), so this is fine. --- src/bitcoinrpc.cpp | 45 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/src/bitcoinrpc.cpp b/src/bitcoinrpc.cpp index 84f93c433..ed51b7afb 100644 --- a/src/bitcoinrpc.cpp +++ b/src/bitcoinrpc.cpp @@ -362,6 +362,41 @@ static string HTTPReply(int nStatus, const string& strMsg, bool keepalive) strMsg.c_str()); } +bool ReadHTTPRequestLine(std::basic_istream& stream, int &proto, + string& http_method, string& http_uri) +{ + string str; + getline(stream, str); + + // HTTP request line is space-delimited + vector vWords; + boost::split(vWords, str, boost::is_any_of(" ")); + if (vWords.size() < 2) + return false; + + // HTTP methods permitted: GET, POST + http_method = vWords[0]; + if (http_method != "GET" && http_method != "POST") + return false; + + // HTTP URI must be an absolute path, relative to current host + http_uri = vWords[1]; + if (http_uri.size() == 0 || http_uri[0] != '/') + return false; + + // parse proto, if present + string strProto = ""; + if (vWords.size() > 2) + strProto = vWords[2]; + + proto = 0; + const char *ver = strstr(strProto.c_str(), "HTTP/1."); + if (ver != NULL) + proto = atoi(ver+7); + + return true; +} + int ReadHTTPStatus(std::basic_istream& stream, int &proto) { string str; @@ -939,12 +974,14 @@ void ThreadRPCServer3(void* parg) } return; } - map mapHeaders; - string strRequest; - // Read HTTP status (um, we mean, HTTP request line) int nProto = 0; - ReadHTTPStatus(conn->stream(), nProto); + map mapHeaders; + string strRequest, strMethod, strURI; + + // Read HTTP request line + if (!ReadHTTPRequestLine(conn->stream(), nProto, strMethod, strURI)) + break; // Read HTTP message headers and body ReadHTTPMessage(conn->stream(), mapHeaders, strRequest, nProto);