From f77b94de4e2cf3c7fc78d755428659ee2e08d1e9 Mon Sep 17 00:00:00 2001 From: halibobo1205 Date: Fri, 28 Aug 2026 17:52:48 +0800 Subject: [PATCH 1/2] feat(api): sanitize HTTP API error responses Standard HTTP error paths used to expose internal details to clients: Util.processError prefixed every message with the Java exception class name, several servlets printed raw Throwable.getMessage() directly, and the two solidity query endpoints returned bare-text error bodies. Centralize the client-facing text decision in Util.processError: * keep the raw non-blank message only for the exact runtime types JsonFormat.ParseException, ContractValidateException and MaintenanceUnavailableException; a null, empty or whitespace-only message falls back to "internal server error" * preserve the events-deprecation message only for the exact IllegalArgumentException type carrying EVENTS_DEPRECATED_MSG * write the fixed rate-limit and INVALID address messages, along with existing GetBlock validation messages, through the package-private writeAuditedError helper; these audited callers bypass exception classification, and printErrorMsg is private to the shared writer * return {"Error":"internal server error"} for every other exception, with no exception class name Client-visible changes: * all processError-based error bodies lose the "class : " prefix; unclassified raw messages become "internal server error" * the rate-limit rejection body becomes {"Error":"lack of computing resources"} on every endpoint extending RateLimiterServlet, including full-node, solidity and PBFT /jsonrpc * gettransactionbyid / gettransactioninfobyid on solidity return standard {"Error":...} JSON instead of bare text * validateaddress, getBrokerage and getReward replace leaked library messages in their failure branches with existing fixed texts; the "INVALID address" body is now written via writeAuditedError and loses the space after the colon * getblock keeps its exact error bodies (refactor only) Cover Solidity transaction and transaction-info GET/POST input errors, backend failures, successful lookups and missing records directly with mocked Wallet calls and in-memory requests and responses. Replace the transaction servlet tests that accidentally exercised POST in both cases, changed global stdout and used a shared temporary response file. Verify both endpoint and global rate-limit rejections across the three JSON-RPC servlet variants, including status, response body and the absence of business dispatch on rejection. HTTP status codes, success responses, request validation rules and gRPC behavior are unchanged. JSON-RPC behavior is unchanged except for the shared HTTP rate-limit response described above. Closes #6936 --- .../core/services/http/GetBlockServlet.java | 4 +- .../services/http/GetBrokerageServlet.java | 8 +- .../core/services/http/GetBurnTrxServlet.java | 8 +- .../services/http/GetNodeInfoServlet.java | 8 +- .../services/http/GetPendingSizeServlet.java | 8 +- .../core/services/http/GetRewardServlet.java | 15 +- .../GetTransactionInfoByBlockNumServlet.java | 15 +- .../services/http/RateLimiterServlet.java | 3 +- .../org/tron/core/services/http/Util.java | 40 ++- .../services/http/ValidateAddressServlet.java | 2 +- .../GetTransactionByIdSolidityServlet.java | 14 +- ...GetTransactionInfoByIdSolidityServlet.java | 15 +- .../services/http/BroadcastServletTest.java | 2 +- .../http/JsonRpcRateLimiterServletTest.java | 129 ++++++++ .../services/http/UtilProcessErrorTest.java | 93 ++++++ ...GetTransactionByIdSolidityServletTest.java | 286 +++++++----------- ...ransactionInfoByIdSolidityServletTest.java | 135 +++++++++ 17 files changed, 523 insertions(+), 262 deletions(-) create mode 100644 framework/src/test/java/org/tron/core/services/http/JsonRpcRateLimiterServletTest.java create mode 100644 framework/src/test/java/org/tron/core/services/http/UtilProcessErrorTest.java create mode 100644 framework/src/test/java/org/tron/core/services/http/solidity/GetTransactionInfoByIdSolidityServletTest.java diff --git a/framework/src/main/java/org/tron/core/services/http/GetBlockServlet.java b/framework/src/main/java/org/tron/core/services/http/GetBlockServlet.java index 2320fc87c7d..a953ae11802 100644 --- a/framework/src/main/java/org/tron/core/services/http/GetBlockServlet.java +++ b/framework/src/main/java/org/tron/core/services/http/GetBlockServlet.java @@ -77,9 +77,7 @@ private void fillResponse(boolean visible, BlockReq request, HttpServletResponse response.getWriter().println("{}"); } } catch (IllegalArgumentException e) { - JSONObject jsonObject = new JSONObject(); - jsonObject.put("Error", e.getMessage()); - response.getWriter().println(jsonObject.toJSONString()); + Util.writeAuditedError(e.getMessage(), response); } } diff --git a/framework/src/main/java/org/tron/core/services/http/GetBrokerageServlet.java b/framework/src/main/java/org/tron/core/services/http/GetBrokerageServlet.java index 1fbd94fe690..b735878d1e1 100644 --- a/framework/src/main/java/org/tron/core/services/http/GetBrokerageServlet.java +++ b/framework/src/main/java/org/tron/core/services/http/GetBrokerageServlet.java @@ -1,6 +1,5 @@ package org.tron.core.services.http; -import java.io.IOException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import lombok.extern.slf4j.Slf4j; @@ -27,12 +26,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) { } response.getWriter().println("{\"brokerage\": " + value + "}"); } catch (DecoderException | IllegalArgumentException e) { - try { - response.getWriter() - .println("{\"Error\": " + "\"INVALID address, " + e.getMessage() + "\"}"); - } catch (IOException ioe) { - logger.debug("IOException: {}", ioe.getMessage()); - } + Util.writeAuditedError(Util.INVALID_ADDRESS_MSG, response); } catch (Exception e) { Util.processError(e, response); } diff --git a/framework/src/main/java/org/tron/core/services/http/GetBurnTrxServlet.java b/framework/src/main/java/org/tron/core/services/http/GetBurnTrxServlet.java index ea066a6e98c..26740ffed0f 100644 --- a/framework/src/main/java/org/tron/core/services/http/GetBurnTrxServlet.java +++ b/framework/src/main/java/org/tron/core/services/http/GetBurnTrxServlet.java @@ -1,6 +1,5 @@ package org.tron.core.services.http; -import java.io.IOException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import lombok.extern.slf4j.Slf4j; @@ -24,12 +23,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) { : "{\"burnTrxAmount\": " + value + "}"; response.getWriter().println(out); } catch (Exception e) { - logger.error("", e); - try { - response.getWriter().println(Util.printErrorMsg(e)); - } catch (IOException ioe) { - logger.debug("IOException: {}", ioe.getMessage()); - } + Util.processError(e, response); } } diff --git a/framework/src/main/java/org/tron/core/services/http/GetNodeInfoServlet.java b/framework/src/main/java/org/tron/core/services/http/GetNodeInfoServlet.java index 0b8f7b9ce2b..b1746e1ff1f 100644 --- a/framework/src/main/java/org/tron/core/services/http/GetNodeInfoServlet.java +++ b/framework/src/main/java/org/tron/core/services/http/GetNodeInfoServlet.java @@ -1,6 +1,5 @@ package org.tron.core.services.http; -import java.io.IOException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import lombok.extern.slf4j.Slf4j; @@ -24,12 +23,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) { response.getWriter().println(JSON.toJSONString(nodeInfo)); } catch (Exception e) { - logger.error("", e); - try { - response.getWriter().println(Util.printErrorMsg(e)); - } catch (IOException ioe) { - logger.debug("IOException: {}", ioe.getMessage()); - } + Util.processError(e, response); } } diff --git a/framework/src/main/java/org/tron/core/services/http/GetPendingSizeServlet.java b/framework/src/main/java/org/tron/core/services/http/GetPendingSizeServlet.java index 9788c926586..6369600352d 100644 --- a/framework/src/main/java/org/tron/core/services/http/GetPendingSizeServlet.java +++ b/framework/src/main/java/org/tron/core/services/http/GetPendingSizeServlet.java @@ -1,6 +1,5 @@ package org.tron.core.services.http; -import java.io.IOException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import lombok.extern.slf4j.Slf4j; @@ -24,12 +23,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) { : "{\"pendingSize\": " + value + "}"; response.getWriter().println(out); } catch (Exception e) { - logger.error("", e); - try { - response.getWriter().println(Util.printErrorMsg(e)); - } catch (IOException ioe) { - logger.debug("IOException: {}", ioe.getMessage()); - } + Util.processError(e, response); } } diff --git a/framework/src/main/java/org/tron/core/services/http/GetRewardServlet.java b/framework/src/main/java/org/tron/core/services/http/GetRewardServlet.java index 61b88d1160f..a7278e054b0 100644 --- a/framework/src/main/java/org/tron/core/services/http/GetRewardServlet.java +++ b/framework/src/main/java/org/tron/core/services/http/GetRewardServlet.java @@ -1,6 +1,5 @@ package org.tron.core.services.http; -import java.io.IOException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import lombok.extern.slf4j.Slf4j; @@ -29,19 +28,9 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) { : "{\"reward\": " + value + "}"; response.getWriter().println(out); } catch (DecoderException | IllegalArgumentException e) { - try { - response.getWriter() - .println("{\"Error\": " + "\"INVALID address, " + e.getMessage() + "\"}"); - } catch (IOException ioe) { - logger.debug("IOException: {}", ioe.getMessage()); - } + Util.writeAuditedError(Util.INVALID_ADDRESS_MSG, response); } catch (Exception e) { - logger.error("", e); - try { - response.getWriter().println(Util.printErrorMsg(e)); - } catch (IOException ioe) { - logger.debug("IOException: {}", ioe.getMessage()); - } + Util.processError(e, response); } } diff --git a/framework/src/main/java/org/tron/core/services/http/GetTransactionInfoByBlockNumServlet.java b/framework/src/main/java/org/tron/core/services/http/GetTransactionInfoByBlockNumServlet.java index 5d0a09b1a68..25998c909b6 100644 --- a/framework/src/main/java/org/tron/core/services/http/GetTransactionInfoByBlockNumServlet.java +++ b/framework/src/main/java/org/tron/core/services/http/GetTransactionInfoByBlockNumServlet.java @@ -1,6 +1,5 @@ package org.tron.core.services.http; -import java.io.IOException; import java.util.List; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -52,12 +51,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) { response.getWriter().println("{}"); } } catch (Exception e) { - logger.debug("Exception: {}", e.getMessage()); - try { - response.getWriter().println(Util.printErrorMsg(e)); - } catch (IOException ioe) { - logger.debug("IOException: {}", ioe.getMessage()); - } + Util.processError(e, response); } } @@ -75,12 +69,7 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) response.getWriter().println("{}"); } } catch (Exception e) { - logger.debug("Exception: {}", e.getMessage()); - try { - response.getWriter().println(Util.printErrorMsg(e)); - } catch (IOException ioe) { - logger.debug("IOException: {}", ioe.getMessage()); - } + Util.processError(e, response); } } } 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 b5ae7d58623..6f67aba3020 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 @@ -131,8 +131,7 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) super.service(req, resp); Metrics.histogramObserve(requestTimer); } else { - resp.getWriter() - .println(Util.printErrorMsg(new IllegalAccessException("lack of computing resources"))); + Util.writeAuditedError(Util.RATE_LIMITER_ERROR_MSG, resp); } } catch (ServletException | IOException | BadMessageException e) { throw e; diff --git a/framework/src/main/java/org/tron/core/services/http/Util.java b/framework/src/main/java/org/tron/core/services/http/Util.java index 5be2495e1f7..60d8527aa92 100644 --- a/framework/src/main/java/org/tron/core/services/http/Util.java +++ b/framework/src/main/java/org/tron/core/services/http/Util.java @@ -48,6 +48,8 @@ import org.tron.core.capsule.TransactionCapsule; import org.tron.core.config.args.Args; import org.tron.core.db.TransactionTrace; +import org.tron.core.exception.ContractValidateException; +import org.tron.core.exception.MaintenanceUnavailableException; import org.tron.core.services.http.JsonFormat.ParseException; import org.tron.json.JSON; import org.tron.json.JSONArray; @@ -65,6 +67,10 @@ @Slf4j(topic = "API") public class Util { + private static final String INTERNAL_SERVER_ERROR = "internal server error"; + public static final String RATE_LIMITER_ERROR_MSG = "lack of computing resources"; + static final String INVALID_ADDRESS_MSG = "INVALID address"; + public static final String EVENTS_DEPRECATED_MSG = "'events' field is deprecated and no longer supported"; @@ -114,12 +120,31 @@ public static String printTransactionFee(String transactionFee) { return jsonObject.toJSONString(); } - public static String printErrorMsg(Exception e) { + private static String printErrorMsg(String msg) { JSONObject jsonObject = new JSONObject(); - jsonObject.put("Error", e.getClass() + " : " + e.getMessage()); + jsonObject.put("Error", msg); return jsonObject.toJSONString(); } + private static String clientMessage(Exception e) { + if (e == null) { + return INTERNAL_SERVER_ERROR; + } + + Class type = e.getClass(); + if (type == IllegalArgumentException.class) { + return EVENTS_DEPRECATED_MSG.equals(e.getMessage()) + ? EVENTS_DEPRECATED_MSG : INTERNAL_SERVER_ERROR; + } + if (type == ParseException.class + || type == ContractValidateException.class + || type == MaintenanceUnavailableException.class) { + String message = e.getMessage(); + return StringUtils.isBlank(message) ? INTERNAL_SERVER_ERROR : message; + } + return INTERNAL_SERVER_ERROR; + } + public static String printBlockList(BlockList list, boolean selfType) { List blocks = list.getBlockList(); JSONObject jsonObject = new JSONObject(); @@ -526,11 +551,16 @@ public static String getMemo(byte[] memo) { } public static void processError(Exception e, HttpServletResponse response) { - logger.debug(e.getMessage(), e); + logger.debug("HTTP request failed", e); + writeAuditedError(clientMessage(e), response); + } + + // Bypasses clientMessage: callers must pass audited fixed or pre-existing client texts only. + static void writeAuditedError(String msg, HttpServletResponse response) { try { - response.getWriter().println(Util.printErrorMsg(e)); + response.getWriter().println(Util.printErrorMsg(msg)); } catch (IOException ioe) { - logger.debug("IOException: {}", ioe.getMessage()); + logger.debug("Failed to write HTTP error response", ioe); } } diff --git a/framework/src/main/java/org/tron/core/services/http/ValidateAddressServlet.java b/framework/src/main/java/org/tron/core/services/http/ValidateAddressServlet.java index 07eecfc5466..3ef45b42a7e 100644 --- a/framework/src/main/java/org/tron/core/services/http/ValidateAddressServlet.java +++ b/framework/src/main/java/org/tron/core/services/http/ValidateAddressServlet.java @@ -47,7 +47,7 @@ private String validAddress(String input) { } } catch (Exception e) { result = false; - msg = e.getMessage(); + msg = "Invalid address"; } JSONObject jsonAddress = new JSONObject(); diff --git a/framework/src/main/java/org/tron/core/services/http/solidity/GetTransactionByIdSolidityServlet.java b/framework/src/main/java/org/tron/core/services/http/solidity/GetTransactionByIdSolidityServlet.java index f98c7450afc..5998bc0850f 100644 --- a/framework/src/main/java/org/tron/core/services/http/solidity/GetTransactionByIdSolidityServlet.java +++ b/framework/src/main/java/org/tron/core/services/http/solidity/GetTransactionByIdSolidityServlet.java @@ -30,12 +30,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) { String input = request.getParameter("value"); fillResponse(ByteString.copyFrom(ByteArray.fromHexString(input)), visible, response); } catch (Exception e) { - logger.debug("Exception: {}", e.getMessage()); - try { - response.getWriter().println(e.getMessage()); - } catch (IOException ioe) { - logger.debug("IOException: {}", ioe.getMessage()); - } + Util.processError(e, response); } } @@ -46,12 +41,7 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) JsonFormat.merge(params.getParams(), build, params.isVisible()); fillResponse(build.build().getValue(), params.isVisible(), response); } catch (Exception e) { - logger.debug("Exception: {}", e.getMessage()); - try { - response.getWriter().println(e.getMessage()); - } catch (IOException ioe) { - logger.debug("IOException: {}", ioe.getMessage()); - } + Util.processError(e, response); } } diff --git a/framework/src/main/java/org/tron/core/services/http/solidity/GetTransactionInfoByIdSolidityServlet.java b/framework/src/main/java/org/tron/core/services/http/solidity/GetTransactionInfoByIdSolidityServlet.java index 0408215f09d..197f5aaec0d 100644 --- a/framework/src/main/java/org/tron/core/services/http/solidity/GetTransactionInfoByIdSolidityServlet.java +++ b/framework/src/main/java/org/tron/core/services/http/solidity/GetTransactionInfoByIdSolidityServlet.java @@ -1,7 +1,6 @@ package org.tron.core.services.http.solidity; import com.google.protobuf.ByteString; -import java.io.IOException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import lombok.extern.slf4j.Slf4j; @@ -37,12 +36,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) { response.getWriter().println(JsonFormat.printToString(transInfo, visible)); } } catch (Exception e) { - logger.debug("Exception: {}", e.getMessage()); - try { - response.getWriter().println(e.getMessage()); - } catch (IOException ioe) { - logger.debug("IOException: {}", ioe.getMessage()); - } + Util.processError(e, response); } } @@ -60,12 +54,7 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) response.getWriter().println(JsonFormat.printToString(transInfo, params.isVisible())); } } catch (Exception e) { - logger.debug("Exception: {}", e.getMessage()); - try { - response.getWriter().println(e.getMessage()); - } catch (IOException ioe) { - logger.debug("IOException: {}", ioe.getMessage()); - } + Util.processError(e, response); } } diff --git a/framework/src/test/java/org/tron/core/services/http/BroadcastServletTest.java b/framework/src/test/java/org/tron/core/services/http/BroadcastServletTest.java index d6bf3850f30..532ddcd5521 100644 --- a/framework/src/test/java/org/tron/core/services/http/BroadcastServletTest.java +++ b/framework/src/test/java/org/tron/core/services/http/BroadcastServletTest.java @@ -156,7 +156,7 @@ public void doPostTest() throws IOException { while ((text = bufferedReader.readLine()) != null) { sb.append(text); } - Assert.assertTrue(sb.toString().contains("null")); + Assert.assertTrue(sb.toString().contains("{\"Error\":\"internal server error\"}")); httpUrlConnection.disconnect(); } } \ No newline at end of file diff --git a/framework/src/test/java/org/tron/core/services/http/JsonRpcRateLimiterServletTest.java b/framework/src/test/java/org/tron/core/services/http/JsonRpcRateLimiterServletTest.java new file mode 100644 index 00000000000..52ff23a7d2d --- /dev/null +++ b/framework/src/test/java/org/tron/core/services/http/JsonRpcRateLimiterServletTest.java @@ -0,0 +1,129 @@ +package org.tron.core.services.http; + +import static org.junit.Assert.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +import com.googlecode.jsonrpc4j.JsonRpcServer; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.Collection; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.mockito.MockedStatic; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.test.util.ReflectionTestUtils; +import org.tron.common.TestConstants; +import org.tron.core.config.args.Args; +import org.tron.core.services.interfaceJsonRpcOnPBFT.JsonRpcOnPBFTServlet; +import org.tron.core.services.interfaceJsonRpcOnSolidity.JsonRpcOnSolidityServlet; +import org.tron.core.services.interfaceOnPBFT.WalletOnPBFT; +import org.tron.core.services.interfaceOnSolidity.WalletOnSolidity; +import org.tron.core.services.jsonrpc.JsonRpcServlet; +import org.tron.core.services.ratelimiter.GlobalRateLimiter; +import org.tron.core.services.ratelimiter.RateLimiterContainer; +import org.tron.core.services.ratelimiter.RuntimeData; +import org.tron.core.services.ratelimiter.adapter.IRateLimiter; + +@RunWith(Parameterized.class) +public class JsonRpcRateLimiterServletTest { + + private final Class servletClass; + private RateLimiterServlet servlet; + private IRateLimiter perEndpoint; + private Object dispatcher; + private MockHttpServletRequest request; + private MockHttpServletResponse response; + + public JsonRpcRateLimiterServletTest(Class servletClass) { + this.servletClass = servletClass; + } + + @Parameterized.Parameters(name = "{0}") + public static Collection servlets() { + return Arrays.asList(new Object[][] { + {JsonRpcServlet.class}, + {JsonRpcOnSolidityServlet.class}, + {JsonRpcOnPBFTServlet.class} + }); + } + + @Before + public void setUp() throws Exception { + // Initialize Args before GlobalRateLimiter's static QPS limiters are loaded. + Args.setParam(new String[0], TestConstants.TEST_CONF); + servlet = servletClass.getDeclaredConstructor().newInstance(); + RateLimiterContainer container = new RateLimiterContainer(); + perEndpoint = mock(IRateLimiter.class); + container.add("http_", servletClass.getSimpleName(), perEndpoint); + ReflectionTestUtils.setField(servlet, "container", container); + + if (servlet instanceof JsonRpcOnSolidityServlet) { + dispatcher = mock(WalletOnSolidity.class); + ReflectionTestUtils.setField(servlet, "walletOnSolidity", dispatcher); + } else if (servlet instanceof JsonRpcOnPBFTServlet) { + dispatcher = mock(WalletOnPBFT.class); + ReflectionTestUtils.setField(servlet, "walletOnPBFT", dispatcher); + } else { + dispatcher = mock(JsonRpcServer.class); + ReflectionTestUtils.setField(servlet, "rpcServer", dispatcher); + } + + request = new MockHttpServletRequest("POST", "/jsonrpc"); + request.setServletPath("/jsonrpc"); + request.setRemoteAddr("10.0.0.1"); + request.setContentType("application/json"); + request.setContent("{\"jsonrpc\":\"2.0\",\"method\":\"eth_blockNumber\",\"id\":1}" + .getBytes(StandardCharsets.UTF_8)); + response = new MockHttpServletResponse(); + } + + @After + public void tearDown() { + Args.clearParam(); + } + + @Test + public void testPerEndpointRejectionReturnsSanitizedHttpError() throws Exception { + when(perEndpoint.acquirePermit(any(RuntimeData.class))).thenReturn(false); + + try (MockedStatic global = mockStatic(GlobalRateLimiter.class)) { + servlet.service(request, response); + + global.verify(() -> GlobalRateLimiter.acquirePermit(any()), never()); + assertRateLimitResponse(); + } + } + + @Test + public void testGlobalRejectionReturnsSanitizedHttpError() throws Exception { + when(perEndpoint.acquirePermit(any(RuntimeData.class))).thenReturn(true); + + try (MockedStatic global = mockStatic(GlobalRateLimiter.class)) { + global.when(() -> GlobalRateLimiter.acquirePermit(any())).thenReturn(false); + + servlet.service(request, response); + + global.verify(() -> GlobalRateLimiter.acquirePermit(any())); + assertRateLimitResponse(); + } + } + + private void assertRateLimitResponse() throws Exception { + assertEquals(200, response.getStatus()); + assertEquals("application/json; charset=utf-8", response.getContentType()); + assertEquals("{\"Error\":\"lack of computing resources\"}", + response.getContentAsString().trim()); + verify(perEndpoint).acquirePermit(any(RuntimeData.class)); + verifyNoInteractions(dispatcher); + } +} diff --git a/framework/src/test/java/org/tron/core/services/http/UtilProcessErrorTest.java b/framework/src/test/java/org/tron/core/services/http/UtilProcessErrorTest.java new file mode 100644 index 00000000000..61bdff78078 --- /dev/null +++ b/framework/src/test/java/org/tron/core/services/http/UtilProcessErrorTest.java @@ -0,0 +1,93 @@ +package org.tron.core.services.http; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; + +import com.google.protobuf.InvalidProtocolBufferException; +import org.bouncycastle.util.encoders.DecoderException; +import org.bouncycastle.util.encoders.Hex; +import org.junit.Test; +import org.springframework.mock.web.MockHttpServletResponse; +import org.tron.core.exception.ContractValidateException; +import org.tron.core.exception.HeaderNotFound; +import org.tron.core.exception.MaintenanceUnavailableException; +import org.tron.core.exception.ZkProofValidateException; +import org.tron.json.JSONException; +import org.tron.json.JSONObject; + +public class UtilProcessErrorTest { + + private static final String INTERNAL_SERVER_ERROR = "internal server error"; + private static final String RATE_LIMITER_ERROR_MSG = "lack of computing resources"; + + @Test + public void exactCompatibilityTypesPreserveNonBlankMessage() throws Exception { + assertError(new JsonFormat.ParseException("1:2: invalid \"field\"\nvalue"), + "1:2: invalid \"field\"\nvalue"); + assertError(new ContractValidateException("balance is not sufficient"), + "balance is not sufficient"); + assertError(new MaintenanceUnavailableException("maintenance in progress"), + "maintenance in progress"); + } + + @Test + public void unclassifiedTypesFailClosed() throws Exception { + DecoderException decoder = assertThrows(DecoderException.class, () -> Hex.decode("zz")); + Exception[] errors = { + new NullPointerException("internal field name"), + new JSONException("server serialization detail"), + new InvalidProtocolBufferException("stored protobuf detail"), + decoder, + new HeaderNotFound("latest block not found"), + new IllegalArgumentException("No enum constant internal.Type.VALUE"), + new IllegalAccessException(RATE_LIMITER_ERROR_MSG), + new IllegalAccessException("other access failure"), + new ZkProofValidateException("wrapped validation detail", true) + }; + + for (Exception error : errors) { + assertError(error, INTERNAL_SERVER_ERROR); + } + } + + @Test + public void onlyExactFixedControlSignalsArePreserved() throws Exception { + assertError(new IllegalArgumentException(Util.EVENTS_DEPRECATED_MSG), + Util.EVENTS_DEPRECATED_MSG); + assertError(new IllegalArgumentException("other argument failure"), INTERNAL_SERVER_ERROR); + assertError(new NumberFormatException(Util.EVENTS_DEPRECATED_MSG), INTERNAL_SERVER_ERROR); + } + + @Test + public void nullBlankAndSubclassMessagesFailClosed() throws Exception { + assertError(null, INTERNAL_SERVER_ERROR); + assertError(new JsonFormat.ParseException(null), INTERNAL_SERVER_ERROR); + assertError(new JsonFormat.ParseException(""), INTERNAL_SERVER_ERROR); + assertError(new JsonFormat.ParseException(" "), INTERNAL_SERVER_ERROR); + assertError(new ContractValidateException("subclass message") { }, INTERNAL_SERVER_ERROR); + } + + @Test + public void auditedErrorWriterPreservesTextVerbatim() throws Exception { + for (String audited : new String[] {Util.INVALID_ADDRESS_MSG, Util.RATE_LIMITER_ERROR_MSG}) { + MockHttpServletResponse response = new MockHttpServletResponse(); + Util.writeAuditedError(audited, response); + JSONObject body = JSONObject.parseObject(response.getContentAsString()); + assertEquals(audited, body.getString("Error")); + } + } + + @Test + public void auditedErrorWriterWithNullMessageWritesEmptyObject() throws Exception { + MockHttpServletResponse response = new MockHttpServletResponse(); + Util.writeAuditedError(null, response); + assertEquals("{}", response.getContentAsString().trim()); + } + + private static void assertError(Exception error, String expected) throws Exception { + MockHttpServletResponse response = new MockHttpServletResponse(); + Util.processError(error, response); + JSONObject body = JSONObject.parseObject(response.getContentAsString()); + assertEquals(expected, body.getString("Error")); + } +} diff --git a/framework/src/test/java/org/tron/core/services/http/solidity/GetTransactionByIdSolidityServletTest.java b/framework/src/test/java/org/tron/core/services/http/solidity/GetTransactionByIdSolidityServletTest.java index e1abb41d1e1..cacb904d9b9 100644 --- a/framework/src/test/java/org/tron/core/services/http/solidity/GetTransactionByIdSolidityServletTest.java +++ b/framework/src/test/java/org/tron/core/services/http/solidity/GetTransactionByIdSolidityServletTest.java @@ -1,202 +1,146 @@ package org.tron.core.services.http.solidity; -import static org.mockito.BDDMockito.given; +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; -import java.io.BufferedReader; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.File; -import java.io.FileInputStream; -import java.io.IOException; -import java.io.InputStreamReader; -import java.io.OutputStreamWriter; -import java.io.PrintStream; -import java.io.PrintWriter; -import java.net.HttpURLConnection; -import java.net.URL; -import java.net.URLStreamHandlerFactory; -import java.nio.charset.StandardCharsets; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import lombok.extern.slf4j.Slf4j; +import com.google.protobuf.ByteString; +import java.util.Arrays; +import java.util.Collection; import org.junit.After; -import org.junit.Assert; import org.junit.Before; -import org.junit.BeforeClass; import org.junit.Test; -import org.tron.common.utils.FileUtil; -import org.tron.common.utils.PublicMethod; -import org.tron.core.services.http.solidity.mockito.HttpUrlStreamHandler; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.test.util.ReflectionTestUtils; +import org.tron.common.utils.ByteArray; +import org.tron.common.utils.Sha256Hash; +import org.tron.core.Wallet; +import org.tron.core.config.args.Args; +import org.tron.json.JSONObject; +import org.tron.protos.Protocol.Transaction; + +@RunWith(Parameterized.class) +public class GetTransactionByIdSolidityServletTest { + private static final String TRANSACTION_ID = + "309b6fa3d01353e46f57dd8a8f27611f98e392b50d035cef213f2c55225a8bd2"; + private static final ByteString TRANSACTION_ID_BYTES = + ByteString.copyFrom(ByteArray.fromHexString(TRANSACTION_ID)); -@Slf4j -public class GetTransactionByIdSolidityServletTest { + @Parameter + public String method; - private static HttpUrlStreamHandler httpUrlStreamHandler; - private GetTransactionByIdSolidityServlet getTransactionByIdSolidityServlet; - private HttpServletRequest request; - private HttpServletResponse response; - private HttpURLConnection httpUrlConnection; - private OutputStreamWriter outputStreamWriter; - private URL url; - - /** - * . - */ - @BeforeClass - public static void init() { - // Allows for mocking URL connections - URLStreamHandlerFactory urlStreamHandlerFactory = mock(URLStreamHandlerFactory.class); - try { - URL.setURLStreamHandlerFactory(urlStreamHandlerFactory); - } catch (Error e) { - logger.info("Ignore error: {}", e.getMessage()); - } + private GetTransactionByIdSolidityServlet servlet; + private Wallet wallet; + private long savedMaxMessageSize; - httpUrlStreamHandler = new HttpUrlStreamHandler(); - given(urlStreamHandlerFactory.createURLStreamHandler("http")).willReturn(httpUrlStreamHandler); + @Parameters(name = "{0}") + public static Collection methods() { + return Arrays.asList(new Object[][] {{"GET"}, {"POST"}}); } - /** - * Init. - */ - @Before public void setUp() { - getTransactionByIdSolidityServlet = new GetTransactionByIdSolidityServlet(); - this.request = mock(HttpServletRequest.class); - this.response = mock(HttpServletResponse.class); - this.httpUrlConnection = mock(HttpURLConnection.class); - this.outputStreamWriter = mock(OutputStreamWriter.class); - httpUrlStreamHandler.resetConnections(); + savedMaxMessageSize = Args.getInstance().getHttpMaxMessageSize(); + Args.getInstance().setHttpMaxMessageSize(1024); + servlet = new GetTransactionByIdSolidityServlet(); + wallet = mock(Wallet.class); + ReflectionTestUtils.setField(servlet, "wallet", wallet); } - /** - * Release Resource. - */ @After public void tearDown() { - if (FileUtil.deleteDir(new File("temp.txt"))) { - logger.info("Release resources successful."); + Args.getInstance().setHttpMaxMessageSize(savedMaxMessageSize); + } + + @Test + public void walletFailureReturnsSanitizedJson() throws Exception { + when(wallet.getTransactionById(TRANSACTION_ID_BYTES)) + .thenThrow(new NullPointerException("internal transaction store detail")); + + MockHttpServletResponse response = request(TRANSACTION_ID); + + assertEquals("internal server error", errorMessage(response)); + verify(wallet).getTransactionById(TRANSACTION_ID_BYTES); + } + + @Test + public void invalidHexReturnsJsonWithoutCallingWallet() throws Exception { + MockHttpServletResponse response = request("zz"); + + String message = errorMessage(response); + if ("GET".equals(method)) { + assertEquals("internal server error", message); } else { - logger.info("Release resources failure."); + assertTrue(message.matches("\\d+:\\d+: INVALID hex String")); } + verifyNoInteractions(wallet); } @Test - public void doPostTest() throws IOException { - - //send Post request - - final ByteArrayOutputStream outContent = new ByteArrayOutputStream(); - System.setOut(new PrintStream(outContent)); - String href = "http://127.0.0.1:" - + PublicMethod.chooseRandomPort() + "/walletsolidity/gettransactioninfobyid"; - httpUrlStreamHandler.addConnection(new URL(href), httpUrlConnection); - httpUrlConnection.setRequestMethod("POST"); - httpUrlConnection.setRequestProperty("Content-Type", "application/json"); - httpUrlConnection.setRequestProperty("Connection", "Keep-Alive"); - httpUrlConnection.setUseCaches(false); - httpUrlConnection.setDoOutput(true); - String postData = "{\"value\": \"309b6fa3d01353e46f57dd8a8f27611f98e392b50d035cef21" - + "3f2c55225a8bd2\"}"; - httpUrlConnection.setRequestProperty("Content-Length", String.valueOf(postData.length())); - - when(httpUrlConnection.getOutputStream()).thenReturn(outContent); - OutputStreamWriter out = new OutputStreamWriter(httpUrlConnection.getOutputStream(), - StandardCharsets.UTF_8); - out.write(postData); - out.flush(); - out.close(); - PrintWriter writer = new PrintWriter("temp.txt"); - when(response.getWriter()).thenReturn(writer); - - getTransactionByIdSolidityServlet.doPost(request, response); - // Get Response Body - String line; - StringBuilder result = new StringBuilder(); - - byte[] buffer = new byte[1024]; - ByteArrayInputStream byteArrayInputStream = new ByteArrayInputStream(buffer); - when(httpUrlConnection.getInputStream()).thenReturn(byteArrayInputStream); - BufferedReader in = new BufferedReader(new InputStreamReader(httpUrlConnection.getInputStream(), - StandardCharsets.UTF_8)); - - while ((line = in.readLine()) != null) { - result.append(line).append("\n"); - } - Assert.assertNotNull(result); - in.close(); - writer.flush(); - FileInputStream fileInputStream = new FileInputStream("temp.txt"); - InputStreamReader inputStreamReader = new InputStreamReader(fileInputStream); - BufferedReader bufferedReader = new BufferedReader(inputStreamReader); - - StringBuilder sb = new StringBuilder(); - String text; - while ((text = bufferedReader.readLine()) != null) { - sb.append(text); - } - Assert.assertTrue(sb.toString().contains("null")); - httpUrlConnection.disconnect(); + public void missingTransactionKeepsEmptyObject() throws Exception { + MockHttpServletResponse response = request(TRANSACTION_ID); + + assertEquals(200, response.getStatus()); + assertEquals("{}", response.getContentAsString().trim()); + verify(wallet).getTransactionById(TRANSACTION_ID_BYTES); } @Test - public void doGetTest() throws IOException { - - final ByteArrayOutputStream outContent = new ByteArrayOutputStream(); - System.setOut(new PrintStream(outContent)); - String href = "http://127.0.0.1:" - + PublicMethod.chooseRandomPort() + "/walletsolidity/gettransactioninfobyid"; - httpUrlStreamHandler.addConnection(new URL(href), httpUrlConnection); - httpUrlConnection.setRequestMethod("GET"); - httpUrlConnection.setRequestProperty("Content-Type", "application/json"); - httpUrlConnection.setRequestProperty("Connection", "Keep-Alive"); - httpUrlConnection.setUseCaches(false); - httpUrlConnection.setDoOutput(true); - String postData = "{\"value\": \"309b6fa3d01353e46f57dd8a8f27611f98e392b50d035cef21" - + "3f2c55225a8bd2\"}"; - httpUrlConnection.setRequestProperty("Content-Length", String.valueOf(postData.length())); - - when(httpUrlConnection.getOutputStream()).thenReturn(outContent); - OutputStreamWriter out = new OutputStreamWriter(httpUrlConnection.getOutputStream(), - StandardCharsets.UTF_8); - out.write(postData); - out.flush(); - out.close(); - PrintWriter writer = new PrintWriter("temp.txt"); - when(response.getWriter()).thenReturn(writer); - - getTransactionByIdSolidityServlet.doPost(request, response); - // Get Response Body - String line; - StringBuilder result = new StringBuilder(); - - byte[] buffer = new byte[1024]; - ByteArrayInputStream byteArrayInputStream = new ByteArrayInputStream(buffer); - when(httpUrlConnection.getInputStream()).thenReturn(byteArrayInputStream); - BufferedReader in = new BufferedReader(new InputStreamReader(httpUrlConnection.getInputStream(), - StandardCharsets.UTF_8)); - - while ((line = in.readLine()) != null) { - result.append(line).append("\n"); - } - Assert.assertNotNull(result); - in.close(); - writer.flush(); - FileInputStream fileInputStream = new FileInputStream("temp.txt"); - InputStreamReader inputStreamReader = new InputStreamReader(fileInputStream); - BufferedReader bufferedReader = new BufferedReader(inputStreamReader); - - StringBuilder sb = new StringBuilder(); - String text; - while ((text = bufferedReader.readLine()) != null) { - sb.append(text); + public void successfulLookupKeepsTransaction() throws Exception { + ByteString signature = ByteString.copyFromUtf8("transaction signature"); + Transaction transaction = Transaction.newBuilder() + .setRawData(Transaction.raw.newBuilder().setTimestamp(123).setExpiration(456)) + .addSignature(signature).build(); + when(wallet.getTransactionById(TRANSACTION_ID_BYTES)).thenReturn(transaction); + + MockHttpServletResponse response = request(TRANSACTION_ID); + + assertEquals(200, response.getStatus()); + JSONObject body = JSONObject.parseObject(response.getContentAsString()); + assertEquals(4, body.size()); + JSONObject rawData = body.getJSONObject("raw_data"); + assertEquals(123L, rawData.getLongValue("timestamp")); + assertEquals(456L, rawData.getLongValue("expiration")); + assertEquals(0, rawData.getJSONArray("contract").size()); + assertEquals(ByteArray.toHexString(transaction.getRawData().toByteArray()), + body.getString("raw_data_hex")); + assertEquals(Sha256Hash.of(Args.getInstance().isECKeyCryptoEngine(), + transaction.getRawData().toByteArray()).toString(), body.getString("txID")); + assertEquals(1, body.getJSONArray("signature").size()); + assertEquals(ByteArray.toHexString(signature.toByteArray()), + body.getJSONArray("signature").getString(0)); + verify(wallet).getTransactionById(TRANSACTION_ID_BYTES); + } + + private MockHttpServletResponse request(String value) throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest(method, + "/walletsolidity/gettransactionbyid"); + MockHttpServletResponse response = new MockHttpServletResponse(); + if ("GET".equals(method)) { + request.setParameter("value", value); + servlet.doGet(request, response); + } else { + request.setContentType("application/json"); + request.setContent(("{\"value\":\"" + value + "\"}").getBytes(UTF_8)); + servlet.doPost(request, response); } - Assert.assertTrue(sb.toString().contains("null")); - httpUrlConnection.disconnect(); + return response; } -} + private static String errorMessage(MockHttpServletResponse response) throws Exception { + assertEquals(200, response.getStatus()); + JSONObject body = JSONObject.parseObject(response.getContentAsString()); + assertEquals(1, body.size()); + return body.getString("Error"); + } +} diff --git a/framework/src/test/java/org/tron/core/services/http/solidity/GetTransactionInfoByIdSolidityServletTest.java b/framework/src/test/java/org/tron/core/services/http/solidity/GetTransactionInfoByIdSolidityServletTest.java new file mode 100644 index 00000000000..a8810114f82 --- /dev/null +++ b/framework/src/test/java/org/tron/core/services/http/solidity/GetTransactionInfoByIdSolidityServletTest.java @@ -0,0 +1,135 @@ +package org.tron.core.services.http.solidity; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +import com.google.protobuf.ByteString; +import java.util.Arrays; +import java.util.Collection; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.test.util.ReflectionTestUtils; +import org.tron.common.utils.ByteArray; +import org.tron.core.Wallet; +import org.tron.core.config.args.Args; +import org.tron.json.JSONObject; +import org.tron.protos.Protocol.TransactionInfo; + +@RunWith(Parameterized.class) +public class GetTransactionInfoByIdSolidityServletTest { + + private static final String TRANSACTION_ID = + "309b6fa3d01353e46f57dd8a8f27611f98e392b50d035cef213f2c55225a8bd2"; + private static final ByteString TRANSACTION_ID_BYTES = + ByteString.copyFrom(ByteArray.fromHexString(TRANSACTION_ID)); + + @Parameter + public String method; + + private GetTransactionInfoByIdSolidityServlet servlet; + private Wallet wallet; + private long savedMaxMessageSize; + + @Parameters(name = "{0}") + public static Collection methods() { + return Arrays.asList(new Object[][] {{"GET"}, {"POST"}}); + } + + @Before + public void setUp() { + savedMaxMessageSize = Args.getInstance().getHttpMaxMessageSize(); + Args.getInstance().setHttpMaxMessageSize(1024); + servlet = new GetTransactionInfoByIdSolidityServlet(); + wallet = mock(Wallet.class); + ReflectionTestUtils.setField(servlet, "wallet", wallet); + } + + @After + public void tearDown() { + Args.getInstance().setHttpMaxMessageSize(savedMaxMessageSize); + } + + @Test + public void walletFailureReturnsSanitizedJson() throws Exception { + when(wallet.getTransactionInfoById(TRANSACTION_ID_BYTES)) + .thenThrow(new NullPointerException("internal transaction store detail")); + + MockHttpServletResponse response = request(TRANSACTION_ID); + + assertEquals("internal server error", errorMessage(response)); + verify(wallet).getTransactionInfoById(TRANSACTION_ID_BYTES); + } + + @Test + public void invalidHexReturnsJsonWithoutCallingWallet() throws Exception { + MockHttpServletResponse response = request("zz"); + + String message = errorMessage(response); + if ("GET".equals(method)) { + assertEquals("internal server error", message); + } else { + assertTrue(message.matches("\\d+:\\d+: INVALID hex String")); + } + verifyNoInteractions(wallet); + } + + @Test + public void missingTransactionKeepsEmptyObject() throws Exception { + MockHttpServletResponse response = request(TRANSACTION_ID); + + assertEquals(200, response.getStatus()); + assertEquals("{}", response.getContentAsString().trim()); + verify(wallet).getTransactionInfoById(TRANSACTION_ID_BYTES); + } + + @Test + public void successfulLookupKeepsTransactionInfo() throws Exception { + TransactionInfo info = TransactionInfo.newBuilder() + .setId(TRANSACTION_ID_BYTES).setFee(7).setBlockNumber(123).build(); + when(wallet.getTransactionInfoById(TRANSACTION_ID_BYTES)).thenReturn(info); + + MockHttpServletResponse response = request(TRANSACTION_ID); + + assertEquals(200, response.getStatus()); + JSONObject body = JSONObject.parseObject(response.getContentAsString()); + assertEquals(3, body.size()); + assertEquals(TRANSACTION_ID, body.getString("id")); + assertEquals(7L, body.getLongValue("fee")); + assertEquals(123L, body.getLongValue("blockNumber")); + verify(wallet).getTransactionInfoById(TRANSACTION_ID_BYTES); + } + + private MockHttpServletResponse request(String value) throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest(method, + "/walletsolidity/gettransactioninfobyid"); + MockHttpServletResponse response = new MockHttpServletResponse(); + if ("GET".equals(method)) { + request.setParameter("value", value); + servlet.doGet(request, response); + } else { + request.setContentType("application/json"); + request.setContent(("{\"value\":\"" + value + "\"}").getBytes(UTF_8)); + servlet.doPost(request, response); + } + return response; + } + + private static String errorMessage(MockHttpServletResponse response) throws Exception { + assertEquals(200, response.getStatus()); + JSONObject body = JSONObject.parseObject(response.getContentAsString()); + assertEquals(1, body.size()); + return body.getString("Error"); + } +} From 16e464f6d441b60891cd73031cdccdf11432bb27 Mon Sep 17 00:00:00 2001 From: halibobo1205 Date: Thu, 10 Sep 2026 19:11:23 +0800 Subject: [PATCH 2/2] fix(api): keep server-side failure logging at error level The previous commit routed four catch-all blocks through the shared processError entry point, which logs at debug. Those four catches cover server-side work only: getburntrx, getnodeinfo and getpendingsize read no request parameters, and in getreward malformed addresses are already handled by the preceding DecoderException | IllegalArgumentException catch. Their failures therefore left no trace under the default log configuration, where the API topic is INFO. Add a dedicated processServerError entry point that logs at error and then applies the same sanitization, and use it at those four call sites. Logging the exception once inside the helper keeps a single record at any log level, instead of pairing an error log in the servlet with the debug log in the shared path. The shared Exception entry point keeps debug on purpose: its callers also cover request parsing, so an unauthenticated client can fail it cheaply and repeatedly, and an unconditional stack trace per request would amplify that into log pressure. Distinguishing client from server faults on that path is the parameter/internal split tracked as follow-up in #6936. Client-facing responses are unchanged. --- .../tron/core/services/http/GetBurnTrxServlet.java | 2 +- .../core/services/http/GetNodeInfoServlet.java | 2 +- .../core/services/http/GetPendingSizeServlet.java | 2 +- .../tron/core/services/http/GetRewardServlet.java | 2 +- .../java/org/tron/core/services/http/Util.java | 8 ++++++++ .../core/services/http/UtilProcessErrorTest.java | 14 ++++++++++++++ 6 files changed, 26 insertions(+), 4 deletions(-) diff --git a/framework/src/main/java/org/tron/core/services/http/GetBurnTrxServlet.java b/framework/src/main/java/org/tron/core/services/http/GetBurnTrxServlet.java index 26740ffed0f..3a19825ba75 100644 --- a/framework/src/main/java/org/tron/core/services/http/GetBurnTrxServlet.java +++ b/framework/src/main/java/org/tron/core/services/http/GetBurnTrxServlet.java @@ -23,7 +23,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) { : "{\"burnTrxAmount\": " + value + "}"; response.getWriter().println(out); } catch (Exception e) { - Util.processError(e, response); + Util.processServerError(e, response); } } diff --git a/framework/src/main/java/org/tron/core/services/http/GetNodeInfoServlet.java b/framework/src/main/java/org/tron/core/services/http/GetNodeInfoServlet.java index b1746e1ff1f..c8b4aa39785 100644 --- a/framework/src/main/java/org/tron/core/services/http/GetNodeInfoServlet.java +++ b/framework/src/main/java/org/tron/core/services/http/GetNodeInfoServlet.java @@ -23,7 +23,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) { response.getWriter().println(JSON.toJSONString(nodeInfo)); } catch (Exception e) { - Util.processError(e, response); + Util.processServerError(e, response); } } diff --git a/framework/src/main/java/org/tron/core/services/http/GetPendingSizeServlet.java b/framework/src/main/java/org/tron/core/services/http/GetPendingSizeServlet.java index 6369600352d..41a47c49001 100644 --- a/framework/src/main/java/org/tron/core/services/http/GetPendingSizeServlet.java +++ b/framework/src/main/java/org/tron/core/services/http/GetPendingSizeServlet.java @@ -23,7 +23,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) { : "{\"pendingSize\": " + value + "}"; response.getWriter().println(out); } catch (Exception e) { - Util.processError(e, response); + Util.processServerError(e, response); } } diff --git a/framework/src/main/java/org/tron/core/services/http/GetRewardServlet.java b/framework/src/main/java/org/tron/core/services/http/GetRewardServlet.java index a7278e054b0..780bab6ac94 100644 --- a/framework/src/main/java/org/tron/core/services/http/GetRewardServlet.java +++ b/framework/src/main/java/org/tron/core/services/http/GetRewardServlet.java @@ -30,7 +30,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) { } catch (DecoderException | IllegalArgumentException e) { Util.writeAuditedError(Util.INVALID_ADDRESS_MSG, response); } catch (Exception e) { - Util.processError(e, response); + Util.processServerError(e, response); } } diff --git a/framework/src/main/java/org/tron/core/services/http/Util.java b/framework/src/main/java/org/tron/core/services/http/Util.java index 60d8527aa92..ca20902c4d8 100644 --- a/framework/src/main/java/org/tron/core/services/http/Util.java +++ b/framework/src/main/java/org/tron/core/services/http/Util.java @@ -555,6 +555,14 @@ public static void processError(Exception e, HttpServletResponse response) { writeAuditedError(clientMessage(e), response); } + // For catch blocks that cover server-side work only, so the failure stays visible at the + // default log level. The Exception entry point above keeps debug because its callers also + // cover request parsing, which an unauthenticated client can fail cheaply and repeatedly. + static void processServerError(Exception e, HttpServletResponse response) { + logger.error("HTTP request failed", e); + writeAuditedError(clientMessage(e), response); + } + // Bypasses clientMessage: callers must pass audited fixed or pre-existing client texts only. static void writeAuditedError(String msg, HttpServletResponse response) { try { diff --git a/framework/src/test/java/org/tron/core/services/http/UtilProcessErrorTest.java b/framework/src/test/java/org/tron/core/services/http/UtilProcessErrorTest.java index 61bdff78078..5d4baa34c6f 100644 --- a/framework/src/test/java/org/tron/core/services/http/UtilProcessErrorTest.java +++ b/framework/src/test/java/org/tron/core/services/http/UtilProcessErrorTest.java @@ -84,6 +84,20 @@ public void auditedErrorWriterWithNullMessageWritesEmptyObject() throws Exceptio assertEquals("{}", response.getContentAsString().trim()); } + @Test + public void serverErrorChannelSanitizesLikeTheSharedPath() throws Exception { + assertServerError(new NullPointerException("internal field name"), INTERNAL_SERVER_ERROR); + assertServerError(new ContractValidateException("balance is not sufficient"), + "balance is not sufficient"); + } + + private static void assertServerError(Exception error, String expected) throws Exception { + MockHttpServletResponse response = new MockHttpServletResponse(); + Util.processServerError(error, response); + JSONObject body = JSONObject.parseObject(response.getContentAsString()); + assertEquals(expected, body.getString("Error")); + } + private static void assertError(Exception error, String expected) throws Exception { MockHttpServletResponse response = new MockHttpServletResponse(); Util.processError(error, response);