From 69e9531181566de1306d500ff62b0fd6ae9373c7 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Wed, 15 Sep 2021 19:06:35 +0100 Subject: [PATCH 1/3] Add JSON tests for unicode, all types, and conformance to ECMA-262/ECMA-404 Also avoid using toStyledString --- test/libsolidity/SolidityNatspecJSON.cpp | 8 +- test/libsolutil/JSON.cpp | 93 ++++++++++++++++++++++-- 2 files changed, 90 insertions(+), 11 deletions(-) diff --git a/test/libsolidity/SolidityNatspecJSON.cpp b/test/libsolidity/SolidityNatspecJSON.cpp index b295b8091..6ee55d586 100644 --- a/test/libsolidity/SolidityNatspecJSON.cpp +++ b/test/libsolidity/SolidityNatspecJSON.cpp @@ -63,8 +63,8 @@ public: BOOST_CHECK_MESSAGE( expectedDocumentation == generatedDocumentation, - "Expected:\n" << expectedDocumentation.toStyledString() << - "\n but got:\n" << generatedDocumentation.toStyledString() + "Expected:\n" << util::jsonPrettyPrint(expectedDocumentation) << + "\n but got:\n" << util::jsonPrettyPrint(generatedDocumentation) ); } @@ -2089,8 +2089,8 @@ BOOST_AUTO_TEST_CASE(dev_explicit_inehrit_complex) BOOST_CHECK_MESSAGE( expectedDocumentation == generatedDocumentation, - "Expected:\n" << expectedDocumentation.toStyledString() << - "\n but got:\n" << generatedDocumentation.toStyledString() + "Expected:\n" << util::jsonPrettyPrint(expectedDocumentation) << + "\n but got:\n" << util::jsonPrettyPrint(generatedDocumentation) ); } diff --git a/test/libsolutil/JSON.cpp b/test/libsolutil/JSON.cpp index 666397877..1b0ed9388 100644 --- a/test/libsolutil/JSON.cpp +++ b/test/libsolutil/JSON.cpp @@ -33,6 +33,48 @@ namespace solidity::util::test BOOST_AUTO_TEST_SUITE(JsonTest, *boost::unit_test::label("nooptions")) +BOOST_AUTO_TEST_CASE(json_types) +{ + auto check = [](Json::Value value, string const& expectation) { + BOOST_CHECK(jsonCompactPrint(value) == expectation); + }; + + Json::Value value; + BOOST_CHECK(value.empty()); + value = {}; + BOOST_CHECK(value.empty()); + value = Json::Value(); + BOOST_CHECK(value.empty()); + value = Json::nullValue; + BOOST_CHECK(value.empty()); + + check(value, "null"); + check({}, "null"); + check(Json::Value(), "null"); + check(Json::nullValue, "null"); + check(Json::objectValue, "{}"); + check(Json::arrayValue, "[]"); + check(Json::UInt(1), "1"); + check(Json::UInt(-1), "4294967295"); + check(Json::UInt64(1), "1"); + check(Json::UInt64(-1), "18446744073709551615"); + check(Json::LargestUInt(1), "1"); + check(Json::LargestUInt(-1), "18446744073709551615"); + check(Json::LargestUInt(0xffffffff), "4294967295"); + check(Json::Value("test"), "\"test\""); + check("test", "\"test\""); + check(true, "true"); + + value = Json::objectValue; + value["key"] = "value"; + check(value, "{\"key\":\"value\"}"); + + value = Json::arrayValue; + value.append(1); + value.append(2); + check(value, "[1,2]"); +} + BOOST_AUTO_TEST_CASE(json_pretty_print) { Json::Value json; @@ -43,6 +85,8 @@ BOOST_AUTO_TEST_CASE(json_pretty_print) json["1"] = 1; json["2"] = "2"; json["3"] = jsonChild; + json["4"] = "ऑ ऒ ओ औ क ख"; + json["5"] = "\xff"; BOOST_CHECK( "{\n" @@ -52,7 +96,9 @@ BOOST_AUTO_TEST_CASE(json_pretty_print) " {\n" " \"3.1\": \"3.1\",\n" " \"3.2\": 2\n" - " }\n" + " },\n" + " \"4\": \"\\u0911 \\u0912 \\u0913 \\u0914 \\u0915 \\u0916\",\n" + " \"5\": \"\\ufffd\"\n" "}" == jsonPrettyPrint(json)); } @@ -66,26 +112,32 @@ BOOST_AUTO_TEST_CASE(json_compact_print) json["1"] = 1; json["2"] = "2"; json["3"] = jsonChild; + json["4"] = "ऑ ऒ ओ औ क ख"; + json["5"] = "\xff"; - BOOST_CHECK("{\"1\":1,\"2\":\"2\",\"3\":{\"3.1\":\"3.1\",\"3.2\":2}}" == jsonCompactPrint(json)); + BOOST_CHECK("{\"1\":1,\"2\":\"2\",\"3\":{\"3.1\":\"3.1\",\"3.2\":2},\"4\":\"\\u0911 \\u0912 \\u0913 \\u0914 \\u0915 \\u0916\",\"5\":\"\\ufffd\"}" == jsonCompactPrint(json)); } BOOST_AUTO_TEST_CASE(parse_json_strict) { + // In this test we check conformance against JSON.parse (https://tc39.es/ecma262/multipage/structured-data.html#sec-json.parse) + // and ECMA-404 (https://www.ecma-international.org/publications-and-standards/standards/ecma-404/) + Json::Value json; std::string errors; - // just parse a valid json input + // Just parse a valid json input BOOST_CHECK(jsonParseStrict("{\"1\":1,\"2\":\"2\",\"3\":{\"3.1\":\"3.1\",\"3.2\":2}}", json, &errors)); BOOST_CHECK(json["1"] == 1); BOOST_CHECK(json["2"] == "2"); BOOST_CHECK(json["3"]["3.1"] == "3.1"); BOOST_CHECK(json["3"]["3.2"] == 2); - // trailing garbage is not allowed in strict-mode + // Trailing garbage is not allowed in ECMA-262 BOOST_CHECK(!jsonParseStrict("{\"1\":2,\"2\":\"2\",\"3\":{\"3.1\":\"3.1\",\"3.2\":3}}}}}}}}}}", json, &errors)); - // comments are allowed in strict-mode? - that's strange... + // Comments are not allowed in ECMA-262 + // ... but JSONCPP allows them BOOST_CHECK(jsonParseStrict( "{\"1\":3, // awesome comment\n\"2\":\"2\",\"3\":{\"3.1\":\"3.1\",\"3.2\":5}}", json, &errors )); @@ -94,15 +146,42 @@ BOOST_AUTO_TEST_CASE(parse_json_strict) BOOST_CHECK(json["3"]["3.1"] == "3.1"); BOOST_CHECK(json["3"]["3.2"] == 5); - // root element can only be object or array + // According to ECMA-404 object, array, number, string, true, false, null are allowed + // ... but JSONCPP disallows value types BOOST_CHECK(jsonParseStrict("[]", json, &errors)); + BOOST_CHECK(json.isArray()); BOOST_CHECK(jsonParseStrict("{}", json, &errors)); + BOOST_CHECK(json.isObject()); BOOST_CHECK(!jsonParseStrict("1", json, &errors)); + // BOOST_CHECK(json.isNumeric()); BOOST_CHECK(!jsonParseStrict("\"hello\"", json, &errors)); + // BOOST_CHECK(json.isString()); + BOOST_CHECK(!jsonParseStrict("true", json, &errors)); + // BOOST_CHECK(json.isBool()); + BOOST_CHECK(!jsonParseStrict("null", json, &errors)); + // BOOST_CHECK(json.isNull()); - // non-UTF-8 escapes allowed?? + // Single quotes are also disallowed by ECMA-404 + BOOST_CHECK(!jsonParseStrict("'hello'", json, &errors)); + // BOOST_CHECK(json.isString()); + + // Only string keys in objects are allowed in ECMA-404 + BOOST_CHECK(!jsonParseStrict("{ 42: \"hello\" }", json, &errors)); + + // According to ECMA-404 hex escape sequences are not allowed, only unicode (\uNNNN) and + // a few control characters (\b, \f, \n, \r, \t) + // + // More lenient parsers allow hex escapes as long as they translate to a valid UTF-8 encoding. + // + // ... but JSONCPP allows any hex escapes BOOST_CHECK(jsonParseStrict("[ \"\x80\xec\x80\" ]", json, &errors)); + BOOST_CHECK(json.isArray()); BOOST_CHECK(json[0] == "\x80\xec\x80"); + + // This would be valid more lenient parsers. + BOOST_CHECK(jsonParseStrict("[ \"\xF0\x9F\x98\x8A\" ]", json, &errors)); + BOOST_CHECK(json.isArray()); + BOOST_CHECK(json[0] == "😊"); } BOOST_AUTO_TEST_SUITE_END() From 55c64e3ca1688a45842a442c319c113154a741c4 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Wed, 15 Sep 2021 19:09:03 +0100 Subject: [PATCH 2/3] Always explicitly initialise Json objects --- libevmasm/Assembly.cpp | 8 +++--- libsolidity/interface/ABI.cpp | 17 ++++++------ libsolidity/interface/CompilerStack.cpp | 2 +- libsolidity/interface/Natspec.cpp | 11 +++++--- libsolidity/interface/StandardCompiler.cpp | 31 +++++++++++----------- 5 files changed, 36 insertions(+), 33 deletions(-) diff --git a/libevmasm/Assembly.cpp b/libevmasm/Assembly.cpp index acf655c82..2978fdcdb 100644 --- a/libevmasm/Assembly.cpp +++ b/libevmasm/Assembly.cpp @@ -200,7 +200,7 @@ string Assembly::assemblyString(StringMap const& _sourceCodes) const Json::Value Assembly::createJsonValue(string _name, int _source, int _begin, int _end, string _value, string _jumpType) { - Json::Value value; + Json::Value value{Json::objectValue}; value["name"] = _name; value["source"] = _source; value["begin"] = _begin; @@ -222,8 +222,9 @@ string Assembly::toStringInHex(u256 _value) Json::Value Assembly::assemblyJSON(map const& _sourceIndices) const { Json::Value root; + root[".code"] = Json::arrayValue; - Json::Value& collection = root[".code"] = Json::arrayValue; + Json::Value& collection = root[".code"]; for (AssemblyItem const& i: m_items) { int sourceIndex = -1; @@ -317,7 +318,8 @@ Json::Value Assembly::assemblyJSON(map const& _sourceIndices) if (!m_data.empty() || !m_subs.empty()) { - Json::Value& data = root[".data"] = Json::objectValue; + root[".data"] = Json::objectValue; + Json::Value& data = root[".data"]; for (auto const& i: m_data) if (u256(i.first) >= m_subs.size()) data[toStringInHex((u256)i.first)] = toHex(i.second); diff --git a/libsolidity/interface/ABI.cpp b/libsolidity/interface/ABI.cpp index c4a2dfe5c..cf395ffa0 100644 --- a/libsolidity/interface/ABI.cpp +++ b/libsolidity/interface/ABI.cpp @@ -56,7 +56,7 @@ Json::Value ABI::generate(ContractDefinition const& _contractDef) FunctionType const* externalFunctionType = it.second->interfaceFunctionType(); solAssert(!!externalFunctionType, ""); - Json::Value method; + Json::Value method{Json::objectValue}; method["type"] = "function"; method["name"] = it.second->declaration().name(); method["stateMutability"] = stateMutabilityToString(externalFunctionType->stateMutability()); @@ -80,7 +80,7 @@ Json::Value ABI::generate(ContractDefinition const& _contractDef) FunctionType constrType(*constructor); FunctionType const* externalFunctionType = constrType.interfaceFunctionType(); solAssert(!!externalFunctionType, ""); - Json::Value method; + Json::Value method{Json::objectValue}; method["type"] = "constructor"; method["stateMutability"] = stateMutabilityToString(externalFunctionType->stateMutability()); method["inputs"] = formatTypeList( @@ -96,23 +96,22 @@ Json::Value ABI::generate(ContractDefinition const& _contractDef) { auto const* externalFunctionType = FunctionType(*fallbackOrReceive).interfaceFunctionType(); solAssert(!!externalFunctionType, ""); - Json::Value method; + Json::Value method{Json::objectValue}; method["type"] = TokenTraits::toString(fallbackOrReceive->kind()); method["stateMutability"] = stateMutabilityToString(externalFunctionType->stateMutability()); abi.emplace(std::move(method)); } for (auto const& it: _contractDef.interfaceEvents()) { - Json::Value event; + Json::Value event{Json::objectValue}; event["type"] = "event"; event["name"] = it->name(); event["anonymous"] = it->isAnonymous(); - Json::Value params(Json::arrayValue); + Json::Value params{Json::arrayValue}; for (auto const& p: it->parameters()) { Type const* type = p->annotation().type->interfaceType(false); solAssert(type, ""); - Json::Value input; auto param = formatType(p->name(), *type, *p->annotation().type, false); param["indexed"] = p->isIndexed(); params.append(std::move(param)); @@ -123,7 +122,7 @@ Json::Value ABI::generate(ContractDefinition const& _contractDef) for (ErrorDefinition const* error: _contractDef.interfaceErrors()) { - Json::Value errorJson; + Json::Value errorJson{Json::objectValue}; errorJson["type"] = "error"; errorJson["name"] = error->name(); errorJson["inputs"] = Json::arrayValue; @@ -151,7 +150,7 @@ Json::Value ABI::formatTypeList( bool _forLibrary ) { - Json::Value params(Json::arrayValue); + Json::Value params{Json::arrayValue}; solAssert(_names.size() == _encodingTypes.size(), "Names and types vector size does not match"); solAssert(_names.size() == _solidityTypes.size(), ""); for (unsigned i = 0; i < _names.size(); ++i) @@ -169,7 +168,7 @@ Json::Value ABI::formatType( bool _forLibrary ) { - Json::Value ret; + Json::Value ret{Json::objectValue}; ret["name"] = _name; ret["internalType"] = _solidityType.toString(true); string suffix = (_forLibrary && _encodingType.dataStoredIn(DataLocation::Storage)) ? " storage" : ""; diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index 3b5fb8cb5..c08ba6607 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -1429,7 +1429,7 @@ CompilerStack::Source const& CompilerStack::source(string const& _sourceName) co string CompilerStack::createMetadata(Contract const& _contract, bool _forIR) const { - Json::Value meta; + Json::Value meta{Json::objectValue}; meta["version"] = 1; meta["language"] = m_importedSources ? "SolidityAST" : "Solidity"; meta["compiler"]["version"] = VersionStringStrict; diff --git a/libsolidity/interface/Natspec.cpp b/libsolidity/interface/Natspec.cpp index 402d8e15c..533cbaa3d 100644 --- a/libsolidity/interface/Natspec.cpp +++ b/libsolidity/interface/Natspec.cpp @@ -37,7 +37,7 @@ using namespace solidity::frontend; Json::Value Natspec::userDocumentation(ContractDefinition const& _contractDef) { - Json::Value doc; + Json::Value doc{Json::objectValue}; doc["version"] = Json::Value(c_natspecVersion); doc["kind"] = Json::Value("user"); @@ -50,7 +50,7 @@ Json::Value Natspec::userDocumentation(ContractDefinition const& _contractDef) if (!value.empty()) { // add the constructor, only if we have any documentation to add - Json::Value user; + Json::Value user{Json::objectValue}; user["notice"] = Json::Value(value); doc["methods"]["constructor"] = user; } @@ -90,7 +90,7 @@ Json::Value Natspec::userDocumentation(ContractDefinition const& _contractDef) string value = extractDoc(error->annotation().docTags, "notice"); if (!value.empty()) { - Json::Value errorDoc; + Json::Value errorDoc{Json::objectValue}; errorDoc["notice"] = value; doc["errors"][error->functionType(true)->externalSignature()].append(move(errorDoc)); } @@ -225,7 +225,10 @@ Json::Value Natspec::extractCustomDoc(multimap const& _tags) for (auto const& [tag, value]: _tags) if (boost::starts_with(tag, "custom")) concatenated[tag] += value.content; - Json::Value result; + // We do not want to create an object if there are no custom tags found. + if (concatenated.empty()) + return Json::nullValue; + Json::Value result{Json::objectValue}; for (auto& [tag, value]: concatenated) result[tag] = move(value); return result; diff --git a/libsolidity/interface/StandardCompiler.cpp b/libsolidity/interface/StandardCompiler.cpp index 1031ec7c3..7d354a405 100644 --- a/libsolidity/interface/StandardCompiler.cpp +++ b/libsolidity/interface/StandardCompiler.cpp @@ -59,7 +59,7 @@ Json::Value formatError( Json::Value const& _secondarySourceLocation = Json::Value() ) { - Json::Value error = Json::objectValue; + Json::Value error{Json::objectValue}; error["type"] = _type; error["component"] = _component; error["severity"] = Error::formatErrorSeverityLowercase(_severity); @@ -74,7 +74,7 @@ Json::Value formatError( Json::Value formatFatalError(string const& _type, string const& _message) { - Json::Value output = Json::objectValue; + Json::Value output{Json::objectValue}; output["errors"] = Json::arrayValue; output["errors"].append(formatError(Error::Severity::Error, _type, "general", _message)); return output; @@ -82,23 +82,22 @@ Json::Value formatFatalError(string const& _type, string const& _message) Json::Value formatSourceLocation(SourceLocation const* location) { - Json::Value sourceLocation; - if (location && location->sourceName) - { - sourceLocation["file"] = *location->sourceName; - sourceLocation["start"] = location->start; - sourceLocation["end"] = location->end; - } + if (!location || !location->sourceName) + return Json::nullValue; + Json::Value sourceLocation{Json::objectValue}; + sourceLocation["file"] = *location->sourceName; + sourceLocation["start"] = location->start; + sourceLocation["end"] = location->end; return sourceLocation; } Json::Value formatSecondarySourceLocation(SecondarySourceLocation const* _secondaryLocation) { if (!_secondaryLocation) - return {}; + return Json::nullValue; - Json::Value secondarySourceLocation = Json::arrayValue; + Json::Value secondarySourceLocation{Json::arrayValue}; for (auto const& location: _secondaryLocation->infos) { Json::Value msg = formatSourceLocation(&location.second); @@ -330,7 +329,7 @@ bool isIRRequested(Json::Value const& _outputSelection) Json::Value formatLinkReferences(std::map const& linkReferences) { - Json::Value ret(Json::objectValue); + Json::Value ret{Json::objectValue}; for (auto const& ref: linkReferences) { @@ -345,7 +344,7 @@ Json::Value formatLinkReferences(std::map const& linkRefere Json::Value fileObject = ret.get(file, Json::objectValue); Json::Value libraryArray = fileObject.get(name, Json::arrayValue); - Json::Value entry = Json::objectValue; + Json::Value entry{Json::objectValue}; entry["start"] = Json::UInt(ref.first); entry["length"] = 20; @@ -359,7 +358,7 @@ Json::Value formatLinkReferences(std::map const& linkRefere Json::Value formatImmutableReferences(map>> const& _immutableReferences) { - Json::Value ret(Json::objectValue); + Json::Value ret{Json::objectValue}; for (auto const& immutableReference: _immutableReferences) { @@ -367,7 +366,7 @@ Json::Value formatImmutableReferences(map>> co Json::Value array(Json::arrayValue); for (size_t byteOffset: byteOffsets) { - Json::Value byteRange(Json::objectValue); + Json::Value byteRange{Json::objectValue}; byteRange["start"] = Json::UInt(byteOffset); byteRange["length"] = Json::UInt(32); // immutable references are currently always 32 bytes wide array.append(byteRange); @@ -386,7 +385,7 @@ Json::Value collectEVMObject( function const& _artifactRequested ) { - Json::Value output = Json::objectValue; + Json::Value output{Json::objectValue}; if (_artifactRequested("object")) output["object"] = _object.toHex(); if (_artifactRequested("opcodes")) From b5e68df3cdb1c31bca03646b242e7fcbd175258d Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Wed, 15 Sep 2021 19:18:42 +0100 Subject: [PATCH 3/3] Document JsonFormat --- libsolutil/JSON.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libsolutil/JSON.h b/libsolutil/JSON.h index 3a326a5e3..43dddfd4a 100644 --- a/libsolutil/JSON.h +++ b/libsolutil/JSON.h @@ -38,8 +38,8 @@ struct JsonFormat { enum Format { - Compact, - Pretty + Compact, // No unnecessary whitespace (including new lines and indentation) + Pretty, // Nicely indented, with new lines }; static constexpr uint32_t defaultIndent = 2;