Minor RPC cleanups #4288

pull jgarzik wants to merge 3 commits into bitcoin:master from jgarzik:rest-prep changing 4 files +99 −71

jgarzik commented at 3:59 PM on June 4, 2014: contributor

Some code movement and minor improvements, to prepare for HTTP REST support (#2844).

Even if HTTP REST is delayed or does not go in, these cleanups remain useful.

in src/rpcprotocol.cpp:None in 9c83e708ac outdated

  53 | @@ -54,7 +54,8 @@ static string rfc1123Time()
  54 |      return DateTimeStrFormat("%a, %d %b %Y %H:%M:%S +0000", GetTime());
  55 |  }
  56 |  
  57 | -string HTTPReply(int nStatus, const string& strMsg, bool keepalive)
  58 | +string HTTPReply(int nStatus, const string& strMsg, bool keepalive,
  59 | +                 bool headersOnly, const char *contentType)

Diapolo commented at 8:07 PM on June 4, 2014:

Nit: You may use 4 spaces as identation here.

gavinandresen commented at 12:15 AM on June 5, 2014: contributor

Untested ACK.

in src/rpcprotocol.cpp:None in 9c83e708ac outdated

 102 | @@ -94,8 +103,10 @@ string HTTPReply(int nStatus, const string& strMsg, bool keepalive)
 103 |          rfc1123Time(),
 104 |          keepalive ? "keep-alive" : "close",
 105 |          strMsg.size(),
 106 | +        contentType,
 107 |          FormatFullVersion(),
 108 | -        strMsg);
 109 | +        headersOnly ? "" :

laanwj commented at 4:59 AM on June 5, 2014:

Please use parentheses here. Multiple nested tertiary operators are harder to read, but more so if the grouping is not explicit.

laanwj commented at 5:35 AM on June 5, 2014: member

ACK (apart from the above nit), this is a good step toward making the RPC mechanism more modular

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

c912e22db0

RPC code movement: separate out JSON-RPC execution logic from HTTP server logic 854d013012
Move AcceptedConnection class to rpcserver.h.
Also, add parens to HTTPReply() to assist readability.
ed5769f536

in src/rpcprotocol.h:None in 9c83e708ac outdated

  70 | @@ -71,6 +71,16 @@ enum RPCErrorCode
  71 |      RPC_WALLET_ALREADY_UNLOCKED     = -17, // Wallet is already unlocked
  72 |  };
  73 |  
  74 | +class AcceptedConnection

laanwj commented at 9:46 AM on June 23, 2014:

This belongs in rpcserver.h, as it is specific to the server side.

jgarzik commented at 4:14 AM on June 27, 2014: contributor

Rebased and addressed feedback.
BitcoinPullTester commented at 4:32 AM on June 27, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4288_ed5769f536f663a8deb8e8b7a68681cebaa52bdd/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
laanwj merged this on Jun 27, 2014
laanwj closed this on Jun 27, 2014
laanwj referenced this in commit d77f761be9 on Jun 27, 2014
kazcw referenced this in commit 8cd3c07c27 on Jun 29, 2014
kazcw referenced this in commit 5ad73fd22d on Jun 29, 2014
kazcw referenced this in commit 16f33f163d on Jun 29, 2014
MathyV referenced this in commit 7996b397e2 on Nov 3, 2014
reddink referenced this in commit 24c6096b4d on May 27, 2020
MarcoFalke locked this on Sep 8, 2021

Contributors

Linked (view graph)

#2844 Add unauthenticated HTTP REST interface #4440 fix RPC error replies