From 85393e5fdec2ce3c56df93f8d59eb1fcc826268b Mon Sep 17 00:00:00 2001 From: waynercheung Date: Thu, 5 Nov 2026 17:08:18 +0800 Subject: [PATCH] fix(http): preserve U+FFFF in JsonFormat string output escapeText stopped at CharacterIterator.DONE, which equals U+FFFF, so string fields containing it were truncated in HTTP JSON while txID still covered the full bytes, causing SIGERROR on broadcast. Iterate by index, replace unpaired surrogates with '?' to match the encoded bytes, and drop the escapeNameStringText workaround. --- .../services/http/servlets/JsonFormat.java | 51 +--- .../http/servlets/JsonFormatEscapeTest.java | 217 +++++++++++++++++- 2 files changed, 223 insertions(+), 45 deletions(-) diff --git a/framework/src/main/java/org/tron/core/services/http/servlets/JsonFormat.java b/framework/src/main/java/org/tron/core/services/http/servlets/JsonFormat.java index d69e9b1fb25..1aeca080399 100644 --- a/framework/src/main/java/org/tron/core/services/http/servlets/JsonFormat.java +++ b/framework/src/main/java/org/tron/core/services/http/servlets/JsonFormat.java @@ -44,8 +44,6 @@ SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT import java.math.BigInteger; import java.nio.CharBuffer; import java.nio.charset.StandardCharsets; -import java.text.CharacterIterator; -import java.text.StringCharacterIterator; import java.util.Iterator; import java.util.List; import java.util.Locale; @@ -903,7 +901,7 @@ static String escapeBytesSelfType(ByteString input, final String fieldName) { if (!input.isValidUtf8()) { return ByteArray.toHexString(input.toByteArray()); } - return escapeNameStringText(input.toStringUtf8()); + return escapeText(input.toStringUtf8()); } //HEX return ByteArray.toHexString(input.toByteArray()); @@ -934,41 +932,18 @@ static ByteString unescapeBytes(CharSequence input) throws InvalidEscapeSequence // Some of these methods are package-private because Descriptors.java uses // them. - /** - * Escapes a valid UTF-8 name-string without exposing it to the U+FFFF sentinel used by - * {@link StringCharacterIterator}. Keeping this workaround on the new bytes path avoids - * changing the established behavior of {@link #escapeText(String)} for proto string fields. - */ - private static String escapeNameStringText(String input) { - int index = input.indexOf(Character.MAX_VALUE); - if (index < 0) { - return escapeText(input); - } - - StringBuilder result = new StringBuilder(input.length()); - int start = 0; - while (index >= 0) { - result.append(escapeText(input.substring(start, index))); - result.append(Character.MAX_VALUE); - start = index + 1; - index = input.indexOf(Character.MAX_VALUE, start); - } - result.append(escapeText(input.substring(start))); - return result.toString(); - } - /** * Implements JSON string escaping as specified here. * + * notation; unpaired high or low surrogates are replaced with '?', matching their UTF-8 + * encoding
  • any other character, including U+FFFF, is printed as-is
    • */ static String escapeText(String input) { StringBuilder builder = new StringBuilder(input.length()); - CharacterIterator iter = new StringCharacterIterator(input); - for (char c = iter.first(); c != CharacterIterator.DONE; c = iter.next()) { + for (int i = 0; i < input.length(); i++) { + char c = input.charAt(i); switch (c) { case '\b': builder.append("\\b"); @@ -995,18 +970,12 @@ static String escapeText(String input) { // Check for other control characters if (c >= 0x0000 && c <= 0x001F) { appendEscapedUnicode(builder, c); - } else if (Character.isLowSurrogate(c)) { - builder.append(replaceMalformedSurrogates(String.valueOf(c))); - } else if (Character.isHighSurrogate(c)) { - // Encode the surrogate pair using 2 six-character sequence (\\uXXXX\\uXXXX) - appendEscapedUnicode(builder, c); - c = iter.next(); - if (c == CharacterIterator.DONE) { - throw new IllegalArgumentException( - "invalid unicode string: unexpected high surrogate pair value " - + "without corresponding low value."); - } + } else if (Character.isHighSurrogate(c) + && i + 1 < input.length() && Character.isLowSurrogate(input.charAt(i + 1))) { appendEscapedUnicode(builder, c); + appendEscapedUnicode(builder, input.charAt(++i)); + } else if (Character.isSurrogate(c)) { + builder.append('?'); } else { // Anything else can be printed as-is builder.append(c); diff --git a/framework/src/test/java/org/tron/core/services/http/servlets/JsonFormatEscapeTest.java b/framework/src/test/java/org/tron/core/services/http/servlets/JsonFormatEscapeTest.java index 4ee3e69e987..9e15f341866 100644 --- a/framework/src/test/java/org/tron/core/services/http/servlets/JsonFormatEscapeTest.java +++ b/framework/src/test/java/org/tron/core/services/http/servlets/JsonFormatEscapeTest.java @@ -12,18 +12,28 @@ import com.fasterxml.jackson.databind.json.JsonMapper; import com.google.protobuf.Any; import com.google.protobuf.ByteString; +import com.google.protobuf.Message; import java.nio.CharBuffer; import java.nio.charset.StandardCharsets; +import org.junit.BeforeClass; import org.junit.Test; import org.tron.common.utils.ByteArray; +import org.tron.common.utils.Sha256Hash; import org.tron.core.capsule.TransactionCapsule; import org.tron.core.services.http.HttpSelfFormatFieldName; +import org.tron.core.utils.TransactionRegister; +import org.tron.protos.Protocol.Permission; import org.tron.protos.Protocol.Transaction; import org.tron.protos.Protocol.Transaction.Contract.ContractType; +import org.tron.protos.Protocol.Witness; +import org.tron.protos.contract.AccountContract.AccountPermissionUpdateContract; import org.tron.protos.contract.AssetIssueContractOuterClass.AssetIssueContract; +import org.tron.protos.contract.SmartContractOuterClass.CreateSmartContract; +import org.tron.protos.contract.SmartContractOuterClass.SmartContract; +import org.tron.protos.contract.SmartContractOuterClass.SmartContract.ABI; /** - * Escaping and decoding of name-string {@code bytes} fields. + * Escaping and decoding of name-string {@code bytes} fields and proto {@code string} fields. * *

    Before this fix {@code escapeBytesSelfType} only escaped the double quote, leaving backslash * and control chars raw, and validated the result with a lenient parser. That let attacker @@ -89,12 +99,24 @@ public class JsonFormatEscapeTest { "protocol.MarketPriceList.buy_token_id" }; + private static final String NONCHAR_FFFF = String.valueOf((char) 0xFFFF); + private static final String NONCHAR_FFFE = String.valueOf((char) 0xFFFE); + private static final String HIGH = String.valueOf((char) 0xD83D); + private static final String LOW = String.valueOf((char) 0xDE00); + private static final String EMOJI_ESCAPED = "\\ud83d\\ude00"; + private static final String UFFFF_VALUE = "Safe" + NONCHAR_FFFF + "Token"; + private static final byte[] OWNER = new byte[21]; static { OWNER[0] = 0x41; } + @BeforeClass + public static void registerContractTypes() { + TransactionRegister.registerActuator(); + } + /** * Strict RFC 8259 baseline for the features this issue turns on: structural leniency and * unescaped control chars. Each is disabled explicitly rather than relying on Jackson's @@ -212,8 +234,8 @@ public void testInvalidUtf8FallsBackToHexWithoutReplacementChar() { @Test public void testUnpairedSurrogateBytesFallBackToHexInsteadOfThrowing() { - // 0xED 0xA0 0x80 is the UTF-8 style encoding of an unpaired high surrogate. escapeText() - // would throw on it, so isValidUtf8() must divert it to hex first. + // 0xED 0xA0 0x80 is the UTF-8 style encoding of an unpaired high surrogate. It is not + // valid UTF-8, so isValidUtf8() must divert it to hex to keep the original bytes. byte[] raw = new byte[] {(byte) 0xed, (byte) 0xa0, (byte) 0x80}; assertEquals("eda080", escapeName(raw, URL_FIELD)); @@ -543,7 +565,7 @@ public void testInvalidUtf8IsHexAndNotPromisedToRoundTrip() { @Test public void testUffffDoesNotTruncateNameStringField() { - // EF BF BF is valid UTF-8, so it must remain text without reaching escapeText's sentinel. + // EF BF BF is valid UTF-8, so it must remain text instead of truncating the value. String value = "SAFE\uFFFF TAIL"; assertTrue(ByteString.copyFromUtf8(value).isValidUtf8()); @@ -555,6 +577,193 @@ public void testUffffLeadingCharDoesNotEmptyTheValue() { assertEquals("\uFFFFwhole value", escapeName("\uFFFFwhole value", URL_FIELD)); } + // U+FFFF and surrogate handling on proto string fields + + @Test + public void testEscapeTextKeepsNoncharacters() { + for (String value : new String[] { + "Safe" + NONCHAR_FFFF + "Token", + NONCHAR_FFFF + "Safe", + "Safe" + NONCHAR_FFFF, + "Safe" + NONCHAR_FFFF + NONCHAR_FFFF + "Token", + "Safe" + NONCHAR_FFFE + "Token"}) { + assertEquals(value, JsonFormat.escapeText(value)); + } + assertEquals(EMOJI_ESCAPED + NONCHAR_FFFF + "Tail", + JsonFormat.escapeText(HIGH + LOW + NONCHAR_FFFF + "Tail")); + } + + @Test + public void testLiteralBackslashUffffTextIsNotTheCodePoint() { + assertEquals("Safe\\\\uFFFFToken", JsonFormat.escapeText("Safe\\uFFFFToken")); + } + + @Test + public void testUnpairedHighSurrogateIsReplaced() { + assertEquals("?" + NONCHAR_FFFF, JsonFormat.escapeText(HIGH + NONCHAR_FFFF)); + assertEquals("a?", JsonFormat.escapeText("a" + HIGH)); + assertEquals("?A", JsonFormat.escapeText(HIGH + "A")); + assertEquals("??", JsonFormat.escapeText(HIGH + HIGH)); + assertEquals("?" + EMOJI_ESCAPED, JsonFormat.escapeText(HIGH + HIGH + LOW)); + } + + private static Transaction transactionOf(ContractType type, Message parameter) { + Transaction.Contract contract = Transaction.Contract.newBuilder() + .setType(type) + .setParameter(Any.pack(parameter)) + .build(); + return Transaction.newBuilder() + .setRawData(Transaction.raw.newBuilder() + .setTimestamp(1_234_567_890L) + .addContract(contract)) + .build(); + } + + /** Renders the transaction as the HTTP API does and rebuilds it as broadcasttransaction does. */ + private static Transaction assertRoundTrip(Transaction transaction, boolean visible) + throws Exception { + byte[] rawData = transaction.getRawData().toByteArray(); + org.tron.json.JSONObject json = Util.printTransactionToJSON(transaction, visible); + String txId = json.getString("txID"); + String where = " (visible=" + visible + ")"; + + assertEquals("sanity: raw_data_hex is derived from the original bytes" + where, + ByteArray.toHexString(rawData), json.getString("raw_data_hex")); + assertEquals("sanity: txID is the hash of the decoded raw_data_hex bytes" + where, + ByteArray.toHexString(Sha256Hash.hash( + ByteArray.fromHexString(json.getString("raw_data_hex")))), txId); + + Transaction rebuilt = Util.packTransaction(json.toJSONString(), visible); + assertEquals("structured raw_data must rebuild the original bytes" + where, + transaction.getRawData().toByteString(), rebuilt.getRawData().toByteString()); + return rebuilt; + } + + private static T firstParameter(Transaction transaction, Class clazz) + throws Exception { + return transaction.getRawData().getContract(0).getParameter().unpack(clazz); + } + + @Test + public void testCreateSmartContractWithUffffRoundTrips() throws Exception { + String mixed = "A" + HIGH + LOW + NONCHAR_FFFF + NONCHAR_FFFE + "B"; + ABI.Entry.Param param = ABI.Entry.Param.newBuilder() + .setName(mixed) + .setType("uint256" + NONCHAR_FFFF + "Tail") + .build(); + ABI abi = ABI.newBuilder() + .addEntrys(ABI.Entry.newBuilder() + .setName(UFFFF_VALUE) + .setType(ABI.Entry.EntryType.Function) + .addInputs(param) + .addOutputs(param)) + .build(); + CreateSmartContract create = CreateSmartContract.newBuilder() + .setOwnerAddress(ByteString.copyFrom(OWNER)) + .setNewContract(SmartContract.newBuilder() + .setOriginAddress(ByteString.copyFrom(OWNER)) + .setName(UFFFF_VALUE) + .setAbi(abi) + .setBytecode(ByteString.copyFrom(new byte[] {0x60, 0x00}))) + .build(); + Transaction transaction = transactionOf(ContractType.CreateSmartContract, create); + + for (boolean visible : new boolean[] {true, false}) { + SmartContract rebuilt = firstParameter(assertRoundTrip(transaction, visible), + CreateSmartContract.class).getNewContract(); + ABI.Entry entry = rebuilt.getAbi().getEntrys(0); + + assertEquals(UFFFF_VALUE, rebuilt.getName()); + assertEquals(UFFFF_VALUE, entry.getName()); + assertEquals(mixed, entry.getInputs(0).getName()); + assertEquals("uint256" + NONCHAR_FFFF + "Tail", entry.getInputs(0).getType()); + assertEquals(mixed, entry.getOutputs(0).getName()); + assertEquals("uint256" + NONCHAR_FFFF + "Tail", entry.getOutputs(0).getType()); + } + } + + @Test + public void testAccountPermissionUpdateWithUffffRoundTrips() throws Exception { + Permission permission = Permission.newBuilder() + .setPermissionName(UFFFF_VALUE) + .setThreshold(1) + .build(); + AccountPermissionUpdateContract update = AccountPermissionUpdateContract.newBuilder() + .setOwnerAddress(ByteString.copyFrom(OWNER)) + .setOwner(permission.toBuilder().setType(Permission.PermissionType.Owner)) + .setWitness(permission.toBuilder().setType(Permission.PermissionType.Witness)) + .addActives(permission.toBuilder().setType(Permission.PermissionType.Active)) + .build(); + Transaction transaction = + transactionOf(ContractType.AccountPermissionUpdateContract, update); + + for (boolean visible : new boolean[] {true, false}) { + AccountPermissionUpdateContract rebuilt = firstParameter( + assertRoundTrip(transaction, visible), AccountPermissionUpdateContract.class); + + assertEquals(UFFFF_VALUE, rebuilt.getOwner().getPermissionName()); + assertEquals(UFFFF_VALUE, rebuilt.getWitness().getPermissionName()); + assertEquals(UFFFF_VALUE, rebuilt.getActives(0).getPermissionName()); + } + } + + @Test + public void testAssetIssueIdWithUffffRoundTrips() throws Exception { + AssetIssueContract asset = AssetIssueContract.newBuilder() + .setOwnerAddress(ByteString.copyFrom(OWNER)) + .setName(ByteString.copyFromUtf8("ASSET")) + .setId("Id" + NONCHAR_FFFF + "Tail") + .build(); + Transaction transaction = transactionOf(ContractType.AssetIssueContract, asset); + + for (boolean visible : new boolean[] {true, false}) { + AssetIssueContract rebuilt = firstParameter(assertRoundTrip(transaction, visible), + AssetIssueContract.class); + + assertEquals("Id" + NONCHAR_FFFF + "Tail", rebuilt.getId()); + } + } + + @Test + public void testLoneHighSurrogateRendersAsStoredBytes() throws Exception { + SmartContract inMemory = SmartContract.newBuilder() + .setName("abc" + HIGH) + .build(); + for (boolean visible : new boolean[] {true, false}) { + assertEquals("abc?", strict().readTree(JsonFormat.printToString(inMemory, visible)) + .get("name").asText()); + } + + CreateSmartContract create = CreateSmartContract.newBuilder() + .setOwnerAddress(ByteString.copyFrom(OWNER)) + .setNewContract(inMemory) + .build(); + Transaction transaction = transactionOf(ContractType.CreateSmartContract, create); + for (boolean visible : new boolean[] {true, false}) { + assertEquals("abc?", firstParameter(assertRoundTrip(transaction, visible), + CreateSmartContract.class).getNewContract().getName()); + } + } + + @Test + public void testWitnessUrlWithUffffIsNotTruncated() throws Exception { + Witness witness = Witness.newBuilder() + .setAddress(ByteString.copyFrom(OWNER)) + .setUrl(UFFFF_VALUE) + .setVoteCount(7) + .build(); + + for (boolean visible : new boolean[] {true, false}) { + String json = JsonFormat.printToString(witness, visible); + assertTrue(parsesStrictly(json)); + assertEquals(UFFFF_VALUE, strict().readTree(json).get("url").asText()); + + Witness.Builder rebuilt = Witness.newBuilder(); + JsonFormat.merge(json, rebuilt, visible); + assertEquals(witness, rebuilt.build()); + } + } + // Proto string inbound compatibility private static String mergeId(String rawJsonToken, boolean visible) throws Exception {