highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-06-03 17:54:19 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#26612: refactor: RPC: pass named argument value as string_view

Browse Source 
545ff924ab refactor: use string_view for RPC named argument values (stickies-v)
7727603e44 refactor: reduce unnecessary complexity in ParseNonRFCJSONValue (stickies-v)
1d02e59901 test: add cases to JSON parsing (stickies-v)

Pull request description:

  Inspired by MarcoFalke's [comment](https://github.com/bitcoin/bitcoin/pull/26506#discussion_r1036149426). Main purpose of this PR is to minimize copying (potentially large) RPC named arguments when calling `.substr()` by using `std::string_view` instead of `std::string`. Furthermore, cleans up the code by removing unnecessary complexity in `ParseNonRFCJSONValue()` (done first to avoid refactoring required to concatenate `string` and `string_view`), updates some naming and adds a few test cases. Should not introduce any behaviour change.

  ## Questions
  - ~Was there actually any merit to `ParseNonRFCJSONValue()` surrounding the value with brackets and then parsing it as an array? I don't see it, and the new approach doesn't fail any tests. Still a bit suspicious about it though.~
    - Cleared up by https://github.com/bitcoin/bitcoin/pull/26506#pullrequestreview-1211984059
    - If there are no objections to 7727603e44, I think we should follow up with a PR to rename `ParseNonRFCJSONValue()` to a local `Parse()` helper function (that throws if invalid), remove it from `client.h` and merge the test coverage we currently have on `ParseNonRFCJSONValue()` with the coverage we have on `UniValue::read()`.

ACKs for top commit:
  ryanofsky:
    Code review ACK 545ff924ab
  MarcoFalke:
    review ACK 545ff924ab 📻

Tree-SHA512: b1c89fb010ac9c3054b023cac1acbba2a539a09cf39a7baffbd7f7571ee268d5a6d98701c7ac10d68a814526e8fd0fe96ac1d1fb072f272033e415b753f64a5c
This commit is contained in:
fanquake
2023-03-03 15:23:10 +01:00
parent a12b27a2a6 545ff924ab
commit 3b88c85025
4 changed files with 31 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
src/test/rpc_tests.cpp
View File

@@ -289,6 +289,10 @@ BOOST_AUTO_TEST_CASE(json_parse_errors)
{
// Valid
BOOST_CHECK_EQUAL(ParseNonRFCJSONValue("1.0").get_real(), 1.0);
BOOST_CHECK_EQUAL(ParseNonRFCJSONValue("true").get_bool(), true);
BOOST_CHECK_EQUAL(ParseNonRFCJSONValue("[false]")[0].get_bool(), false);
BOOST_CHECK_EQUAL(ParseNonRFCJSONValue("{\"a\": true}")["a"].get_bool(), true);
BOOST_CHECK_EQUAL(ParseNonRFCJSONValue("{\"1\": \"true\"}")["1"].get_str(), "true");
// Valid, with leading or trailing whitespace
BOOST_CHECK_EQUAL(ParseNonRFCJSONValue(" 1.0").get_real(), 1.0);
BOOST_CHECK_EQUAL(ParseNonRFCJSONValue("1.0 ").get_real(), 1.0);
@@ -301,6 +305,11 @@ BOOST_AUTO_TEST_CASE(json_parse_errors)
// Invalid, trailing garbage
BOOST_CHECK_THROW(ParseNonRFCJSONValue("1.0sds"), std::runtime_error);
BOOST_CHECK_THROW(ParseNonRFCJSONValue("1.0]"), std::runtime_error);
// Invalid, keys have to be names
BOOST_CHECK_THROW(ParseNonRFCJSONValue("{1: \"true\"}"), std::runtime_error);
BOOST_CHECK_THROW(ParseNonRFCJSONValue("{true: 1}"), std::runtime_error);
BOOST_CHECK_THROW(ParseNonRFCJSONValue("{[1]: 1}"), std::runtime_error);
BOOST_CHECK_THROW(ParseNonRFCJSONValue("{{\"a\": \"a\"}: 1}"), std::runtime_error);
// BTC addresses should fail parsing
BOOST_CHECK_THROW(ParseNonRFCJSONValue("175tWpb8K1S7NmH4Zx6rewF9WQrcZv245W"), std::runtime_error);
BOOST_CHECK_THROW(ParseNonRFCJSONValue("3J98t1WpEZ73CNmQviecrnyiWrnqRhWNL"), std::runtime_error);