From d3dfdc9d7bacd748e80b781f96608593ad3c4b5d Mon Sep 17 00:00:00 2001 From: Steve Ayers Date: Wed, 28 May 2025 19:01:22 -0400 Subject: [PATCH 1/4] Supplemental tests --- build.gradle.kts | 1 + .../java/build/buf/protovalidate/Format.java | 6 +++- .../build/buf/protovalidate/FormatTest.java | 30 +++++++++++-------- .../string_ext_supplemental.textproto | 26 ++++++++++++++++ .../testdata/string_ext_v0.24.0.textproto | 14 +++++++++ 5 files changed, 64 insertions(+), 13 deletions(-) create mode 100644 src/test/resources/testdata/string_ext_supplemental.textproto diff --git a/build.gradle.kts b/build.gradle.kts index 894fd20e..9bc2f8e2 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -272,6 +272,7 @@ allprojects { showExceptions = true showCauses = true showStackTraces = true + showStandardStreams = true } } } diff --git a/src/main/java/build/buf/protovalidate/Format.java b/src/main/java/build/buf/protovalidate/Format.java index cfb42eb6..73050a78 100644 --- a/src/main/java/build/buf/protovalidate/Format.java +++ b/src/main/java/build/buf/protovalidate/Format.java @@ -20,6 +20,8 @@ import com.google.protobuf.Timestamp; import java.math.BigInteger; import java.nio.charset.StandardCharsets; +import java.nio.ByteBuffer; +import java.nio.CharBuffer; import java.text.DecimalFormat; import java.time.Instant; import java.util.Locale; @@ -144,7 +146,9 @@ private static String formatString(Val val) { } return val.value().toString(); case Bytes: - return new String((byte[]) val.value(), StandardCharsets.UTF_8); + String byteStr = new String((byte[]) val.value(), StandardCharsets.UTF_8); + // Collapse any contiguous placeholders into one + return byteStr.replaceAll("\\ufffd+", "\ufffd"); case Double: Optional result = validateNumber(val); if (result.isPresent()) { diff --git a/src/test/java/build/buf/protovalidate/FormatTest.java b/src/test/java/build/buf/protovalidate/FormatTest.java index f63c91d4..51b2b9b1 100644 --- a/src/test/java/build/buf/protovalidate/FormatTest.java +++ b/src/test/java/build/buf/protovalidate/FormatTest.java @@ -35,6 +35,7 @@ import java.util.List; import java.util.Map; import java.util.stream.Stream; +import java.util.stream.Collectors; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Named; import org.junit.jupiter.params.ParameterizedTest; @@ -78,35 +79,40 @@ class FormatTest { // found no matching overload for 'format' applied to 'string.(list(map(int, dyn)))' "object inside map"); - @BeforeAll - static void setUp() throws Exception { + private static List loadTestData(String fileName) throws Exception { byte[] encoded = Files.readAllBytes( - Paths.get("src/test/resources/testdata/string_ext_" + CEL_SPEC_VERSION + ".textproto")); + Paths.get(fileName)); String data = new String(encoded, StandardCharsets.UTF_8); SimpleTestFile.Builder bldr = SimpleTestFile.newBuilder(); TextFormat.getParser().merge(data, bldr); SimpleTestFile testData = bldr.build(); - List sections = testData.getSectionList(); + return testData.getSectionList(); + } + + @BeforeAll + private static void setUp() throws Exception { + + List celSpecSections = loadTestData("src/test/resources/testdata/string_ext_" + CEL_SPEC_VERSION + ".textproto"); + List supplementalSections = loadTestData("src/test/resources/testdata/string_ext_supplemental.textproto"); + + List sections = Stream.concat(celSpecSections.stream(), supplementalSections.stream()) + .collect(Collectors.toList()); // Find the format tests which test successful formatting - // Defaults to an empty list if nothing is found formatTests = sections.stream() .filter(s -> s.getName().equals("format")) - .findFirst() - .map(SimpleTestSection::getTestList) - .orElse(Collections.emptyList()); + .flatMap(s -> s.getTestList().stream()) + .collect(Collectors.toList()); // Find the format error tests which test errors during formatting - // Defaults to an empty list if nothing is found formatErrorTests = sections.stream() .filter(s -> s.getName().equals("format_errors")) - .findFirst() - .map(SimpleTestSection::getTestList) - .orElse(Collections.emptyList()); + .flatMap(s -> s.getTestList().stream()) + .collect(Collectors.toList()); env = Env.newEnv(Library.Lib(new ValidateLibrary())); } diff --git a/src/test/resources/testdata/string_ext_supplemental.textproto b/src/test/resources/testdata/string_ext_supplemental.textproto new file mode 100644 index 00000000..6f4402fc --- /dev/null +++ b/src/test/resources/testdata/string_ext_supplemental.textproto @@ -0,0 +1,26 @@ +# proto-file: ../../../proto/cel/expr/conformance/test/simple.proto +# proto-message: cel.expr.conformance.test.SimpleTestFile + +# Ideally these tests should be in the cel-spec conformance test suite. +# Until they are added, we can use this to test for additional functionality +# listed in the spec. + +name: "string_ext_supplemental" +description: "Supplemental tests for the strings extension library." +section: { + name: "format" + test: { + name: "bytes support for string with invalid utf-8 encoding" + expr: '"%s".format([b"\xF0abc\x8C\xF0xyz"])' + value: { + string_value: '\ufffdabc\ufffdxyz', + } + } + test: { + name: "bytes support for string with only invalid utf-8 sequences" + expr: '"%s".format([b"\xF0\x8C\xF0"])' + value: { + string_value: '\ufffd', + } + } +} diff --git a/src/test/resources/testdata/string_ext_v0.24.0.textproto b/src/test/resources/testdata/string_ext_v0.24.0.textproto index d2455583..1170f323 100644 --- a/src/test/resources/testdata/string_ext_v0.24.0.textproto +++ b/src/test/resources/testdata/string_ext_v0.24.0.textproto @@ -632,6 +632,20 @@ section: { string_value: 'xyz', } } + test: { + name: "bytes support for string with invalid utf-8 encoding" + expr: '"%s".format([b"\xF0abc\x8C\xF0xyz"])' + value: { + string_value: '\ufffdabc\ufffdxyz', + } + } + test: { + name: "bytes support for string with only invalid utf-8 sequences" + expr: '"%s".format([b"\xF0\x8C\xF0"])' + value: { + string_value: '\ufffd', + } + } test: { name: "type() support for string" expr: '"%s".format([type("test string")])' From edfa868da4e451566f84176ee7dab1872b9cb946 Mon Sep 17 00:00:00 2001 From: Steve Ayers Date: Thu, 29 May 2025 10:56:38 -0400 Subject: [PATCH 2/4] Tests --- .../build/buf/protovalidate/FormatTest.java | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/test/java/build/buf/protovalidate/FormatTest.java b/src/test/java/build/buf/protovalidate/FormatTest.java index 51b2b9b1..898fd0fc 100644 --- a/src/test/java/build/buf/protovalidate/FormatTest.java +++ b/src/test/java/build/buf/protovalidate/FormatTest.java @@ -79,24 +79,14 @@ class FormatTest { // found no matching overload for 'format' applied to 'string.(list(map(int, dyn)))' "object inside map"); - private static List loadTestData(String fileName) throws Exception { - byte[] encoded = - Files.readAllBytes( - Paths.get(fileName)); - String data = new String(encoded, StandardCharsets.UTF_8); - SimpleTestFile.Builder bldr = SimpleTestFile.newBuilder(); - TextFormat.getParser().merge(data, bldr); - SimpleTestFile testData = bldr.build(); - - return testData.getSectionList(); - } - @BeforeAll private static void setUp() throws Exception { - + // The test data from the cel-spec conformance tests List celSpecSections = loadTestData("src/test/resources/testdata/string_ext_" + CEL_SPEC_VERSION + ".textproto"); + // Our supplemental tests of functionality not in the cel conformance file, but defined in the spec. List supplementalSections = loadTestData("src/test/resources/testdata/string_ext_supplemental.textproto"); + // Combine the test data from both files into one List sections = Stream.concat(celSpecSections.stream(), supplementalSections.stream()) .collect(Collectors.toList()); @@ -133,6 +123,19 @@ void testFormatError(SimpleTest test) { assertThat(result.getVal().type().typeEnum()).isEqualTo(TypeEnum.Err); } + // Loads test data from the given text format file + private static List loadTestData(String fileName) throws Exception { + byte[] encoded = + Files.readAllBytes( + Paths.get(fileName)); + String data = new String(encoded, StandardCharsets.UTF_8); + SimpleTestFile.Builder bldr = SimpleTestFile.newBuilder(); + TextFormat.getParser().merge(data, bldr); + SimpleTestFile testData = bldr.build(); + + return testData.getSectionList(); + } + // Runs a test by extending the cel environment with the specified // types, variables and declarations, then evaluating it with the cel runtime. private static Program.EvalResult evaluate(SimpleTest test) { From f13f774c8bfe6be657447a9991662a000f180e27 Mon Sep 17 00:00:00 2001 From: Steve Ayers Date: Thu, 29 May 2025 10:57:05 -0400 Subject: [PATCH 3/4] Tests --- .../java/build/buf/protovalidate/Format.java | 2 -- .../build/buf/protovalidate/FormatTest.java | 23 ++++++++++--------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/main/java/build/buf/protovalidate/Format.java b/src/main/java/build/buf/protovalidate/Format.java index 73050a78..38a7008a 100644 --- a/src/main/java/build/buf/protovalidate/Format.java +++ b/src/main/java/build/buf/protovalidate/Format.java @@ -20,8 +20,6 @@ import com.google.protobuf.Timestamp; import java.math.BigInteger; import java.nio.charset.StandardCharsets; -import java.nio.ByteBuffer; -import java.nio.CharBuffer; import java.text.DecimalFormat; import java.time.Instant; import java.util.Locale; diff --git a/src/test/java/build/buf/protovalidate/FormatTest.java b/src/test/java/build/buf/protovalidate/FormatTest.java index 898fd0fc..88f51b39 100644 --- a/src/test/java/build/buf/protovalidate/FormatTest.java +++ b/src/test/java/build/buf/protovalidate/FormatTest.java @@ -30,12 +30,11 @@ import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.stream.Stream; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Named; import org.junit.jupiter.params.ParameterizedTest; @@ -81,14 +80,18 @@ class FormatTest { @BeforeAll private static void setUp() throws Exception { - // The test data from the cel-spec conformance tests - List celSpecSections = loadTestData("src/test/resources/testdata/string_ext_" + CEL_SPEC_VERSION + ".textproto"); - // Our supplemental tests of functionality not in the cel conformance file, but defined in the spec. - List supplementalSections = loadTestData("src/test/resources/testdata/string_ext_supplemental.textproto"); + // The test data from the cel-spec conformance tests + List celSpecSections = + loadTestData("src/test/resources/testdata/string_ext_" + CEL_SPEC_VERSION + ".textproto"); + // Our supplemental tests of functionality not in the cel conformance file, but defined in the + // spec. + List supplementalSections = + loadTestData("src/test/resources/testdata/string_ext_supplemental.textproto"); // Combine the test data from both files into one - List sections = Stream.concat(celSpecSections.stream(), supplementalSections.stream()) - .collect(Collectors.toList()); + List sections = + Stream.concat(celSpecSections.stream(), supplementalSections.stream()) + .collect(Collectors.toList()); // Find the format tests which test successful formatting formatTests = @@ -125,9 +128,7 @@ void testFormatError(SimpleTest test) { // Loads test data from the given text format file private static List loadTestData(String fileName) throws Exception { - byte[] encoded = - Files.readAllBytes( - Paths.get(fileName)); + byte[] encoded = Files.readAllBytes(Paths.get(fileName)); String data = new String(encoded, StandardCharsets.UTF_8); SimpleTestFile.Builder bldr = SimpleTestFile.newBuilder(); TextFormat.getParser().merge(data, bldr); From d9dc9a791137dfc099477ce0bdce2f5d555cd212 Mon Sep 17 00:00:00 2001 From: Steve Ayers Date: Thu, 29 May 2025 10:57:53 -0400 Subject: [PATCH 4/4] Revert --- build.gradle.kts | 1 - .../testdata/string_ext_v0.24.0.textproto | 14 -------------- 2 files changed, 15 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 9bc2f8e2..894fd20e 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -272,7 +272,6 @@ allprojects { showExceptions = true showCauses = true showStackTraces = true - showStandardStreams = true } } } diff --git a/src/test/resources/testdata/string_ext_v0.24.0.textproto b/src/test/resources/testdata/string_ext_v0.24.0.textproto index 1170f323..d2455583 100644 --- a/src/test/resources/testdata/string_ext_v0.24.0.textproto +++ b/src/test/resources/testdata/string_ext_v0.24.0.textproto @@ -632,20 +632,6 @@ section: { string_value: 'xyz', } } - test: { - name: "bytes support for string with invalid utf-8 encoding" - expr: '"%s".format([b"\xF0abc\x8C\xF0xyz"])' - value: { - string_value: '\ufffdabc\ufffdxyz', - } - } - test: { - name: "bytes support for string with only invalid utf-8 sequences" - expr: '"%s".format([b"\xF0\x8C\xF0"])' - value: { - string_value: '\ufffd', - } - } test: { name: "type() support for string" expr: '"%s".format([type("test string")])'