Skip to content

Commit

Permalink
Fix integer overflow bug in validation error message (#75)
Browse files Browse the repository at this point in the history
## 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
bufbuild/protovalidate#144
  • Loading branch information
lyang authored Dec 19, 2023
1 parent b35a45f commit 581067b
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -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");
}
}

0 comments on commit 581067b

Please sign in to comment.