From 85938dc0a7f6bc0ef6d14d7bc9ed5d641c247324 Mon Sep 17 00:00:00 2001 From: waynercheung Date: Thu, 24 Sep 2026 19:34:11 +0800 Subject: [PATCH] feat(http): support numeric field aliases in json parsing JsonFormat.mergeField resolves a single-digit key such as "2" to the field with that number, but then marked that field as unknown. For a message-typed field the flag made handleObject read the value as a hex-encoded protobuf message instead of parsing a JSON object, so the two spellings of one field accepted different syntaxes. Drop the flag and the hex branch so a single-digit alias is parsed by JsonFormat exactly like the field name, at any nesting level and for singular, repeated and map fields alike. The alias range is unchanged: only fields 1 to 9 can be addressed by number, and a key that resolves to no field is still skipped. Scalar and bytes aliases, unknown keys, null and empty arrays keep their current behaviour. This applies to protobuf fields only; the wallet endpoints still read raw_data, contract, parameter, value and type as literal JSON keys. Also compute the request url before the rate limiter checks in RateLimiterServlet, and pair the GET setup with the cleanup that follows it by wrapping both in the outer try, leaving the request handling in an inner try with the catch clauses it already had. BREAKING CHANGE: a message-typed field addressed by its field number now accepts JSON objects instead of hex-encoded protobuf strings. Direct JSON-to-protobuf merges reject the old string form with a parse error. During per-contract parsing, Util.packTransaction omits contracts that fail to parse; remaining contracts continue through normal endpoint validation, and if none remain the endpoint's existing empty-contract handling applies. Parse errors during the final transaction merge instead make Util.packTransaction return null, invoking the caller's existing error handling. JSON-RPC ABI parsing rejects this form with "invalid abi". Clients using numeric aliases should send the same JSON objects they would send for field names. --- .../tron/core/services/http/JsonFormat.java | 28 +- .../services/http/RateLimiterServlet.java | 52 +-- .../tron/core/jsonrpc/JsonrpcServiceTest.java | 52 +++ .../http/JsonFormatNumericAliasTest.java | 386 ++++++++++++++++++ .../services/http/RateLimiterServletTest.java | 45 ++ .../org/tron/core/services/http/UtilTest.java | 79 ++++ 6 files changed, 598 insertions(+), 44 deletions(-) create mode 100644 framework/src/test/java/org/tron/core/services/http/JsonFormatNumericAliasTest.java diff --git a/framework/src/main/java/org/tron/core/services/http/JsonFormat.java b/framework/src/main/java/org/tron/core/services/http/JsonFormat.java index 2fa7d9fbb42..29224635155 100644 --- a/framework/src/main/java/org/tron/core/services/http/JsonFormat.java +++ b/framework/src/main/java/org/tron/core/services/http/JsonFormat.java @@ -37,7 +37,6 @@ SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT import com.google.protobuf.Descriptors.EnumValueDescriptor; import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.ExtensionRegistry; -import com.google.protobuf.InvalidProtocolBufferException; import com.google.protobuf.Message; import com.google.protobuf.UnknownFieldSet; import java.io.IOException; @@ -567,7 +566,6 @@ protected static void mergeField(Tokenizer tokenizer, FieldDescriptor field; Descriptor type = builder.getDescriptorForType(); final ExtensionRegistry.ExtensionInfo extension; - boolean unknown = false; String name = tokenizer.consumeIdentifier(); field = type.findFieldByName(name); @@ -591,11 +589,10 @@ protected static void mergeField(Tokenizer tokenizer, field = null; } - // Last try to lookup by field-index if 'name' is numeric, - // which indicates a possible unknown field + // Last try to look the field up by number if 'name' is a single digit. The alias + // resolves to the same descriptor as the field name and is parsed the same way. if (field == null && DIGITS.matcher(name).matches()) { field = type.findFieldByNumber(Integer.parseInt(name)); - unknown = true; } // Finally, look for extensions @@ -627,11 +624,11 @@ protected static void mergeField(Tokenizer tokenizer, if (array) { while (!tokenizer.tryConsume("]")) { - handleValue(tokenizer, extensionRegistry, builder, field, extension, unknown, selfType); + handleValue(tokenizer, extensionRegistry, builder, field, extension, selfType); tokenizer.tryConsume(","); } } else { - handleValue(tokenizer, extensionRegistry, builder, field, extension, unknown, selfType); + handleValue(tokenizer, extensionRegistry, builder, field, extension, selfType); } } } @@ -684,12 +681,11 @@ private static void handleValue(Tokenizer tokenizer, Message.Builder builder, FieldDescriptor field, ExtensionRegistry.ExtensionInfo extension, - boolean unknown, boolean selfType) throws ParseException { + boolean selfType) throws ParseException { Object value = null; if (field.getJavaType() == FieldDescriptor.JavaType.MESSAGE) { - value = handleObject(tokenizer, extensionRegistry, builder, field, extension, unknown, - selfType); + value = handleObject(tokenizer, extensionRegistry, builder, field, extension, selfType); } else { value = handlePrimitive(tokenizer, field, selfType); } @@ -798,7 +794,7 @@ private static Object handleObject(Tokenizer tokenizer, Message.Builder builder, FieldDescriptor field, ExtensionRegistry.ExtensionInfo extension, - boolean unknown, boolean selfType) throws ParseException { + boolean selfType) throws ParseException { Message.Builder subBuilder; if (extension == null) { @@ -807,16 +803,6 @@ private static Object handleObject(Tokenizer tokenizer, subBuilder = extension.defaultInstance.newBuilderForType(); } - if (unknown) { - ByteString data = tokenizer.consumeByteString("", selfType); - try { - subBuilder.mergeFrom(data); - return subBuilder.build(); - } catch (InvalidProtocolBufferException e) { - throw tokenizer.parseException("Failed to build " + field.getFullName() + " from " + data); - } - } - tokenizer.consume("{"); tokenizer.enterRecursion(); try { diff --git a/framework/src/main/java/org/tron/core/services/http/RateLimiterServlet.java b/framework/src/main/java/org/tron/core/services/http/RateLimiterServlet.java index 6f67aba3020..314eb6609cf 100644 --- a/framework/src/main/java/org/tron/core/services/http/RateLimiterServlet.java +++ b/framework/src/main/java/org/tron/core/services/http/RateLimiterServlet.java @@ -107,36 +107,42 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) RuntimeData runtimeData = new RuntimeData(req); IRateLimiter rateLimiter = container.get(KEY_PREFIX_HTTP, getClass().getSimpleName()); + String contextPath = req.getContextPath(); + String url = Strings.isNullOrEmpty(req.getServletPath()) + ? MetricLabels.UNDEFINED : contextPath + req.getServletPath(); + // Check per-endpoint first to avoid consuming global IP/QPS quota for requests // that would be rejected by the per-endpoint limiter anyway. acquirePermit() // chooses blocking or non-blocking semantics based on rate.limiter.apiNonBlocking. boolean perEndpointAcquired = rateLimiter == null || rateLimiter.acquirePermit(runtimeData); boolean acquireResource = perEndpointAcquired && GlobalRateLimiter.acquirePermit(runtimeData); - - String contextPath = req.getContextPath(); - String url = Strings.isNullOrEmpty(req.getServletPath()) - ? MetricLabels.UNDEFINED : contextPath + req.getServletPath(); - // int64_as_string is honored only on GET requests (URL query). POST is intentionally - // unsupported because reading the body here would consume request.getReader() and - // break downstream servlets that read it themselves. - if ("GET".equalsIgnoreCase(req.getMethod())) { - JsonFormat.setInt64AsString(Util.getInt64AsString(req)); - } + // The outer try only pairs the GET setup below with the cleanup in the finally block; it + // deliberately has no catch, so that setup keeps propagating its exceptions as before. + // Everything the inner try holds keeps the catch clauses it already had. try { - resp.setContentType("application/json; charset=utf-8"); - - if (acquireResource) { - Histogram.Timer requestTimer = Metrics.histogramStartTimer( - MetricKeys.Histogram.HTTP_SERVICE_LATENCY, url); - super.service(req, resp); - Metrics.histogramObserve(requestTimer); - } else { - Util.writeAuditedError(Util.RATE_LIMITER_ERROR_MSG, resp); + // int64_as_string is honored only on GET requests (URL query). POST is intentionally + // unsupported because reading the body here would consume request.getReader() and + // break downstream servlets that read it themselves. + if ("GET".equalsIgnoreCase(req.getMethod())) { + JsonFormat.setInt64AsString(Util.getInt64AsString(req)); + } + + try { + resp.setContentType("application/json; charset=utf-8"); + + if (acquireResource) { + Histogram.Timer requestTimer = Metrics.histogramStartTimer( + MetricKeys.Histogram.HTTP_SERVICE_LATENCY, url); + super.service(req, resp); + Metrics.histogramObserve(requestTimer); + } else { + Util.writeAuditedError(Util.RATE_LIMITER_ERROR_MSG, resp); + } + } catch (ServletException | IOException | BadMessageException e) { + throw e; + } catch (Exception unexpected) { + logger.error("Http Api {}, Method:{}. Error：", url, req.getMethod(), unexpected); } - } catch (ServletException | IOException | BadMessageException e) { - throw e; - } catch (Exception unexpected) { - logger.error("Http Api {}, Method:{}. Error：", url, req.getMethod(), unexpected); } finally { // CRITICAL: this clear pairs with the setInt64AsString call above. Removing it // will leak int64_as_string state across requests on reused Tomcat threads, diff --git a/framework/src/test/java/org/tron/core/jsonrpc/JsonrpcServiceTest.java b/framework/src/test/java/org/tron/core/jsonrpc/JsonrpcServiceTest.java index e8d14ace060..c6d02dc71f7 100644 --- a/framework/src/test/java/org/tron/core/jsonrpc/JsonrpcServiceTest.java +++ b/framework/src/test/java/org/tron/core/jsonrpc/JsonrpcServiceTest.java @@ -52,6 +52,7 @@ import org.tron.core.services.interfaceJsonRpcOnPBFT.JsonRpcServiceOnPBFT; import org.tron.core.services.interfaceJsonRpcOnSolidity.JsonRpcServiceOnSolidity; import org.tron.core.services.jsonrpc.FullNodeJsonRpcHttpService; +import org.tron.core.services.jsonrpc.TronJsonRpc; import org.tron.core.services.jsonrpc.TronJsonRpc.FilterRequest; import org.tron.core.services.jsonrpc.TronJsonRpc.LogFilterElement; import org.tron.core.services.jsonrpc.TronJsonRpcImpl; @@ -1558,6 +1559,57 @@ public void testBuildTransactionRejectsDeeplyNestedAbi() { Assert.assertEquals("invalid abi", e.getMessage()); } + private static BuildArguments createSmartContractArgs(String abi, boolean visible) { + BuildArguments args = new BuildArguments(); + args.setFrom("0xabd4b9367799eaa3197fecb144eb71de1e049abc"); + args.setData("608060405234801561001057600080fd5b50"); + args.setGas("0x3b9aca00"); + args.setAbi(abi); + args.setVisible(visible); + return args; + } + + private static JSONObject abiOf(TronJsonRpc.TransactionJson transactionJson) { + JSONArray contracts = transactionJson.getTransaction().getJSONObject("raw_data") + .getJSONArray("contract"); + Assert.assertEquals(1, contracts.size()); + return contracts.getJSONObject(0).getJSONObject("parameter").getJSONObject("value") + .getJSONObject("new_contract").getJSONObject("abi"); + } + + @Test + public void testBuildCreateSmartContractAbiNumericAliasMatchesFieldName() throws Exception { + // ABI.Entry: name = 3, inputs = 4; ABI.Entry.Param: name = 2, type = 3 + String named = "[{\"name\":\"f\",\"inputs\":[{\"name\":\"a\",\"type\":\"uint256\"}]," + + "\"type\":\"function\"}]"; + String alias = "[{\"3\":\"f\",\"4\":[{\"2\":\"a\",\"3\":\"uint256\"}]," + + "\"type\":\"function\"}]"; + + JSONObject fromNamed = abiOf(tronJsonRpc.buildTransaction( + createSmartContractArgs(named, false))); + JSONObject fromAlias = abiOf(tronJsonRpc.buildTransaction( + createSmartContractArgs(alias, false))); + + Assert.assertEquals(fromNamed.toJSONString(), fromAlias.toJSONString()); + JSONObject entry = fromAlias.getJSONArray("entrys").getJSONObject(0); + Assert.assertEquals("f", entry.getString("name")); + Assert.assertEquals("uint256", + entry.getJSONArray("inputs").getJSONObject(0).getString("type")); + } + + @Test + public void testBuildCreateSmartContractAbiRejectsStringForNumericMessageField() { + // ABI.Entry.inputs = 4 is a repeated message field; a string value is not a JSON object, + // whichever address format the request uses. + String abi = "[{\"name\":\"f\",\"4\":\"1201611a0775696e74323536\"}]"; + for (boolean visible : new boolean[] {false, true}) { + JsonRpcInvalidParamsException e = Assert.assertThrows( + JsonRpcInvalidParamsException.class, + () -> tronJsonRpc.buildTransaction(createSmartContractArgs(abi, visible))); + Assert.assertEquals("invalid abi", e.getMessage()); + } + } + @Test public void testWeb3ClientVersion() { try { diff --git a/framework/src/test/java/org/tron/core/services/http/JsonFormatNumericAliasTest.java b/framework/src/test/java/org/tron/core/services/http/JsonFormatNumericAliasTest.java new file mode 100644 index 00000000000..cf68c7f00c3 --- /dev/null +++ b/framework/src/test/java/org/tron/core/services/http/JsonFormatNumericAliasTest.java @@ -0,0 +1,386 @@ +package org.tron.core.services.http; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +import com.google.protobuf.Any; +import com.google.protobuf.ByteString; +import org.junit.Test; +import org.tron.common.utils.ByteArray; +import org.tron.core.Constant; +import org.tron.json.JSON; +import org.tron.protos.Protocol.Permission; +import org.tron.protos.Protocol.Transaction; +import org.tron.protos.contract.AccountContract.AccountPermissionUpdateContract; +import org.tron.protos.contract.AccountContract.AccountUpdateContract; +import org.tron.protos.contract.BalanceContract.TransferContract; +import org.tron.protos.contract.ProposalContract.ProposalCreateContract; + +/* + * A single-digit key resolves to the field with that number. These cases pin down that such a + * numeric alias is parsed exactly like the field name: message fields take a JSON object (never a + * string), scalar and bytes fields are unchanged, and keys that resolve to nothing are still + * skipped. + */ +public class JsonFormatNumericAliasTest { + + private static final String OWNER_HEX = "41d3136787e667d1e055d2cd5db4b5f6c880563049"; + private static final String OWNER_BASE58 = "TVDGpn4hCSzJ5nkHPLetk8KQBtwaTppnkr"; + + private static String permissionJson(String address) { + return "{\"type\":\"Owner\",\"permission_name\":\"owner\",\"threshold\":2," + + "\"keys\":[{\"address\":\"" + address + "\",\"weight\":1},{\"weight\":1}]}"; + } + + private static String keysBody(String ownerKey, int count) { + StringBuilder sb = new StringBuilder("{\"").append(ownerKey).append("\":{\"keys\":["); + for (int i = 0; i < count; i++) { + sb.append(i == 0 ? "{}" : ",{}"); + } + return sb.append("]}}").toString(); + } + + private static void assertRejectedAsObject(String json, boolean visible) { + AccountPermissionUpdateContract.Builder builder = AccountPermissionUpdateContract.newBuilder(); + JsonFormat.ParseException e = assertThrows(JsonFormat.ParseException.class, + () -> JsonFormat.merge(json, builder, visible)); + assertTrue(e.getMessage(), e.getMessage().contains("Expected \"{\".")); + assertFalse(builder.hasOwner()); + assertEquals(0, builder.getActivesCount()); + } + + @Test + public void testNumericAliasParsesLikeFieldName() throws Exception { + AccountPermissionUpdateContract.Builder named = AccountPermissionUpdateContract.newBuilder(); + AccountPermissionUpdateContract.Builder alias = AccountPermissionUpdateContract.newBuilder(); + + JsonFormat.merge("{\"owner_address\":\"" + OWNER_HEX + "\",\"owner\":" + + permissionJson(OWNER_HEX) + "}", named, false); + JsonFormat.merge("{\"1\":\"" + OWNER_HEX + "\",\"2\":" + permissionJson(OWNER_HEX) + "}", + alias, false); + + assertEquals(2, alias.getOwner().getKeysCount()); + assertEquals(named.build(), alias.build()); + } + + @Test + public void testNumericAliasParsesLikeFieldNameWhenVisible() throws Exception { + AccountPermissionUpdateContract.Builder named = AccountPermissionUpdateContract.newBuilder(); + AccountPermissionUpdateContract.Builder alias = AccountPermissionUpdateContract.newBuilder(); + + JsonFormat.merge("{\"owner_address\":\"" + OWNER_BASE58 + "\",\"owner\":" + + permissionJson(OWNER_BASE58) + "}", named, true); + JsonFormat.merge("{\"1\":\"" + OWNER_BASE58 + "\",\"2\":" + permissionJson(OWNER_BASE58) + + "}", alias, true); + + assertEquals(named.build(), alias.build()); + assertEquals(ByteString.copyFrom(ByteArray.fromHexString(OWNER_HEX)), + alias.getOwner().getKeys(0).getAddress()); + } + + @Test + public void testNumericAliasInsideNestedMessage() throws Exception { + AccountPermissionUpdateContract.Builder named = AccountPermissionUpdateContract.newBuilder(); + AccountPermissionUpdateContract.Builder alias = AccountPermissionUpdateContract.newBuilder(); + + JsonFormat.merge("{\"owner\":" + permissionJson(OWNER_HEX) + "}", named, false); + // Permission: type = 1, permission_name = 3, threshold = 4, keys = 7 + JsonFormat.merge("{\"owner\":{\"1\":\"Owner\",\"3\":\"owner\",\"4\":2,\"7\":[{\"address\":\"" + + OWNER_HEX + "\",\"weight\":1},{\"weight\":1}]}}", alias, false); + + assertEquals(named.build(), alias.build()); + } + + @Test + public void testNumericAliasRepeatedMessageField() throws Exception { + Transaction.Builder named = Transaction.newBuilder(); + Transaction.Builder alias = Transaction.newBuilder(); + + JsonFormat.merge("{\"ret\":[{\"fee\":1},{\"fee\":2}]}", named, false); + JsonFormat.merge("{\"5\":[{\"fee\":1},{\"fee\":2}]}", alias, false); + + assertEquals(2, alias.getRetCount()); + assertEquals(named.build(), alias.build()); + } + + @Test + public void testNumericAliasMessageFieldRejectsStringValue() { + assertRejectedAsObject("{\"2\":\"3a003a00\"}", false); + } + + @Test + public void testNumericAliasMessageFieldRejectsStringValueWhenVisible() { + assertRejectedAsObject("{\"2\":\"3a003a00\"}", true); + } + + @Test + public void testNumericAliasRepeatedMessageFieldRejectsStringElements() { + assertRejectedAsObject("{\"4\":[\"3a00\",\"3a00\"]}", false); + } + + @Test + public void testNumericAliasNestedMessageFieldRejectsStringValue() { + // Permission.keys = 7; "1001" would decode to a Key with weight 1. + assertRejectedAsObject("{\"owner\":{\"threshold\":1,\"7\":\"1001\"}}", false); + assertRejectedAsObject("{\"2\":{\"7\":[\"1001\"]}}", false); + } + + @Test + public void testNumericAliasMessageFieldRejectsEmptyString() { + Transaction.Builder builder = Transaction.newBuilder(); + + JsonFormat.ParseException e = assertThrows(JsonFormat.ParseException.class, + () -> JsonFormat.merge("{\"1\":\"\"}", builder, false)); + + assertTrue(e.getMessage().contains("Expected \"{\".")); + assertFalse(builder.hasRawData()); + } + + @Test + public void testNumericAliasScalarFieldsUnchanged() throws Exception { + TransferContract.Builder named = TransferContract.newBuilder(); + TransferContract.Builder alias = TransferContract.newBuilder(); + JsonFormat.merge("{\"owner_address\":\"" + OWNER_HEX + "\",\"amount\":123}", named, false); + JsonFormat.merge("{\"1\":\"" + OWNER_HEX + "\",\"3\":123}", alias, false); + assertEquals(named.build(), alias.build()); + + TransferContract.Builder visible = TransferContract.newBuilder(); + JsonFormat.merge("{\"1\":\"" + OWNER_BASE58 + "\"}", visible, true); + assertEquals(alias.getOwnerAddress(), visible.getOwnerAddress()); + + Permission.Builder permission = Permission.newBuilder(); + JsonFormat.merge("{\"1\":\"Active\",\"4\":3}", permission, false); + assertEquals(Permission.PermissionType.Active, permission.getType()); + assertEquals(3, permission.getThreshold()); + } + + @Test + public void testNumericAliasBytesFieldKeepsBinaryContent() throws Exception { + // Any.value is bytes: a numeric alias must still store the raw bytes, not parse them. + Any.Builder named = Any.newBuilder(); + Any.Builder alias = Any.newBuilder(); + JsonFormat.merge("{\"type_url\":\"type.googleapis.com/x\",\"value\":\"0a021001\"}", + named, false); + JsonFormat.merge("{\"1\":\"type.googleapis.com/x\",\"2\":\"0a021001\"}", alias, false); + + assertEquals(named.build(), alias.build()); + assertEquals(ByteString.copyFrom(ByteArray.fromHexString("0a021001")), alias.getValue()); + } + + @Test + public void testNumericAliasEmptyObjectSetsDefaultInstance() throws Exception { + // An empty JSON object sets the message field to its default instance, which is distinct + // from the unset state that null and an empty array leave behind. + AccountPermissionUpdateContract.Builder alias = AccountPermissionUpdateContract.newBuilder(); + AccountPermissionUpdateContract.Builder named = AccountPermissionUpdateContract.newBuilder(); + JsonFormat.merge("{\"2\":{}}", alias, false); + JsonFormat.merge("{\"owner\":{}}", named, false); + + assertTrue(alias.hasOwner()); + assertEquals(named.build(), alias.build()); + assertEquals(Permission.getDefaultInstance(), alias.getOwner()); + } + + @Test + public void testNumericAliasNullAndEmptyArrayLeaveFieldUnset() throws Exception { + AccountPermissionUpdateContract.Builder builder = AccountPermissionUpdateContract.newBuilder(); + JsonFormat.merge("{\"2\":null}", builder, false); + assertFalse(builder.hasOwner()); + + JsonFormat.merge("{\"2\":[]}", builder, false); + assertFalse(builder.hasOwner()); + + JsonFormat.merge("{\"4\":[]}", builder, false); + assertEquals(0, builder.getActivesCount()); + } + + @Test + public void testKeysResolvingToNoFieldAreStillSkipped() throws Exception { + // AccountPermissionUpdateContract has fields 1-4 only; "26" is not a single digit. + AccountPermissionUpdateContract.Builder builder = AccountPermissionUpdateContract.newBuilder(); + + JsonFormat.merge("{\"foo\":{\"a\":1},\"26\":1,\"9\":\"3a00\",\"0\":[1,2]," + + "\"2\":{\"threshold\":1}}", builder, false); + + // Only the resolvable key contributes; message equality also covers unknown fields. + assertEquals(AccountPermissionUpdateContract.newBuilder() + .setOwner(Permission.newBuilder().setThreshold(1)).build(), builder.build()); + } + + @Test + public void testUnknownFieldValueMatrixUnchanged() throws Exception { + String[] skipped = { + "{\"foo\":1}", "{\"foo\":\"x\"}", "{\"foo\":true}", "{\"foo\":null}", + "{\"foo\":{\"a\":1}}", "{\"foo\":[1]}", "{\"foo\":-1}", + }; + for (String json : skipped) { + AccountPermissionUpdateContract.Builder builder = + AccountPermissionUpdateContract.newBuilder(); + JsonFormat.merge(json, builder, false); + assertEquals(json, AccountPermissionUpdateContract.getDefaultInstance(), builder.build()); + } + + String[] rejected = { + "{\"foo\":{}}", "{\"foo\":[]}", "{\"foo\":9999999999999999999}", "{\"foo\":1.5}", + }; + for (String json : rejected) { + AccountPermissionUpdateContract.Builder builder = + AccountPermissionUpdateContract.newBuilder(); + assertThrows(json, JsonFormat.ParseException.class, + () -> JsonFormat.merge(json, builder, false)); + } + } + + @Test + public void testNumericAliasParsesAtJsonTokenLimit() throws Exception { + // A body that the servlets' Jackson pre-parse accepts must produce the same number of + // sub-messages through either spelling. {"k":{"keys":[ ... ]}} costs 8 tokens plus 2 per + // empty element. The over-limit case belongs to the pre-parser and is covered by JsonTest. + int within = (Constant.MAX_TOKEN_COUNT - 8) / 2; + for (String key : new String[] {"owner", "2"}) { + String body = keysBody(key, within); + JSON.parseObject(body); + AccountPermissionUpdateContract.Builder builder = + AccountPermissionUpdateContract.newBuilder(); + JsonFormat.merge(body, builder, false); + assertEquals(within, builder.getOwner().getKeysCount()); + } + } + + @Test + public void testLargeHexStringForMessageFieldIsRejected() { + StringBuilder sb = new StringBuilder("{\"2\":\""); + for (int i = 0; i < 100_000; i++) { + sb.append("3a00"); + } + sb.append("\"}"); + AccountPermissionUpdateContract.Builder builder = AccountPermissionUpdateContract.newBuilder(); + + JsonFormat.ParseException e = assertThrows(JsonFormat.ParseException.class, + () -> JsonFormat.merge(sb.toString(), builder, false)); + + assertTrue(e.getMessage(), e.getMessage().contains("Expected \"{\".")); + assertFalse(builder.hasOwner()); + } + + @Test + public void testNamedKeyAndNumericAliasResolveToTheSameField() throws Exception { + // A singular field keeps the value of whichever key comes last, the same way two keys with + // the field name would. + AccountPermissionUpdateContract.Builder aliasLast = + AccountPermissionUpdateContract.newBuilder(); + JsonFormat.merge("{\"owner\":{\"threshold\":1},\"2\":{\"threshold\":2}}", aliasLast, false); + assertEquals(2, aliasLast.getOwner().getThreshold()); + + AccountPermissionUpdateContract.Builder nameLast = AccountPermissionUpdateContract.newBuilder(); + JsonFormat.merge("{\"2\":{\"threshold\":2},\"owner\":{\"threshold\":1}}", nameLast, false); + assertEquals(1, nameLast.getOwner().getThreshold()); + + AccountPermissionUpdateContract.Builder twoNames = + AccountPermissionUpdateContract.newBuilder(); + JsonFormat.merge("{\"owner\":{\"threshold\":1},\"owner\":{\"threshold\":2}}", + twoNames, false); + assertEquals(twoNames.getOwner().getThreshold(), aliasLast.getOwner().getThreshold()); + + // A repeated field appends from both spellings, again as two named keys would. + AccountPermissionUpdateContract.Builder repeated = + AccountPermissionUpdateContract.newBuilder(); + JsonFormat.merge("{\"actives\":[{}],\"4\":[{},{}]}", repeated, false); + assertEquals(3, repeated.getActivesCount()); + + Transaction.Builder transaction = Transaction.newBuilder(); + JsonFormat.merge("{\"raw_data\":{\"timestamp\":1},\"1\":{\"timestamp\":2}}", + transaction, false); + assertEquals(2, transaction.getRawData().getTimestamp()); + } + + @Test + public void testFieldsWrittenBeforeAFailureAreKept() { + // Merging is incremental and is not rolled back on failure, for a numeric alias exactly as + // for the field name. + AccountPermissionUpdateContract.Builder alias = AccountPermissionUpdateContract.newBuilder(); + assertThrows(JsonFormat.ParseException.class, + () -> JsonFormat.merge("{\"2\":{\"threshold\":7},\"4\":[{},\"3a00\"]}", alias, false)); + assertEquals(7, alias.getOwner().getThreshold()); + assertEquals(1, alias.getActivesCount()); + + AccountPermissionUpdateContract.Builder named = AccountPermissionUpdateContract.newBuilder(); + assertThrows(JsonFormat.ParseException.class, + () -> JsonFormat.merge("{\"owner\":{\"threshold\":7},\"actives\":[{},\"3a00\"]}", + named, false)); + assertEquals(named.getOwner().getThreshold(), alias.getOwner().getThreshold()); + assertEquals(named.getActivesCount(), alias.getActivesCount()); + } + + @Test + public void testOnlySingleDigitKeysAreFieldAliases() throws Exception { + // DIGITS matches one character, so only fields 1-9 can be addressed by number. A key with + // two digits resolves to no field and is skipped like any other unknown key. + Transaction.raw.Builder named = Transaction.raw.newBuilder(); + JsonFormat.merge("{\"contract\":[{\"type\":\"TransferContract\"}],\"fee_limit\":9}", + named, false); + assertEquals(1, named.getContractCount()); + assertEquals(9, named.getFeeLimit()); + + Transaction.raw.Builder twoDigits = Transaction.raw.newBuilder(); + JsonFormat.merge("{\"11\":[{\"type\":\"TransferContract\"}],\"18\":9}", twoDigits, false); + assertEquals(Transaction.raw.getDefaultInstance(), twoDigits.build()); + + // A single-digit key in the same message still resolves (expiration = 8). + Transaction.raw.Builder oneDigit = Transaction.raw.newBuilder(); + JsonFormat.merge("{\"8\":7}", oneDigit, false); + assertEquals(7, oneDigit.getExpiration()); + } + + @Test + public void testNumericAliasEnumAcceptsNumberAndName() throws Exception { + // Permission.type = 1 is an enum: both spellings of the field accept both spellings of + // the value. + String[] bodies = {"{\"type\":\"Active\"}", "{\"type\":2}", "{\"1\":\"Active\"}", + "{\"1\":2}"}; + for (String body : bodies) { + Permission.Builder builder = Permission.newBuilder(); + JsonFormat.merge(body, builder, false); + assertEquals(body, Permission.PermissionType.Active, builder.getType()); + } + } + + @Test + public void testNumericAliasForNameStringBytesFieldWhenVisible() throws Exception { + // AccountUpdateContract: account_name = 1 (decoded as text under selfType), owner_address = 2. + AccountUpdateContract.Builder named = AccountUpdateContract.newBuilder(); + AccountUpdateContract.Builder alias = AccountUpdateContract.newBuilder(); + JsonFormat.merge("{\"account_name\":\"alice\",\"owner_address\":\"" + OWNER_BASE58 + + "\"}", named, true); + JsonFormat.merge("{\"1\":\"alice\",\"2\":\"" + OWNER_BASE58 + "\"}", alias, true); + + assertEquals(named.build(), alias.build()); + assertEquals(ByteString.copyFromUtf8("alice"), alias.getAccountName()); + assertEquals(ByteString.copyFrom(ByteArray.fromHexString(OWNER_HEX)), alias.getOwnerAddress()); + } + + @Test + public void testNumericAliasForMapField() throws Exception { + // ProposalCreateContract.parameters = 2 is a map, printed as repeated key/value messages. + ProposalCreateContract.Builder named = ProposalCreateContract.newBuilder(); + ProposalCreateContract.Builder alias = ProposalCreateContract.newBuilder(); + JsonFormat.merge("{\"parameters\":[{\"key\":1,\"value\":2}]}", named, false); + JsonFormat.merge("{\"2\":[{\"key\":1,\"value\":2}]}", alias, false); + + assertEquals(named.build(), alias.build()); + assertEquals(Long.valueOf(2), alias.build().getParametersMap().get(1L)); + + // MapEntry itself has key = 1 and value = 2, so the entry accepts aliases too. + ProposalCreateContract.Builder entryAlias = ProposalCreateContract.newBuilder(); + JsonFormat.merge("{\"2\":[{\"1\":5,\"2\":6}]}", entryAlias, false); + assertEquals(Long.valueOf(6), entryAlias.build().getParametersMap().get(5L)); + + ProposalCreateContract.Builder hex = ProposalCreateContract.newBuilder(); + JsonFormat.ParseException e = assertThrows(JsonFormat.ParseException.class, + () -> JsonFormat.merge("{\"2\":\"08011002\"}", hex, false)); + assertTrue(e.getMessage().contains("Expected \"{\".")); + assertTrue(hex.build().getParametersMap().isEmpty()); + } +} diff --git a/framework/src/test/java/org/tron/core/services/http/RateLimiterServletTest.java b/framework/src/test/java/org/tron/core/services/http/RateLimiterServletTest.java index 26826c5709d..44402a9cb67 100644 --- a/framework/src/test/java/org/tron/core/services/http/RateLimiterServletTest.java +++ b/framework/src/test/java/org/tron/core/services/http/RateLimiterServletTest.java @@ -280,4 +280,49 @@ public void testOversizedRequestBadMessagePropagates() throws Exception { assertEquals(HttpStatus.PAYLOAD_TOO_LARGE_413, e.getCode()); } } + + @Test + public void testResponseFailureIsLoggedAndPermitStillReleased() throws Exception { + // A failure from the response itself stays inside the handler's own catch clauses: it is + // logged rather than propagated, and the cleanup still runs. + IPreemptibleRateLimiter perEndpoint = Mockito.mock(IPreemptibleRateLimiter.class); + when(perEndpoint.acquirePermit(any(RuntimeData.class))).thenReturn(true); + container.add(KEY_HTTP, "TestServlet", perEndpoint); + MockHttpServletResponse failing = new MockHttpServletResponse() { + @Override + public void setContentType(String contentType) { + throw new IllegalStateException("response already committed"); + } + }; + + try (MockedStatic globalMock = mockStatic(GlobalRateLimiter.class)) { + globalMock.when(() -> GlobalRateLimiter.acquirePermit(any())).thenReturn(true); + + servlet.service(request, failing); + + verify(perEndpoint, times(1)).release(); + } + } + + @Test + public void testGetReleasesPermitWhenParameterAccessThrows() throws Exception { + IPreemptibleRateLimiter perEndpoint = Mockito.mock(IPreemptibleRateLimiter.class); + when(perEndpoint.acquirePermit(any(RuntimeData.class))).thenReturn(true); + container.add(KEY_HTTP, "TestServlet", perEndpoint); + MockHttpServletRequest getRequest = new MockHttpServletRequest("GET", "/test") { + @Override + public String getParameter(String name) { + throw new BadMessageException(); + } + }; + getRequest.setRemoteAddr("10.0.0.1"); + + try (MockedStatic globalMock = mockStatic(GlobalRateLimiter.class)) { + globalMock.when(() -> GlobalRateLimiter.acquirePermit(any())).thenReturn(true); + + assertThrows(BadMessageException.class, () -> servlet.service(getRequest, response)); + + verify(perEndpoint, times(1)).release(); + } + } } diff --git a/framework/src/test/java/org/tron/core/services/http/UtilTest.java b/framework/src/test/java/org/tron/core/services/http/UtilTest.java index c619fd0de54..1c763090da7 100644 --- a/framework/src/test/java/org/tron/core/services/http/UtilTest.java +++ b/framework/src/test/java/org/tron/core/services/http/UtilTest.java @@ -301,4 +301,83 @@ public void testPrintSignWeightTooManySigsHttpPath() { Assert.assertTrue(jsonObject.getJSONObject("result").getString("message") .contains("too many signatures")); } + + private static String permissionUpdateContract(String value) { + return "{\"parameter\":{\"value\":" + value + + ",\"type_url\":\"type.googleapis.com/protocol.AccountPermissionUpdateContract\"}," + + "\"type\":\"AccountPermissionUpdateContract\"}"; + } + + private static String transactionWithContracts(String... contracts) { + return "{\"raw_data\":{\"contract\":[" + String.join(",", contracts) + + "],\"timestamp\":1}}"; + } + + @Test + public void testPackTransactionNumericFieldAliasMatchesFieldName() { + String permission = "{\"threshold\":1,\"keys\":[{\"address\":\"" + OWNER_ADDRESS + + "\",\"weight\":1}]}"; + Transaction named = Util.packTransaction(transactionWithContracts(permissionUpdateContract( + "{\"owner_address\":\"" + OWNER_ADDRESS + "\",\"owner\":" + permission + "}")), false); + Transaction alias = Util.packTransaction(transactionWithContracts(permissionUpdateContract( + "{\"1\":\"" + OWNER_ADDRESS + "\",\"2\":" + permission + "}")), false); + + Assert.assertEquals(1, named.getRawData().getContractCount()); + Assert.assertEquals(1, alias.getRawData().getContractCount()); + Assert.assertEquals(named.getRawData().getContract(0).getParameter(), + alias.getRawData().getContract(0).getParameter()); + } + + @Test + public void testPackTransactionNumericFieldAliasRejectsStringForMessageField() { + Transaction transaction = Util.packTransaction( + transactionWithContracts(permissionUpdateContract("{\"2\":\"3a003a00\"}")), false); + + // The contract fails to parse and is dropped, exactly like any other malformed value. + Assert.assertNotNull(transaction); + Assert.assertEquals(0, transaction.getRawData().getContractCount()); + } + + @Test + public void testPackTransactionReturnsNullWhenTheEnvelopeFailsToParse() { + // Util.packTransaction merges twice: once per contract, and once for the whole transaction. + // A failure in the second merge has always returned null, whichever spelling causes it. + String valid = permissionUpdateContract("{\"owner_address\":\"" + OWNER_ADDRESS + + "\",\"owner\":{\"threshold\":1}}"); + String envelope = "{\"raw_data\":{\"contract\":[" + valid + "],\"timestamp\":1}"; + + Transaction control = Util.packTransaction(envelope + "}", false); + Assert.assertNotNull(control); + Assert.assertEquals(1, control.getRawData().getContractCount()); + + // Transaction.ret = 5 is a repeated message field: the object form parses, ... + Transaction withRet = Util.packTransaction(envelope + ",\"5\":[{}]}", false); + Assert.assertNotNull(withRet); + Assert.assertEquals(1, withRet.getRetCount()); + + // ... the string form does not, and the whole merge fails. + Assert.assertNull(Util.packTransaction(envelope + ",\"5\":\"0a00\"}", false)); + // Same outcome for the field name, which is how this path already behaved. + Assert.assertNull(Util.packTransaction(envelope + ",\"ret\":\"0a00\"}", false)); + } + + @Test + public void testPackTransactionDropsOnlyTheContractThatFailsToParse() { + String valid = permissionUpdateContract("{\"owner_address\":\"" + OWNER_ADDRESS + + "\",\"owner\":{\"threshold\":1}}"); + String invalid = permissionUpdateContract("{\"2\":\"3a003a00\"}"); + + Transaction transaction = Util.packTransaction( + transactionWithContracts(invalid, valid), false); + + Assert.assertNotNull(transaction); + Assert.assertEquals(1, transaction.getRawData().getContractCount()); + Assert.assertEquals(Protocol.Transaction.Contract.ContractType.AccountPermissionUpdateContract, + transaction.getRawData().getContract(0).getType()); + + // Both contracts share a type, so compare the payload to prove the valid one survived. + Transaction onlyValid = Util.packTransaction(transactionWithContracts(valid), false); + Assert.assertEquals(onlyValid.getRawData().getContract(0).getParameter(), + transaction.getRawData().getContract(0).getParameter()); + } }