From 581067bedce9f185d26cfae4df894eb7e407fa4b Mon Sep 17 00:00:00 2001 From: Lin Yang Date: Mon, 18 Dec 2023 18:14:56 -0800 Subject: [PATCH] Fix integer overflow bug in validation error message (#75) ## Symptom Some standard validation error messages have `Integer Overflow` bug on large decimal values. ## Example ```protobuf message LargeValue { uint64 count = 1 [(buf.validate.field).uint64 = {gte: 0, lte: 999999999999}]; } ``` ```java @Test public void validate_count() { LargeValue value = LargeValue.newBuilder().setCount(9999999999999).build(); ValidationResult result = new Validator().validate(value); assertThat(result.getViolations().get(0).getMessage()).isEqualTo("value must be greater than or equal to 0 and less than or equal to 999999999999"); // Above fails with: "value must be greater than or equal to 0 and less than or equal to -727379969" } ``` ## Cause This is due to `%s` interpreted as `String` in [`Format.java`](https://github.com/bufbuild/protovalidate-java/blob/b35a45f8eccb0aef7d5a0fdec20d0a4f159529f7/src/main/java/build/buf/protovalidate/internal/celext/Format.java#L101-L103): ```java // ... case 's': formatString(builder, arg); break; // ... ``` which eventually calls: ```java // ... } else if (type == TypeEnum.Int || type == TypeEnum.Uint) { formatInteger(builder, Long.valueOf(val.intValue()).intValue()); // ... ``` ## Note Originally fixed in a different way in https://github.com/bufbuild/protovalidate/pull/144 --- .../protovalidate/internal/celext/Format.java | 16 +-------- .../internal/celext/FormatTest.java | 33 +++++++++++++++++++ 2 files changed, 34 insertions(+), 15 deletions(-) create mode 100644 src/test/java/build/buf/protovalidate/internal/celext/FormatTest.java diff --git a/src/main/java/build/buf/protovalidate/internal/celext/Format.java b/src/main/java/build/buf/protovalidate/internal/celext/Format.java index 81a86aec..0aee9c13 100644 --- a/src/main/java/build/buf/protovalidate/internal/celext/Format.java +++ b/src/main/java/build/buf/protovalidate/internal/celext/Format.java @@ -157,7 +157,7 @@ private static void formatStringSafe(StringBuilder builder, Val val, boolean lis if (type == TypeEnum.Bool) { builder.append(val.booleanValue()); } else if (type == TypeEnum.Int || type == TypeEnum.Uint) { - formatInteger(builder, Long.valueOf(val.intValue()).intValue()); + formatDecimal(builder, val); } else if (type == TypeEnum.Double) { DecimalFormat format = new DecimalFormat("0.#"); builder.append(format.format(val.value())); @@ -247,20 +247,6 @@ private static void formatBytes(StringBuilder builder, Val val) { .append("\""); } - /** - * Formats an integer value. - * - * @param builder the StringBuilder to append the formatted integer value to. - * @param value the value to format. - */ - private static void formatInteger(StringBuilder builder, int value) { - if (value < 0) { - builder.append("-"); - value = -value; - } - builder.append(value); - } - /** * Formats a hexadecimal value. * diff --git a/src/test/java/build/buf/protovalidate/internal/celext/FormatTest.java b/src/test/java/build/buf/protovalidate/internal/celext/FormatTest.java new file mode 100644 index 00000000..63ca34a2 --- /dev/null +++ b/src/test/java/build/buf/protovalidate/internal/celext/FormatTest.java @@ -0,0 +1,33 @@ +// Copyright 2023 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package build.buf.protovalidate.internal.celext; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.*; + +import org.junit.jupiter.api.Test; +import org.projectnessie.cel.common.types.ListT; +import org.projectnessie.cel.common.types.UintT; +import org.projectnessie.cel.common.types.ref.Val; + +class FormatTest { + @Test + void largeDecimalValuesAreProperlyFormatted() { + UintT largeDecimal = UintT.uintOf(999999999999L); + ListT val = (ListT) ListT.newValArrayList(null, new Val[] {largeDecimal}); + String formatted = Format.format("%s", val); + assertThat(formatted).isEqualTo("999999999999"); + } +}