From 482a49fce184a138debeefaa67d0e02e1d141a92 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 13 Feb 2025 11:28:22 -0800 Subject: [PATCH] Improve size limiting string message (#122427) SizeLimitingStringWriter is used to limit the output size of mustache scripts. When the size is limited, the resulting exception lacks detail needed to identify the problem. In particular, the actual size that would result is not given. Additionally, the excerpt lacks any of the new string being added. This commit tweaks the exception to include both of these details. --- docs/changelog/122427.yaml | 5 ++ .../mustache/MustacheScriptEngineTests.java | 2 +- .../common/text/SizeLimitingStringWriter.java | 46 +++++++++++++------ .../text/SizeLimitingStringWriterTests.java | 9 ++++ 4 files changed, 48 insertions(+), 14 deletions(-) create mode 100644 docs/changelog/122427.yaml diff --git a/docs/changelog/122427.yaml b/docs/changelog/122427.yaml new file mode 100644 index 000000000000..2444a0ec894a --- /dev/null +++ b/docs/changelog/122427.yaml @@ -0,0 +1,5 @@ +pr: 122427 +summary: Improve size limiting string message +area: Infra/Core +type: enhancement +issues: [] diff --git a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTests.java b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTests.java index bc1cd30ad45b..c327ba49e6d1 100644 --- a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTests.java +++ b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTests.java @@ -423,7 +423,7 @@ public class MustacheScriptEngineTests extends ESTestCase { ex.getCause().getCause(), allOf( instanceOf(SizeLimitingStringWriter.SizeLimitExceededException.class), - transformedMatch(Throwable::getMessage, endsWith("has exceeded the size limit [1024]")) + transformedMatch(Throwable::getMessage, endsWith("has size [1030] which exceeds the size limit [1024]")) ) ); } diff --git a/server/src/main/java/org/elasticsearch/common/text/SizeLimitingStringWriter.java b/server/src/main/java/org/elasticsearch/common/text/SizeLimitingStringWriter.java index 2df7e6537c60..3aa7c67a14c6 100644 --- a/server/src/main/java/org/elasticsearch/common/text/SizeLimitingStringWriter.java +++ b/server/src/main/java/org/elasticsearch/common/text/SizeLimitingStringWriter.java @@ -30,18 +30,29 @@ public class SizeLimitingStringWriter extends StringWriter { this.sizeLimit = sizeLimit; } - private void checkSizeLimit(int additionalChars) { - int bufLen = getBuffer().length(); - if (bufLen + additionalChars > sizeLimit) { - throw new SizeLimitExceededException( - Strings.format("String [%s...] has exceeded the size limit [%s]", getBuffer().substring(0, Math.min(bufLen, 20)), sizeLimit) - ); + private int limitSize(int additionalChars) { + int neededSize = getBuffer().length() + additionalChars; + if (neededSize > sizeLimit) { + return additionalChars - (neededSize - sizeLimit); } + return additionalChars; + } + + private void throwSizeLimitExceeded(int limitedChars, int requestedChars) { + assert limitedChars < requestedChars; + int bufLen = getBuffer().length(); + int foundSize = bufLen - limitedChars + requestedChars; // reconstitute original + String selection = getBuffer().substring(0, Math.min(bufLen, 20)); + throw new SizeLimitExceededException( + Strings.format("String [%s...] has size [%d] which exceeds the size limit [%d]", selection, foundSize, sizeLimit) + ); } @Override public void write(int c) { - checkSizeLimit(1); + if (limitSize(1) != 1) { + throwSizeLimitExceeded(0, 1); + } super.write(c); } @@ -49,20 +60,29 @@ public class SizeLimitingStringWriter extends StringWriter { @Override public void write(char[] cbuf, int off, int len) { - checkSizeLimit(len); - super.write(cbuf, off, len); + int limitedLen = limitSize(len); + if (limitedLen > 0) { + super.write(cbuf, off, limitedLen); + } + if (limitedLen != len) { + throwSizeLimitExceeded(limitedLen, len); + } } @Override public void write(String str) { - checkSizeLimit(str.length()); - super.write(str); + this.write(str, 0, str.length()); } @Override public void write(String str, int off, int len) { - checkSizeLimit(len); - super.write(str, off, len); + int limitedLen = limitSize(len); + if (limitedLen > 0) { + super.write(str, off, limitedLen); + } + if (limitedLen != len) { + throwSizeLimitExceeded(limitedLen, len); + } } // append(...) delegates to write(...) methods diff --git a/server/src/test/java/org/elasticsearch/common/text/SizeLimitingStringWriterTests.java b/server/src/test/java/org/elasticsearch/common/text/SizeLimitingStringWriterTests.java index 32a8de20df9a..0874a106e59e 100644 --- a/server/src/test/java/org/elasticsearch/common/text/SizeLimitingStringWriterTests.java +++ b/server/src/test/java/org/elasticsearch/common/text/SizeLimitingStringWriterTests.java @@ -11,6 +11,8 @@ package org.elasticsearch.common.text; import org.elasticsearch.test.ESTestCase; +import static org.hamcrest.Matchers.equalTo; + public class SizeLimitingStringWriterTests extends ESTestCase { public void testSizeIsLimited() { SizeLimitingStringWriter writer = new SizeLimitingStringWriter(10); @@ -26,4 +28,11 @@ public class SizeLimitingStringWriterTests extends ESTestCase { expectThrows(SizeLimitingStringWriter.SizeLimitExceededException.class, () -> writer.append("a")); expectThrows(SizeLimitingStringWriter.SizeLimitExceededException.class, () -> writer.append("a", 0, 1)); } + + public void testLimitMessage() { + SizeLimitingStringWriter writer = new SizeLimitingStringWriter(3); + + var e = expectThrows(SizeLimitingStringWriter.SizeLimitExceededException.class, () -> writer.write("abcdefgh")); + assertThat(e.getMessage(), equalTo("String [abc...] has size [8] which exceeds the size limit [3]")); + } }