Repository: bitcoin/bitcoin
File: src/httprpc.cpp (C++)

Code before:

    try {
        // Parse request
        UniValue valRequest;
        if (!valRequest.read(req->ReadBody()))
            throw JSONRPCError(RPC_PARSE_ERROR, "Parse error");

        // Set the URI
        jreq.URI = req->GetURI();

        std::string strReply;
        bool user_has_whitelist = g_rpc_whitelist.count(jreq.authUser);
        if (!user_has_whitelist && g_rpc_whitelist_default) {
            LogPrintf("RPC User %s not allowed to call any methods\n", jreq.authUser);
            req->WriteReply(HTTP_FORBIDDEN);
            return false;

        // singleton request
        } else if (valRequest.isObject()) {
            jreq.parse(valRequest);
            if (user_has_whitelist && !g_rpc_whitelist[jreq.authUser].count(jreq.strMethod)) {
                LogPrintf("RPC User %s not allowed to call method %s\n", jreq.authUser, jreq.strMethod);
                req->WriteReply(HTTP_FORBIDDEN);
                return false;
            }
            UniValue result = tableRPC.execute(jreq);

            // Send reply
            strReply = JSONRPCReply(result, NullUniValue, jreq.id);

        // array of requests
        } else if (valRequest.isArray()) {
            if (user_has_whitelist) {
                for (unsigned int reqIdx = 0; reqIdx < valRequest.size(); reqIdx++) {
                    if (!valRequest[reqIdx].isObject()) {
                        throw JSONRPCError(RPC_INVALID_REQUEST, "Invalid Request object");
                    } else {
                        const UniValue& request = valRequest[reqIdx].get_obj();
                        // Parse method
                        std::string strMethod = request.find_value("method").get_str();
                        if (!g_rpc_whitelist[jreq.authUser].count(strMethod)) {
                            LogPrintf("RPC User %s not allowed to call method %s\n", jreq.authUser, strMethod);
                            req->WriteReply(HTTP_FORBIDDEN);
                            return false;
                        }
                    }
                }
            }
            strReply = JSONRPCExecBatch(jreq, valRequest.get_array());
        }
        else

Review comment (performance):
In commit "rpc: refactor single/batch requests" (125ef7a61885d2ffc0d32b66cdfb3dced2a4417b)

This change doesn't seem right because `jreq.parse` was just called above on line 196, and now the request is going to be parsed for a second time inside `JSONRPCExecOne` for no reason. I think this could be fixed by just dropping the `jreq.parse` call inside `JSONRPCExecOne`. It looks like later commit 4f22286c4aa65a113c44153847c857075a4225d6 does this anyway, so would be better to drop it up front and not change semantics multiple times. It would also be good to make the `ExecOne` `jreq` parameter const at that point since it should no longer be modifying the request. I would also rename `ExecOne` and `ExecBatch` functions to avoid giving the misleading impression that they are alternatives to each other, when actually they are operating at different levels and do not do the same things. ExecBatch handles errors, ExecOne does not. ExecBatch parses and executes, ExecOne should only execute requests without parsing them. Actually, I would probably just drop the ExecBatch function entirely since it is short and only called in one place. It would be clearer to just inline it, IMO.

Code after:
        req->WriteReply(HTTP_UNAUTHORIZED);
        return false;
    }

    try {
        // Parse request
        UniValue valRequest;
        if (!valRequest.read(req->ReadBody()))
            throw JSONRPCError(RPC_PARSE_ERROR, "Parse error");

        // Set the URI
        jreq.URI = req->GetURI();

        UniValue reply;
        bool user_has_whitelist = g_rpc_whitelist.count(jreq.authUser);
        if (!user_has_whitelist && g_rpc_whitelist_default) {
            LogPrintf("RPC User %s not allowed to call any methods\n", jreq.authUser);
            req->WriteReply(HTTP_FORBIDDEN);
            return false;

        // singleton request
        } else if (valRequest.isObject()) {
            jreq.parse(valRequest);
            if (user_has_whitelist && !g_rpc_whitelist[jreq.authUser].count(jreq.strMethod)) {
                LogPrintf("RPC User %s not allowed to call method %s\n", jreq.authUser, jreq.strMethod);
                req->WriteReply(HTTP_FORBIDDEN);
                return false;
            }

            // Legacy 1.0/1.1 behavior is for failed requests to throw
            // exceptions which return HTTP errors and RPC errors to the client.
            // 2.0 behavior is to catch exceptions and return HTTP success with
            // RPC errors, as long as there is not an actual HTTP server error.
            const bool catch_errors{jreq.m_json_version == JSONRPCVersion::V2};
            reply = JSONRPCExec(jreq, catch_errors);

            if (jreq.IsNotification()) {
                // Even though we do execute notifications, we do not respond to them
                req->WriteReply(HTTP_NO_CONTENT);
                return true;
            }

        // array of requests
        } else if (valRequest.isArray()) {
            // Check authorization for each request's method
            if (user_has_whitelist) {
                for (unsigned int reqIdx = 0; reqIdx < valRequest.size(); reqIdx++) {
                    if (!valRequest[reqIdx].isObject()) {
                        throw JSONRPCError(RPC_INVALID_REQUEST, "Invalid Request object");
                    } else {
                        const UniValue& request = valRequest[reqIdx].get_obj();