From e09610c0b27601a75025a1bee48f892eb923b470 Mon Sep 17 00:00:00 2001 From: halibobo1205 Date: Sun, 20 Sep 2026 16:03:59 +0800 Subject: [PATCH 1/2] refactor(api): replace response copy wrappers with Jetty content count --- .../services/filter/CharResponseWrapper.java | 64 ---- .../services/filter/HttpApiAccessFilter.java | 13 +- .../core/services/filter/HttpInterceptor.java | 18 +- .../filter/ServletOutputStreamCopy.java | 39 --- .../filter/HttpInterceptorMetricsTest.java | 314 ++++++++++++++++++ .../services/filter/HttpInterceptorTest.java | 69 +++- 6 files changed, 395 insertions(+), 122 deletions(-) delete mode 100644 framework/src/main/java/org/tron/core/services/filter/CharResponseWrapper.java delete mode 100644 framework/src/main/java/org/tron/core/services/filter/ServletOutputStreamCopy.java create mode 100644 framework/src/test/java/org/tron/core/services/filter/HttpInterceptorMetricsTest.java diff --git a/framework/src/main/java/org/tron/core/services/filter/CharResponseWrapper.java b/framework/src/main/java/org/tron/core/services/filter/CharResponseWrapper.java deleted file mode 100644 index e6421c6a257..00000000000 --- a/framework/src/main/java/org/tron/core/services/filter/CharResponseWrapper.java +++ /dev/null @@ -1,64 +0,0 @@ -package org.tron.core.services.filter; - -import java.io.IOException; -import java.io.OutputStreamWriter; -import java.io.PrintWriter; -import javax.servlet.ServletOutputStream; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpServletResponseWrapper; - -public class CharResponseWrapper extends HttpServletResponseWrapper { - - private ServletOutputStream outputStream; - private PrintWriter writer; - private ServletOutputStreamCopy streamCopy; - - - public CharResponseWrapper(HttpServletResponse response) throws IOException { - super(response); - } - - @Override - public ServletOutputStream getOutputStream() throws IOException { - if (writer != null) { - throw new IllegalStateException("getWriter() has been called ."); - } - - if (outputStream == null) { - outputStream = getResponse().getOutputStream(); - streamCopy = new ServletOutputStreamCopy(outputStream); - } - - return streamCopy; - } - - @Override - public PrintWriter getWriter() throws IOException { - if (outputStream != null) { - throw new IllegalStateException("getOutputStream() has been called."); - } - - if (writer == null) { - streamCopy = new ServletOutputStreamCopy(getResponse().getOutputStream()); - // set auto flash so that copy can be valid - writer = new PrintWriter(new OutputStreamWriter(streamCopy, - getResponse().getCharacterEncoding()), true); - } - - return writer; - } - - @Override - public void flushBuffer() throws IOException { // flush both stream - if (writer != null) { - writer.flush(); - } else if (outputStream != null) { - streamCopy.flush(); - } - } - - public int getByteSize() { - return streamCopy == null ? 0 : streamCopy.getStreamByteSize(); - } - -} \ No newline at end of file diff --git a/framework/src/main/java/org/tron/core/services/filter/HttpApiAccessFilter.java b/framework/src/main/java/org/tron/core/services/filter/HttpApiAccessFilter.java index e18e6541baa..14149934f0b 100644 --- a/framework/src/main/java/org/tron/core/services/filter/HttpApiAccessFilter.java +++ b/framework/src/main/java/org/tron/core/services/filter/HttpApiAccessFilter.java @@ -29,9 +29,9 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha if (request instanceof HttpServletRequest) { String contextPath = ((HttpServletRequest) request).getContextPath(); String endpoint = contextPath + ((HttpServletRequest) request).getServletPath(); - HttpServletResponse resp = (HttpServletResponse) response; if (isDisabled(endpoint)) { + HttpServletResponse resp = (HttpServletResponse) response; resp.setStatus(HttpServletResponse.SC_NOT_FOUND); resp.setContentType("application/json; charset=utf-8"); JSONObject jsonObject = new JSONObject(); @@ -39,14 +39,10 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha resp.getWriter().println(jsonObject.toJSONString()); return; } - - CharResponseWrapper responseWrapper = new CharResponseWrapper(resp); - chain.doFilter(request, responseWrapper); - - } else { - chain.doFilter(request, response); } + chain.doFilter(request, response); + } catch (Exception e) { logger.error("http api access filter exception: {}", e.getMessage()); } @@ -74,6 +70,3 @@ private boolean isDisabled(String endpoint) { } } - - - diff --git a/framework/src/main/java/org/tron/core/services/filter/HttpInterceptor.java b/framework/src/main/java/org/tron/core/services/filter/HttpInterceptor.java index ed20630b780..0367fe25580 100644 --- a/framework/src/main/java/org/tron/core/services/filter/HttpInterceptor.java +++ b/framework/src/main/java/org/tron/core/services/filter/HttpInterceptor.java @@ -10,6 +10,7 @@ import lombok.extern.slf4j.Slf4j; import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.server.Request; import org.tron.common.prometheus.MetricKeys; import org.tron.common.prometheus.MetricLabels; import org.tron.common.prometheus.Metrics; @@ -37,17 +38,15 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha } String contextPath = ((HttpServletRequest) request).getContextPath(); endpoint = contextPath + ((HttpServletRequest) request).getServletPath(); - CharResponseWrapper responseWrapper = new CharResponseWrapper( - (HttpServletResponse) response); - chain.doFilter(request, responseWrapper); + chain.doFilter(request, response); HttpServletResponse resp = (HttpServletResponse) response; - int size = responseWrapper.getByteSize(); + long size = getContentCount(request); MetricsUtil.meterMark(MetricsKey.NET_API_OUT_TRAFFIC, size); MetricsUtil.meterMark(MetricsKey.NET_API_QPS); if (resp.getStatus() >= HTTP_BAD_REQUEST && resp.getStatus() <= HTTP_NOT_ACCEPTABLE) { MetricsUtil.meterMark(MetricsKey.NET_API_FAIL_QPS); Metrics.histogramObserve(MetricKeys.Histogram.HTTP_BYTES, - size, MetricLabels.UNDEFINED, String.valueOf(responseWrapper.getStatus())); + size, MetricLabels.UNDEFINED, String.valueOf(resp.getStatus())); return; } if (resp.getStatus() == HTTP_SUCCESS) { @@ -58,7 +57,7 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha } MetricsUtil.meterMark(MetricsKey.NET_API_DETAIL_OUT_TRAFFIC + endpoint, size); Metrics.histogramObserve(MetricKeys.Histogram.HTTP_BYTES, - size, endpoint, String.valueOf(responseWrapper.getStatus())); + size, endpoint, String.valueOf(resp.getStatus())); } catch (Exception e) { String key = MetricsKey.NET_API_DETAIL_QPS + endpoint; if (MetricsUtil.getMeters(MetricsKey.NET_API_DETAIL_QPS).containsKey(key)) { @@ -74,9 +73,12 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha } } + private long getContentCount(ServletRequest request) { + Request baseRequest = Request.getBaseRequest(request); + return baseRequest == null ? 0L : baseRequest.getResponse().getContentCount(); + } + @Override public void destroy() { } } - - diff --git a/framework/src/main/java/org/tron/core/services/filter/ServletOutputStreamCopy.java b/framework/src/main/java/org/tron/core/services/filter/ServletOutputStreamCopy.java deleted file mode 100644 index d29b4a4fa57..00000000000 --- a/framework/src/main/java/org/tron/core/services/filter/ServletOutputStreamCopy.java +++ /dev/null @@ -1,39 +0,0 @@ -package org.tron.core.services.filter; - -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.OutputStream; -import javax.servlet.ServletOutputStream; -import javax.servlet.WriteListener; - -class ServletOutputStreamCopy extends ServletOutputStream { - - private OutputStream outputStream; - private ByteArrayOutputStream copy; - private int MAX_RESPONSE_SIZE = 4096; - - public ServletOutputStreamCopy(OutputStream outputStream) { - this.outputStream = outputStream; - this.copy = new ByteArrayOutputStream(MAX_RESPONSE_SIZE); - } - - @Override - public void write(int b) throws IOException { - outputStream.write(b); - copy.write(b); - } - - public int getStreamByteSize() { - return this.copy.size(); - } - - @Override - public boolean isReady() { - return false; - } - - @Override - public void setWriteListener(WriteListener writeListener) { - - } -} diff --git a/framework/src/test/java/org/tron/core/services/filter/HttpInterceptorMetricsTest.java b/framework/src/test/java/org/tron/core/services/filter/HttpInterceptorMetricsTest.java new file mode 100644 index 00000000000..a9931dcdf8b --- /dev/null +++ b/framework/src/test/java/org/tron/core/services/filter/HttpInterceptorMetricsTest.java @@ -0,0 +1,314 @@ +package org.tron.core.services.filter; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import io.prometheus.client.CollectorRegistry; +import java.io.IOException; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.util.EnumSet; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import javax.servlet.DispatcherType; +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import org.apache.http.HttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClients; +import org.apache.http.util.EntityUtils; +import org.eclipse.jetty.servlet.FilterHolder; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.tron.common.TestConstants; +import org.tron.common.application.HttpService; +import org.tron.common.parameter.CommonParameter; +import org.tron.common.prometheus.MetricKeys; +import org.tron.common.prometheus.MetricLabels; +import org.tron.common.utils.PublicMethod; +import org.tron.core.config.args.Args; +import org.tron.core.metrics.MetricsKey; +import org.tron.core.metrics.MetricsUtil; + +public class HttpInterceptorMetricsTest { + + private static final String BODY = "{\"blockID\":\"0123456789abcdef\"}"; + private static final int MULTI_BYTE_CODE_POINT = 0x6D4B; + private static final String UTF8_BODY = + "{\"name\":\"" + new String(Character.toChars(MULTI_BYTE_CODE_POINT)) + "\"}"; + private static final int BIG_BODY_SIZE = 200_000; + + private static final String HTTP_BYTES_SUM = MetricKeys.Histogram.HTTP_BYTES + "_sum"; + private static final String HTTP_BYTES_COUNT = MetricKeys.Histogram.HTTP_BYTES + "_count"; + private static final String[] HTTP_BYTES_LABELS = new String[] {"url", "status"}; + + @ClassRule + public static final TemporaryFolder temporaryFolder = new TemporaryFolder(); + + private static MetricsHttpService service; + private static URI serverUri; + private static CloseableHttpClient client; + + public static class PrintlnServlet extends HttpServlet { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { + resp.setContentType("application/json; charset=utf-8"); + resp.getWriter().println(BODY); + } + } + + public static class PrintServlet extends HttpServlet { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { + resp.setContentType("application/json; charset=utf-8"); + resp.getWriter().print(BODY); + } + } + + public static class StreamServlet extends HttpServlet { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { + byte[] bytes = BODY.getBytes(StandardCharsets.UTF_8); + resp.setContentType("application/json-rpc"); + resp.setContentLength(bytes.length); + resp.getOutputStream().write(bytes); + resp.getOutputStream().flush(); + } + } + + public static class Utf8Servlet extends HttpServlet { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { + resp.setContentType("application/json; charset=utf-8"); + resp.getWriter().println(UTF8_BODY); + } + } + + public static class BigBodyServlet extends HttpServlet { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { + resp.setContentType("application/json; charset=utf-8"); + resp.getWriter().print(bigBody()); + } + } + + public static class ErrorStatusServlet extends HttpServlet { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { + resp.setStatus(HttpServletResponse.SC_BAD_REQUEST); + resp.setContentType("application/json; charset=utf-8"); + resp.getWriter().println(BODY); + } + } + + public static class CompletionLatchFilter implements Filter { + + private static volatile CountDownLatch latch = new CountDownLatch(0); + + static void expectOneRequest() { + latch = new CountDownLatch(1); + } + + static boolean awaitRequestAccounted() throws InterruptedException { + return latch.await(10, TimeUnit.SECONDS); + } + + @Override + public void init(FilterConfig filterConfig) { + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) + throws IOException, ServletException { + try { + chain.doFilter(request, response); + } finally { + latch.countDown(); + } + } + + @Override + public void destroy() { + } + } + + static class MetricsHttpService extends HttpService { + MetricsHttpService(int port) { + this.port = port; + this.contextPath = "/"; + } + + @Override + protected void addServlet(ServletContextHandler context) { + context.addServlet(new ServletHolder(new PrintlnServlet()), "/wallet/println"); + context.addServlet(new ServletHolder(new PrintServlet()), "/wallet/print"); + context.addServlet(new ServletHolder(new StreamServlet()), "/wallet/stream"); + context.addServlet(new ServletHolder(new Utf8Servlet()), "/wallet/utf8"); + context.addServlet(new ServletHolder(new BigBodyServlet()), "/wallet/big"); + context.addServlet(new ServletHolder(new ErrorStatusServlet()), "/wallet/error"); + } + + @Override + protected void addFilter(ServletContextHandler context) { + context.addFilter(new FilterHolder(new CompletionLatchFilter()), "/*", + EnumSet.of(DispatcherType.REQUEST)); + context.addFilter(new FilterHolder(new HttpApiAccessFilter()), "/*", + EnumSet.allOf(DispatcherType.class)); + ServletHandler handler = new ServletHandler(); + FilterHolder fh = handler.addFilterWithMapping(HttpInterceptor.class, "/*", + EnumSet.of(DispatcherType.REQUEST)); + context.addFilter(fh, "/*", EnumSet.of(DispatcherType.REQUEST)); + } + } + + @BeforeClass + public static void setup() throws Exception { + Args.setParam(new String[] {"-d", temporaryFolder.newFolder().toString()}, + TestConstants.TEST_CONF); + CommonParameter.getInstance().setNodeMetricsEnable(true); + CommonParameter.getInstance().setMetricsPrometheusEnable(true); + + int port = PublicMethod.chooseRandomPort(); + service = new MetricsHttpService(port); + service.start().get(10, TimeUnit.SECONDS); + serverUri = new URI(String.format("http://localhost:%d/", port)); + client = HttpClients.createDefault(); + } + + @AfterClass + public static void teardown() throws Exception { + try { + if (client != null) { + client.close(); + } + } finally { + try { + if (service != null) { + service.stop(); + } + } finally { + Args.clearParam(); + } + } + } + + @Test + public void testPrintlnBodyIsCountedExactly() throws Exception { + assertTrafficMatchesWire("/wallet/println", BODY + System.lineSeparator()); + } + + @Test + public void testPrintBodyIsCountedExactly() throws Exception { + assertTrafficMatchesWire("/wallet/print", BODY); + } + + @Test + public void testOutputStreamBodyIsCountedExactly() throws Exception { + assertTrafficMatchesWire("/wallet/stream", BODY); + } + + @Test + public void testUtf8BodyIsCountedInBytesNotCharacters() throws Exception { + assertTrue("the UTF-8 body must be longer in bytes than in characters", + UTF8_BODY.getBytes(StandardCharsets.UTF_8).length > UTF8_BODY.length()); + assertTrafficMatchesWire("/wallet/utf8", UTF8_BODY + System.lineSeparator()); + } + + @Test + public void testBodyLargerThanOutputBufferIsCountedExactly() throws Exception { + assertTrafficMatchesWire("/wallet/big", bigBody()); + } + + @Test + public void testErrorStatusReportsGlobalTrafficOnly() throws Exception { + String path = "/wallet/error"; + String detailKey = MetricsKey.NET_API_DETAIL_OUT_TRAFFIC + path; + long trafficBefore = meterCount(MetricsKey.NET_API_OUT_TRAFFIC); + long detailBefore = meterCount(detailKey); + long failBefore = meterCount(MetricsKey.NET_API_FAIL_QPS); + double histogramBefore = httpBytesSum(MetricLabels.UNDEFINED, "400"); + + CompletionLatchFilter.expectOneRequest(); + HttpResponse resp = client.execute(new HttpGet(serverUri.resolve(path))); + assertEquals(400, resp.getStatusLine().getStatusCode()); + byte[] wire = EntityUtils.toByteArray(resp.getEntity()); + assertEquals(BODY + System.lineSeparator(), new String(wire, StandardCharsets.UTF_8)); + assertTrue("the filter must finish accounting before the metrics are read", + CompletionLatchFilter.awaitRequestAccounted()); + + assertEquals("global out-traffic must equal the bytes on the wire", + trafficBefore + wire.length, meterCount(MetricsKey.NET_API_OUT_TRAFFIC)); + assertEquals("a 4xx must not reach the per-endpoint traffic meter", + detailBefore, meterCount(detailKey)); + assertEquals("a 4xx must be counted as a failed call", + failBefore + 1, meterCount(MetricsKey.NET_API_FAIL_QPS)); + assertEquals("the 4xx histogram is labelled undefined, not with the endpoint", + histogramBefore + wire.length, httpBytesSum(MetricLabels.UNDEFINED, "400"), 0.0); + } + + private void assertTrafficMatchesWire(String path, String expectedBody) throws Exception { + String detailKey = MetricsKey.NET_API_DETAIL_OUT_TRAFFIC + path; + long trafficBefore = meterCount(MetricsKey.NET_API_OUT_TRAFFIC); + long detailBefore = meterCount(detailKey); + double histogramBefore = httpBytesSum(path, "200"); + double observationsBefore = httpBytesCount(path, "200"); + + CompletionLatchFilter.expectOneRequest(); + HttpResponse resp = client.execute(new HttpGet(serverUri.resolve(path))); + assertEquals(200, resp.getStatusLine().getStatusCode()); + byte[] wire = EntityUtils.toByteArray(resp.getEntity()); + assertTrue("the filter must finish accounting before the metrics are read", + CompletionLatchFilter.awaitRequestAccounted()); + + assertEquals("the servlet body must reach the client intact", + expectedBody, new String(wire, StandardCharsets.UTF_8)); + assertEquals("global out-traffic must equal the bytes on the wire", + trafficBefore + wire.length, meterCount(MetricsKey.NET_API_OUT_TRAFFIC)); + assertEquals("per-endpoint out-traffic must equal the bytes on the wire", + detailBefore + wire.length, meterCount(detailKey)); + assertEquals("the histogram must be observed once, labelled with the endpoint", + observationsBefore + 1, httpBytesCount(path, "200"), 0.0); + assertEquals("the histogram must record the bytes on the wire", + histogramBefore + wire.length, httpBytesSum(path, "200"), 0.0); + } + + private long meterCount(String key) { + return MetricsUtil.getMeter(key).getCount(); + } + + private double httpBytesSum(String url, String status) { + return sampleValue(HTTP_BYTES_SUM, url, status); + } + + private double httpBytesCount(String url, String status) { + return sampleValue(HTTP_BYTES_COUNT, url, status); + } + + private double sampleValue(String name, String url, String status) { + Double value = CollectorRegistry.defaultRegistry.getSampleValue(name, HTTP_BYTES_LABELS, + new String[] {url, status}); + return value == null ? 0d : value; + } + + private static String bigBody() { + StringBuilder sb = new StringBuilder(BIG_BODY_SIZE); + while (sb.length() < BIG_BODY_SIZE) { + sb.append('a'); + } + return sb.toString(); + } +} diff --git a/framework/src/test/java/org/tron/core/services/filter/HttpInterceptorTest.java b/framework/src/test/java/org/tron/core/services/filter/HttpInterceptorTest.java index b293e8047b5..71f6cff6da3 100644 --- a/framework/src/test/java/org/tron/core/services/filter/HttpInterceptorTest.java +++ b/framework/src/test/java/org/tron/core/services/filter/HttpInterceptorTest.java @@ -1,17 +1,27 @@ package org.tron.core.services.filter; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import javax.servlet.FilterChain; import javax.servlet.ServletException; import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Response; import org.junit.Test; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; +import org.tron.common.parameter.CommonParameter; +import org.tron.core.metrics.MetricsKey; +import org.tron.core.metrics.MetricsUtil; public class HttpInterceptorTest { + private static final String ENDPOINT = "/wallet/getnowblock"; + private final HttpInterceptor interceptor = new HttpInterceptor(); @Test @@ -27,7 +37,7 @@ public void testOversizedBadMessagePropagates() { BadMessageException e = assertThrows(BadMessageException.class, () -> interceptor.doFilter(request, response, chain)); - org.junit.Assert.assertEquals(HttpStatus.PAYLOAD_TOO_LARGE_413, e.getCode()); + assertEquals(HttpStatus.PAYLOAD_TOO_LARGE_413, e.getCode()); } @Test @@ -41,4 +51,61 @@ public void testNonOversizedExceptionIsStillSwallowed() throws Exception { interceptor.doFilter(request, response, chain); } + + @Test + public void testNonJettyRequestRecordsZeroSizeAndNoFailure() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest("GET", ENDPOINT); + request.setServletPath(ENDPOINT); + MockHttpServletResponse response = new MockHttpServletResponse(); + + boolean metricsWereEnabled = CommonParameter.getInstance().isNodeMetricsEnable(); + CommonParameter.getInstance().setNodeMetricsEnable(true); + try { + long trafficBefore = meterCount(MetricsKey.NET_API_OUT_TRAFFIC); + long qpsBefore = meterCount(MetricsKey.NET_API_QPS); + long failBefore = meterCount(MetricsKey.NET_API_FAIL_QPS); + + interceptor.doFilter(request, response, (req, resp) -> resp.getWriter().print("body")); + + assertEquals("body", response.getContentAsString()); + assertEquals(trafficBefore, meterCount(MetricsKey.NET_API_OUT_TRAFFIC)); + assertEquals(qpsBefore + 1, meterCount(MetricsKey.NET_API_QPS)); + assertEquals(failBefore, meterCount(MetricsKey.NET_API_FAIL_QPS)); + } finally { + CommonParameter.getInstance().setNodeMetricsEnable(metricsWereEnabled); + } + } + + @Test + public void testContentCountRecordedAsOutTraffic() throws Exception { + Response jettyResponse = mock(Response.class); + when(jettyResponse.getContentCount()).thenReturn(123L); + Request jettyRequest = mock(Request.class); + when(jettyRequest.getResponse()).thenReturn(jettyResponse); + when(jettyRequest.getContextPath()).thenReturn(""); + when(jettyRequest.getServletPath()).thenReturn(ENDPOINT); + MockHttpServletResponse response = new MockHttpServletResponse(); + + boolean metricsWereEnabled = CommonParameter.getInstance().isNodeMetricsEnable(); + CommonParameter.getInstance().setNodeMetricsEnable(true); + try { + long trafficBefore = meterCount(MetricsKey.NET_API_OUT_TRAFFIC); + long detailBefore = meterCount(MetricsKey.NET_API_DETAIL_OUT_TRAFFIC + ENDPOINT); + long failBefore = meterCount(MetricsKey.NET_API_FAIL_QPS); + + interceptor.doFilter(jettyRequest, response, (req, resp) -> { + }); + + assertEquals(trafficBefore + 123L, meterCount(MetricsKey.NET_API_OUT_TRAFFIC)); + assertEquals(detailBefore + 123L, + meterCount(MetricsKey.NET_API_DETAIL_OUT_TRAFFIC + ENDPOINT)); + assertEquals(failBefore, meterCount(MetricsKey.NET_API_FAIL_QPS)); + } finally { + CommonParameter.getInstance().setNodeMetricsEnable(metricsWereEnabled); + } + } + + private long meterCount(String key) { + return MetricsUtil.getMeter(key).getCount(); + } } From 29a8bfb31a6dfd106d957298c76379184bddc182 Mon Sep 17 00:00:00 2001 From: halibobo1205 Date: Mon, 21 Sep 2026 18:16:26 +0800 Subject: [PATCH 2/2] fix(api): preserve legacy Unicode replacement in JsonFormat Use JDK UTF-8 conversion to replace malformed surrogates in parser error descriptions and isolated low surrogates in string output. Match the legacy writer's question-mark replacement while preserving valid Unicode and parsing behavior. Cover identifier, ABI escape, numeric errors, and low-surrogate output with regression tests, including strict UTF-8 checks through native Jetty responses. --- .../tron/core/services/http/JsonFormat.java | 22 ++- .../services/http/JsonFormatEscapeTest.java | 23 +++ .../http/JsonFormatIdentifierTest.java | 122 ++++++++++++ .../http/JsonFormatUnicodeErrorTest.java | 186 ++++++++++++++++++ 4 files changed, 349 insertions(+), 4 deletions(-) create mode 100644 framework/src/test/java/org/tron/core/services/http/JsonFormatIdentifierTest.java create mode 100644 framework/src/test/java/org/tron/core/services/http/JsonFormatUnicodeErrorTest.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..e9aa801f078 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 @@ -43,6 +43,7 @@ SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT import java.io.IOException; 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; @@ -929,7 +930,8 @@ private static String escapeNameStringText(String input) { * + * notation; isolated low surrogates are replaced with '?' as in the legacy UTF-8 writer + *
  • any other character is printed as-is
    • */ static String escapeText(String input) { StringBuilder builder = new StringBuilder(input.length()); @@ -961,6 +963,8 @@ 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); @@ -1046,7 +1050,8 @@ static String unescapeText(String input) throws InvalidEscapeSequence { } break; default: - throw new InvalidEscapeSequence("Invalid escape sequence: '\\" + c + "'"); + throw new InvalidEscapeSequence( + replaceMalformedSurrogates("Invalid escape sequence: '\\" + c + "'")); } } else { throw new InvalidEscapeSequence("Invalid escape sequence: '\\' at end of string."); @@ -1059,6 +1064,14 @@ static String unescapeText(String input) throws InvalidEscapeSequence { return builder.toString(); } + private static String replaceMalformedSurrogates(String value) { + if (value == null) { + return null; + } + // Match the legacy UTF-8 OutputStreamWriter's replacement of malformed surrogates. + return new String(value.getBytes(StandardCharsets.UTF_8), StandardCharsets.UTF_8); + } + /** * Is this an octal digit. */ @@ -1719,7 +1732,8 @@ public ByteString consumeByteString(final String fieldName, boolean selfType) */ public ParseException parseException(String description) { // Note: People generally prefer one-based line and column numbers. - return new ParseException((line + 1) + ":" + (column + 1) + ": " + description); + return new ParseException((line + 1) + ":" + (column + 1) + ": " + + replaceMalformedSurrogates(description)); } /** @@ -1729,7 +1743,7 @@ public ParseException parseException(String description) { public ParseException parseExceptionPreviousToken(String description) { // Note: People generally prefer one-based line and column numbers. return new ParseException((previousLine + 1) + ":" + (previousColumn + 1) + ": " - + description); + + replaceMalformedSurrogates(description)); } /** diff --git a/framework/src/test/java/org/tron/core/services/http/JsonFormatEscapeTest.java b/framework/src/test/java/org/tron/core/services/http/JsonFormatEscapeTest.java index a5c74cc434d..9a5414b0154 100644 --- a/framework/src/test/java/org/tron/core/services/http/JsonFormatEscapeTest.java +++ b/framework/src/test/java/org/tron/core/services/http/JsonFormatEscapeTest.java @@ -12,6 +12,7 @@ import com.fasterxml.jackson.databind.json.JsonMapper; import com.google.protobuf.Any; import com.google.protobuf.ByteString; +import java.nio.CharBuffer; import java.nio.charset.StandardCharsets; import org.junit.Test; import org.tron.common.utils.ByteArray; @@ -224,6 +225,28 @@ public void testSupplementaryPlaneIsEmittedAsSurrogateEscapes() { assertEquals("\\ud83d\\ude00", escapeName(emoji, URL_FIELD)); } + @Test + public void testIsolatedLowSurrogatesKeepLegacyReplacement() throws Exception { + for (char low = Character.MIN_LOW_SURROGATE; low <= Character.MAX_LOW_SURROGATE; low++) { + String input = String.valueOf(low); + String escaped = JsonFormat.escapeText(input); + + assertEquals("?", escaped); + StandardCharsets.UTF_8.newEncoder().encode(CharBuffer.wrap(escaped)); + } + } + + @Test + public void testLowSurrogateReplacementPreservesFollowingCharacters() { + String low = String.valueOf((char) 0xDE00); + String emoji = new String(Character.toChars(0x1F600)); + + assertEquals("prefix?suffix", JsonFormat.escapeText("prefix" + low + "suffix")); + assertEquals("??", JsonFormat.escapeText(low + low)); + assertEquals("?\\n\\\"\\\\", JsonFormat.escapeText(low + "\n\"\\")); + assertEquals("?\\ud83d\\ude00", JsonFormat.escapeText(low + emoji)); + } + // Field integrity through HTTP normalization /** diff --git a/framework/src/test/java/org/tron/core/services/http/JsonFormatIdentifierTest.java b/framework/src/test/java/org/tron/core/services/http/JsonFormatIdentifierTest.java new file mode 100644 index 00000000000..f77d3730b9c --- /dev/null +++ b/framework/src/test/java/org/tron/core/services/http/JsonFormatIdentifierTest.java @@ -0,0 +1,122 @@ +package org.tron.core.services.http; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.server.LocalConnector; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.junit.Test; +import org.tron.json.JSONObject; +import org.tron.protos.Protocol.Account; + +public class JsonFormatIdentifierTest { + + private static final String EMOJI = new String(Character.toChars(0x1F600)); + + @Test + public void testSupplementaryCharacterKeepsLegacyReplacement() { + assertIdentifierError(EMOJI, "?"); + assertIdentifierError("prefix" + EMOJI, "?"); + } + + @Test + public void testUnpairedSurrogatesKeepLegacyReplacement() { + String highSurrogate = String.valueOf((char) 0xD83D); + String lowSurrogate = String.valueOf((char) 0xDE00); + + assertIdentifierError(highSurrogate, "?"); + assertIdentifierError(highSurrogate + "a", "?"); + assertIdentifierError(highSurrogate + highSurrogate, "?"); + assertIdentifierError(lowSurrogate, "?"); + assertIdentifierError(lowSurrogate + highSurrogate, "?"); + } + + @Test + public void testOtherInvalidCharactersKeepTheirErrorMessages() { + String bmpCharacter = String.valueOf((char) 0x4E2D); + + assertIdentifierError("@", "@"); + assertIdentifierError("bad-name", "-"); + assertIdentifierError(bmpCharacter, bmpCharacter); + } + + @Test + public void testValidIdentifierStillParses() throws Exception { + Account.Builder account = Account.newBuilder(); + + JsonFormat.merge("{\"balance\":7}", account, false); + + assertEquals(7L, account.getBalance()); + } + + @Test + public void testIdentifierErrorIsValidUtf8WithNativeJettyWriter() throws Exception { + Server server = new Server(); + LocalConnector connector = new LocalConnector(server); + server.addConnector(connector); + ServletContextHandler context = new ServletContextHandler(); + context.setContextPath("/"); + context.addServlet(new ServletHolder(new IdentifierServlet()), "/parse"); + server.setHandler(context); + + try { + server.start(); + byte[] body = ("{\"" + EMOJI + "\":1}").getBytes(StandardCharsets.UTF_8); + byte[] headers = ("POST /parse HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Connection: close\r\n" + + "Content-Type: application/json; charset=utf-8\r\n" + + "Content-Length: " + body.length + "\r\n\r\n") + .getBytes(StandardCharsets.US_ASCII); + ByteBuffer request = ByteBuffer.allocate(headers.length + body.length); + request.put(headers).put(body).flip(); + + ByteBuffer response = connector.getResponse(request); + byte[] wire = new byte[response.remaining()]; + response.get(wire); + String raw = new String(wire, StandardCharsets.ISO_8859_1); + assertTrue(raw.startsWith("HTTP/1.1 200 ")); + int headerEnd = raw.indexOf("\r\n\r\n"); + assertTrue("the response must contain complete headers", headerEnd >= 0); + byte[] payload = Arrays.copyOfRange(wire, headerEnd + 4, wire.length); + + // String(byte[], UTF_8) replaces malformed bytes and would hide this regression. + String json = StandardCharsets.UTF_8.newDecoder().decode(ByteBuffer.wrap(payload)).toString(); + String error = JSONObject.parseObject(json).getString("Error"); + assertEquals("1:2: Expected identifier. -?", error); + } finally { + server.stop(); + } + } + + private static void assertIdentifierError(String identifier, String expectedCharacter) { + JsonFormat.ParseException error = assertThrows(JsonFormat.ParseException.class, + () -> JsonFormat.merge("{\"" + identifier + "\":1}", Account.newBuilder(), false)); + + assertEquals("1:2: Expected identifier. -" + expectedCharacter, error.getMessage()); + } + + private static class IdentifierServlet extends HttpServlet { + + @Override + protected void doPost(HttpServletRequest request, HttpServletResponse response) + throws IOException { + response.setContentType("application/json; charset=utf-8"); + try { + JsonFormat.merge(request.getReader(), Account.newBuilder(), false); + } catch (JsonFormat.ParseException e) { + Util.processError(e, response); + } + } + } +} diff --git a/framework/src/test/java/org/tron/core/services/http/JsonFormatUnicodeErrorTest.java b/framework/src/test/java/org/tron/core/services/http/JsonFormatUnicodeErrorTest.java new file mode 100644 index 00000000000..e5d9d5146c8 --- /dev/null +++ b/framework/src/test/java/org/tron/core/services/http/JsonFormatUnicodeErrorTest.java @@ -0,0 +1,186 @@ +package org.tron.core.services.http; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +import java.nio.ByteBuffer; +import java.nio.CharBuffer; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import org.eclipse.jetty.server.LocalConnector; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.junit.Test; +import org.springframework.test.util.ReflectionTestUtils; +import org.tron.common.parameter.CommonParameter; +import org.tron.core.config.args.Args; +import org.tron.core.services.ratelimiter.RateLimiterContainer; +import org.tron.json.JSONObject; +import org.tron.protos.Protocol.Account; + +public class JsonFormatUnicodeErrorTest { + + private static final String EMOJI = new String(Character.toChars(0x1F600)); + + @Test + public void testInvalidSupplementaryEscapeKeepsLegacyReplacement() { + assertEscapeError("\\" + EMOJI, "?"); + assertEscapeError("prefix\\" + EMOJI, "?"); + } + + @Test + public void testInvalidUnpairedSurrogateEscapesKeepLegacyReplacement() { + String highSurrogate = String.valueOf((char) 0xD83D); + String lowSurrogate = String.valueOf((char) 0xDE00); + + assertEscapeError("\\" + highSurrogate, "?"); + assertEscapeError("\\" + highSurrogate + "a", "?"); + assertEscapeError("\\" + highSurrogate + highSurrogate, "?"); + assertEscapeError("\\" + lowSurrogate, "?"); + assertEscapeError("\\" + lowSurrogate + highSurrogate, "?"); + } + + @Test + public void testOtherInvalidEscapesKeepTheirErrorMessages() { + String bmpCharacter = String.valueOf((char) 0x4E2D); + + assertEscapeError("\\q", "q"); + assertEscapeError("\\@", "@"); + assertEscapeError("\\" + bmpCharacter, bmpCharacter); + } + + @Test + public void testValidUnicodeAndEscapesStillDecode() throws Exception { + String bmpCharacter = String.valueOf((char) 0x4E2D); + + assertEquals(bmpCharacter + EMOJI, JsonFormat.unescapeText(bmpCharacter + EMOJI)); + assertEquals("\b\f\n\r\t\\/\"'" + bmpCharacter + EMOJI, + JsonFormat.unescapeText("\\b\\f\\n\\r\\t\\\\\\/\\\"\\'\\u4e2d\\uD83D\\uDE00")); + } + + @Test + public void testDeployContractEscapeErrorIsValidUtf8WithNativeJettyWriter() throws Exception { + JSONObject input = new JSONObject(); + input.put("owner_address", ""); + // The outer JSON is valid; only the embedded ABI contains an invalid escape. + input.put("abi", "[{\"name\":\"\\" + EMOJI + "\"}]"); + + String error = requestError(new DeployContractServlet(), "/wallet/deploycontract", input); + + assertEquals("1:20: Invalid escape sequence: '\\?'", error); + } + + @Test + public void testLongIntegerErrorIsValidUtf8WithNativeJettyWriter() throws Exception { + JSONObject input = new JSONObject(); + // BigInteger's digit groups can split a surrogate pair inside its exception message. + input.put("balance", "11111" + EMOJI + "1111111"); + + String error = requestError(new GetAccountServlet(), "/wallet/getaccount", input); + + assertTrue(error.contains("1:12: Couldn't parse integer:")); + } + + @Test + public void testParserErrorsReplaceOnlyUnpairedSurrogates() throws Exception { + String high = String.valueOf((char) 0xD800); + String low = String.valueOf((char) 0xDC00); + String description = high + "x" + EMOJI + low + high; + String expected = "?x" + EMOJI + "??"; + JsonFormat.Tokenizer tokenizer = new JsonFormat.Tokenizer("first second"); + tokenizer.nextToken(); + + String current = tokenizer.parseException(description).getMessage(); + String previous = tokenizer.parseExceptionPreviousToken(description).getMessage(); + + assertEquals("1:7: " + expected, current); + assertEquals("1:1: " + expected, previous); + StandardCharsets.UTF_8.newEncoder().encode(CharBuffer.wrap(current)); + StandardCharsets.UTF_8.newEncoder().encode(CharBuffer.wrap(previous)); + } + + @Test + public void testParserErrorsPreserveValidDescriptions() { + String description = "invalid " + (char) 0x4E2D + EMOJI; + JsonFormat.Tokenizer tokenizer = new JsonFormat.Tokenizer("field"); + + assertEquals("1:1: " + description, tokenizer.parseException(description).getMessage()); + assertEquals("1:1: " + description, + tokenizer.parseExceptionPreviousToken(description).getMessage()); + assertEquals("1:1: null", tokenizer.parseException(null).getMessage()); + assertEquals("1:1: null", tokenizer.parseExceptionPreviousToken(null).getMessage()); + } + + @Test + public void testOrdinaryIntegerErrorsKeepTheirMessages() { + JsonFormat.ParseException invalid = assertThrows(JsonFormat.ParseException.class, + () -> JsonFormat.merge("{\"balance\":\"bad\"}", Account.newBuilder(), false)); + JsonFormat.ParseException overflow = assertThrows(JsonFormat.ParseException.class, + () -> JsonFormat.merge("{\"balance\":9223372036854775808}", + Account.newBuilder(), false)); + + assertEquals("1:12: Couldn't parse integer: For input string: \"\"bad\"\"", + invalid.getMessage()); + assertEquals("1:12: Couldn't parse integer: Number out of range for 64-bit signed integer: " + + "9223372036854775808", overflow.getMessage()); + } + + private static String requestError(RateLimiterServlet servlet, String path, JSONObject input) + throws Exception { + CommonParameter args = Args.getInstance(); + long originalMaxSize = args.getHttpMaxMessageSize(); + boolean originalNonBlocking = args.isRateLimiterApiNonBlocking(); + Server server = new Server(); + LocalConnector connector = new LocalConnector(server); + server.addConnector(connector); + ServletContextHandler context = new ServletContextHandler(); + context.setContextPath("/"); + ReflectionTestUtils.setField(servlet, "container", new RateLimiterContainer()); + context.addServlet(new ServletHolder(servlet), path); + server.setHandler(context); + + try { + args.setHttpMaxMessageSize(1_000_000L); + args.setRateLimiterApiNonBlocking(false); + server.start(); + byte[] body = input.toJSONString().getBytes(StandardCharsets.UTF_8); + byte[] headers = ("POST " + path + " HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Connection: close\r\n" + + "Content-Type: application/json; charset=utf-8\r\n" + + "Content-Length: " + body.length + "\r\n\r\n") + .getBytes(StandardCharsets.US_ASCII); + ByteBuffer request = ByteBuffer.allocate(headers.length + body.length); + request.put(headers).put(body).flip(); + + ByteBuffer response = connector.getResponse(request); + byte[] wire = new byte[response.remaining()]; + response.get(wire); + String raw = new String(wire, StandardCharsets.ISO_8859_1); + assertTrue(raw.startsWith("HTTP/1.1 200 ")); + int headerEnd = raw.indexOf("\r\n\r\n"); + assertTrue("the response must contain complete headers", headerEnd >= 0); + byte[] payload = Arrays.copyOfRange(wire, headerEnd + 4, wire.length); + + // A replacement decoder would hide invalid bytes emitted by the response writer. + String json = StandardCharsets.UTF_8.newDecoder().decode(ByteBuffer.wrap(payload)).toString(); + return JSONObject.parseObject(json).getString("Error"); + } finally { + try { + server.stop(); + } finally { + args.setHttpMaxMessageSize(originalMaxSize); + args.setRateLimiterApiNonBlocking(originalNonBlocking); + } + } + } + + private static void assertEscapeError(String input, String expectedCharacter) { + JsonFormat.InvalidEscapeSequence error = assertThrows(JsonFormat.InvalidEscapeSequence.class, + () -> JsonFormat.unescapeText(input)); + + assertEquals("Invalid escape sequence: '\\" + expectedCharacter + "'", error.getMessage()); + } +}