Skip to content

Rejects setting numeric fields to numbers of another type in CelRuntime. #1467

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
copybara-service wants to merge 1 commit into
base: main
Choose a base branch
from
+863 −242
Open

Rejects setting numeric fields to numbers of another type in CelRuntime. #1467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions common/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,11 @@ java_library(
exports = ["//common/src/main/java/dev/cel/common:proto_json_adapter"],
)

cel_android_library(
name = "proto_json_adapter_android",
exports = ["//common/src/main/java/dev/cel/common:proto_json_adapter_android"],
)

java_library(
name = "source",
visibility = ["//:internal"],
Expand Down
5 changes: 5 additions & 0 deletions common/internal/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ java_library(
exports = ["//common/src/main/java/dev/cel/common/internal:proto_lite_adapter"],
)

cel_android_library(
name = "proto_lite_adapter_android",
exports = ["//common/src/main/java/dev/cel/common/internal:proto_lite_adapter_android"],
)

java_library(
name = "proto_equality",
exports = ["//common/src/main/java/dev/cel/common/internal:proto_equality"],
Expand Down
16 changes: 16 additions & 0 deletions common/src/main/java/dev/cel/common/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,22 @@ java_library(
],
)

cel_android_library(
name = "proto_json_adapter_android",
srcs = ["CelProtoJsonAdapter.java"],
tags = [
],
deps = [
"//common/internal:date_time_helpers_android",
"//common/internal:proto_time_utils_android",
"//common/values:cel_byte_string",
"//common/values:values_android",
"@maven//:com_google_errorprone_error_prone_annotations",
"@maven_android//:com_google_guava_guava",
"@maven_android//:com_google_protobuf_protobuf_javalite",
],
)

java_library(
name = "source_location",
srcs = ["CelSourceLocation.java"],
Expand Down
21 changes: 20 additions & 1 deletion common/src/main/java/dev/cel/common/internal/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,6 @@ java_library(
":well_known_proto",
"//common:options",
"//common/annotations",
"//common/exceptions:numeric_overflow",
"//common/values",
"//common/values:cel_byte_string",
"@maven//:com_google_code_findbugs_annotations",
Expand Down Expand Up @@ -214,6 +213,26 @@ java_library(
],
)

cel_android_library(
name = "proto_lite_adapter_android",
srcs = ["ProtoLiteAdapter.java"],
tags = [
],
deps = [
":well_known_proto_android",
"//common:options",
"//common:proto_json_adapter_android",
"//common/annotations",
"//common/exceptions:numeric_overflow",
"//common/internal:proto_time_utils_android",
"//common/values:cel_byte_string",
"//common/values:values_android",
"@maven//:com_google_errorprone_error_prone_annotations",
"@maven_android//:com_google_guava_guava",
"@maven_android//:com_google_protobuf_protobuf_javalite",
],
)

java_library(
name = "proto_equality",
srcs = ["ProtoEquality.java"],
Expand Down
12 changes: 11 additions & 1 deletion common/src/main/java/dev/cel/common/internal/DefaultLiteDescriptorPool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@
@Immutable
@Internal
public final class DefaultLiteDescriptorPool implements CelLiteDescriptorPool {
private static final String STRUCT_FIELDS_ENTRY = "google.protobuf.Struct.FieldsEntry";

private final ImmutableMap<String, MessageLiteDescriptor> protoFqnToMessageInfo;
private final ImmutableMap<Class<?>, MessageLiteDescriptor> classToMessageInfo;

Expand Down Expand Up @@ -163,7 +165,7 @@ private static MessageLiteDescriptor newMessageInfo(WellKnownProto wellKnownProt
/* encodingType= */ EncodingType.MAP,
/* protoFieldType= */ FieldLiteDescriptor.Type.MESSAGE,
/* isPacked= */ false,
/* fieldProtoTypeName= */ "google.protobuf.Struct.FieldsEntry"));
/* fieldProtoTypeName= */ STRUCT_FIELDS_ENTRY));
break;
case JSON_VALUE:
messageBuilder = Value::newBuilder;
Expand Down Expand Up @@ -268,6 +270,14 @@ private DefaultLiteDescriptorPool(ImmutableSet<CelLiteDescriptor> descriptors) {
protoFqnMapBuilder.put(wellKnownProto.typeName(), wktMessageInfo);
classMapBuilder.put(wellKnownProto.messageClass(), wktMessageInfo);
}
// Struct's map entry, which `google.protobuf.Struct{fields: ...}` literals are encoded with.
protoFqnMapBuilder.put(
STRUCT_FIELDS_ENTRY,
CelLiteDescriptor.mapEntry(
/* fullName= */ STRUCT_FIELDS_ENTRY,
/* keyType= */ FieldLiteDescriptor.Type.STRING,
/* valueType= */ FieldLiteDescriptor.Type.MESSAGE,
/* valueTypeName= */ WellKnownProto.JSON_VALUE.typeName()));

for (CelLiteDescriptor descriptor : descriptors) {
protoFqnMapBuilder.putAll(descriptor.getProtoTypeNamesToDescriptors());
Expand Down
82 changes: 43 additions & 39 deletions common/src/main/java/dev/cel/common/internal/ProtoAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@
package dev.cel.common.internal;

import static com.google.common.base.Preconditions.checkNotNull;
import static dev.cel.common.internal.ProtoLiteAdapter.intCheckedCast;
import static dev.cel.common.internal.ProtoLiteAdapter.unsignedIntCheckedCast;

import com.google.common.primitives.Ints;
import com.google.common.primitives.UnsignedInts;
import com.google.common.primitives.UnsignedLong;
import com.google.errorprone.annotations.CheckReturnValue;
Expand All @@ -33,7 +34,6 @@
import com.google.protobuf.MessageOrBuilder;
import dev.cel.common.CelOptions;
import dev.cel.common.annotations.Internal;
import dev.cel.common.exceptions.CelNumericOverflowException;
import dev.cel.common.values.CelByteString;
import dev.cel.common.values.NullValue;
import java.util.ArrayList;
Expand Down Expand Up @@ -308,23 +308,21 @@ private BidiConverter fieldToValueConverter(FieldDescriptor fieldDescriptor) {
case SFIXED32:
case SINT32:
case INT32:
return unwrapAndConvert(INT_CONVERTER);
return unwrapAndConvert(fieldDescriptor, Long.class, INT_CONVERTER);
case FIXED32:
case UINT32:
if (celOptions.enableUnsignedLongs()) {
return unwrapAndConvert(UNSIGNED_UINT32_CONVERTER);
return unwrapAndConvert(fieldDescriptor, UnsignedLong.class, UNSIGNED_UINT32_CONVERTER);
}
return unwrapAndConvert(SIGNED_UINT32_CONVERTER);
return unwrapAndConvert(fieldDescriptor, Long.class, SIGNED_UINT32_CONVERTER);
case FIXED64:
case UINT64:
if (celOptions.enableUnsignedLongs()) {
return unwrapAndConvert(UNSIGNED_UINT64_CONVERTER);
return unwrapAndConvert(fieldDescriptor, UnsignedLong.class, UNSIGNED_UINT64_CONVERTER);
}
return BidiConverter.of(
BidiConverter.IDENTITY.forwardConverter(),
value -> BidiConverter.IDENTITY.backwardConverter().convert(maybeUnwrap(value)));
return unwrapAndCheck(fieldDescriptor, Long.class);
case FLOAT:
return unwrapAndConvert(DOUBLE_CONVERTER);
return unwrapAndConvert(fieldDescriptor, Double.class, DOUBLE_CONVERTER);
case BYTES:
if (celOptions.evaluateCanonicalTypesToNativeValues()) {
return BidiConverter.<Object, Object>of(
Expand All @@ -336,25 +334,24 @@ private BidiConverter fieldToValueConverter(FieldDescriptor fieldDescriptor) {
BidiConverter.IDENTITY.forwardConverter(),
value -> BidiConverter.IDENTITY.backwardConverter().convert(maybeUnwrap(value)));
case DOUBLE:
return unwrapAndCheck(fieldDescriptor, Double.class);
case SFIXED64:
case SINT64:
case INT64:
return unwrapAndCheck(fieldDescriptor, Long.class);
case STRING:
case BOOL:
return BidiConverter.of(
BidiConverter.IDENTITY.forwardConverter(),
value -> BidiConverter.IDENTITY.backwardConverter().convert(maybeUnwrap(value)));
case ENUM:
return BidiConverter.<Object, Long>of(
return BidiConverter.<Object, Object>of(
value -> (long) ((EnumValueDescriptor) value).getNumber(),
number -> {
if (number > Integer.MAX_VALUE || number < Integer.MIN_VALUE) {
throw new IllegalArgumentException("Enum value out of int32 range: " + number);
}
return fieldDescriptor
.getEnumType()
.findValueByNumberCreatingIfUnknown(number.intValue());
});
value ->
fieldDescriptor
.getEnumType()
.findValueByNumberCreatingIfUnknown(
intCheckedCast((Long) unwrapNumber(fieldDescriptor, Long.class, value))));
case MESSAGE:
return BidiConverter.<MessageOrBuilder, Object>of(
this::adaptProtoToValue,
Expand Down Expand Up @@ -401,9 +398,9 @@ public Message adaptValueToProto(Object value, String protoTypeName) {
switch (wellKnownProto) {
case ANY_VALUE:
if (value instanceof Message) {
protoTypeName = ((Message) value).getDescriptorForType().getFullName();
return Any.pack((Message) value);
}
return protoLiteAdapter.adaptValueToAny(value, protoTypeName);
return protoLiteAdapter.adaptValueToAny(value);
default:
return (Message) protoLiteAdapter.adaptValueToWellKnownProto(value, wellKnownProto);
}
Expand All @@ -430,22 +427,6 @@ private static String typeName(Descriptor protoType) {
return protoType.getFullName();
}

private static int intCheckedCast(long value) {
try {
return Ints.checkedCast(value);
} catch (IllegalArgumentException e) {
throw new CelNumericOverflowException(e);
}
}

private static int unsignedIntCheckedCast(long value) {
try {
return UnsignedInts.checkedCast(value);
} catch (IllegalArgumentException e) {
throw new CelNumericOverflowException(e);
}
}

private Object maybeUnwrap(Object value) {
if (value instanceof Message) {
return adaptProtoToValue((MessageOrBuilder) value);
Expand All @@ -454,9 +435,32 @@ private Object maybeUnwrap(Object value) {
}

private BidiConverter<Number, Object> unwrapAndConvert(
final BidiConverter<Number, Number> original) {
FieldDescriptor fieldDescriptor, Class<?> celType, BidiConverter<Number, Number> original) {
return BidiConverter.of(
original.forwardConverter()::convert,
value -> original.backwardConverter().convert((Number) maybeUnwrap(value)));
value ->
original
.backwardConverter()
.convert((Number) unwrapNumber(fieldDescriptor, celType, value)));
}

private BidiConverter<Object, Object> unwrapAndCheck(
FieldDescriptor fieldDescriptor, Class<?> celType) {
return BidiConverter.of(
BidiConverter.IDENTITY.forwardConverter(),
value -> unwrapNumber(fieldDescriptor, celType, value));
}

// Per the spec, numbers aren't converted between int, uint, and double when setting a field.
private Object unwrapNumber(FieldDescriptor fieldDescriptor, Class<?> celType, Object value) {
Object celValue = maybeUnwrap(value);
if ((celValue instanceof Long || celValue instanceof UnsignedLong || celValue instanceof Double)
&& !celType.isInstance(celValue)) {
throw new IllegalArgumentException(
String.format(
"unexpected value type for field '%s' in message '%s'",
fieldDescriptor.getName(), fieldDescriptor.getContainingType().getFullName()));
}
return celValue;
}
}
47 changes: 17 additions & 30 deletions common/src/main/java/dev/cel/common/internal/ProtoLiteAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import com.google.protobuf.Int32Value;
import com.google.protobuf.Int64Value;
import com.google.protobuf.ListValue;
import com.google.protobuf.Message;
import com.google.protobuf.MessageLite;
import com.google.protobuf.MessageLiteOrBuilder;
import com.google.protobuf.NullValue;
Expand Down Expand Up @@ -110,11 +109,7 @@ public MessageLite adaptValueToWellKnownProto(Object value, WellKnownProto wellK
}
}

public Any adaptValueToAny(Object value, String typeName) {
if (value instanceof MessageLite) {
return packAnyMessage((MessageLite) value, typeName);
}

public Any adaptValueToAny(Object value) {
if (value instanceof dev.cel.common.values.NullValue) {
return packAnyMessage(
Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build(), WellKnownProto.JSON_VALUE);
Expand Down Expand Up @@ -234,7 +229,7 @@ private ImmutableMap<String, Object> adaptJsonStructToValue(Struct struct) {
.collect(toImmutableMap(Entry::getKey, e -> adaptJsonToValue(e.getValue())));
}

private Message adaptValueToDouble(Object value) {
private MessageLite adaptValueToDouble(Object value) {
if (value instanceof Double) {
return DoubleValue.of((Double) value);
}
Expand All @@ -244,7 +239,7 @@ private Message adaptValueToDouble(Object value) {
throw new IllegalArgumentException("Unexpected value type: " + value);
}

private Message adaptValueToFloat(Object value) {
private MessageLite adaptValueToFloat(Object value) {
if (value instanceof Double) {
return FloatValue.of(((Double) value).floatValue());
}
Expand All @@ -254,7 +249,7 @@ private Message adaptValueToFloat(Object value) {
throw new IllegalArgumentException("Unexpected value type: " + value);
}

private Message adaptValueToInt32(Object value) {
private MessageLite adaptValueToInt32(Object value) {
if (value instanceof Integer) {
return Int32Value.of((Integer) value);
}
Expand All @@ -264,7 +259,7 @@ private Message adaptValueToInt32(Object value) {
throw new IllegalArgumentException("Unexpected value type: " + value);
}

private Message adaptValueToInt64(Object value) {
private MessageLite adaptValueToInt64(Object value) {
if (value instanceof Integer) {
return Int64Value.of(((Integer) value).longValue());
}
Expand All @@ -275,30 +270,22 @@ private Message adaptValueToInt64(Object value) {
throw new IllegalArgumentException("Unexpected value type: " + value);
}

private Message adaptValueToUint32(Object value) {
private MessageLite adaptValueToUint32(Object value) {
if (value instanceof Integer) {
return UInt32Value.of((Integer) value);
}
// Per the spec, an int isn't a uint, unless unsigned longs are disabled.
if (!celOptions.enableUnsignedLongs() && value instanceof Long) {
try {
return UInt32Value.of(unsignedIntCheckedCast((Long) value));
} catch (IllegalArgumentException e) {
throw new CelNumericOverflowException(e);
}
return UInt32Value.of(unsignedIntCheckedCast((Long) value));
}
if (value instanceof UnsignedLong) {
try {
return UInt32Value.of(unsignedIntCheckedCast(((UnsignedLong) value).longValue()));
} catch (IllegalArgumentException e) {
throw new CelNumericOverflowException(e);
}
return UInt32Value.of(unsignedIntCheckedCast(((UnsignedLong) value).longValue()));
}

throw new IllegalArgumentException("Unexpected value type: " + value);
}

private Message adaptValueToUint64(Object value) {
private MessageLite adaptValueToUint64(Object value) {
if (value instanceof Integer) {
return UInt64Value.of(UnsignedInts.toLong((Integer) value));
}
Expand All @@ -312,15 +299,17 @@ private Message adaptValueToUint64(Object value) {
throw new IllegalArgumentException("Unexpected value type: " + value);
}

private static int intCheckedCast(long value) {
/** Narrows {@code value} to an int32, or throws {@link CelNumericOverflowException}. */
public static int intCheckedCast(long value) {
try {
return Ints.checkedCast(value);
} catch (IllegalArgumentException e) {
throw new CelNumericOverflowException(e);
}
}

private static int unsignedIntCheckedCast(long value) {
/** Narrows {@code value} to a uint32, or throws {@link CelNumericOverflowException}. */
public static int unsignedIntCheckedCast(long value) {
try {
return UnsignedInts.checkedCast(value);
} catch (IllegalArgumentException e) {
Expand All @@ -329,14 +318,12 @@ private static int unsignedIntCheckedCast(long value) {
}

private static Any packAnyMessage(MessageLite msg, WellKnownProto wellKnownProto) {
return packAnyMessage(msg, wellKnownProto.typeName());
return packAny(msg.toByteString(), wellKnownProto.typeName());
}

private static Any packAnyMessage(MessageLite msg, String typeUrl) {
return Any.newBuilder()
.setValue(msg.toByteString())
.setTypeUrl("type.googleapis.com/" + typeUrl)
.build();
/** Packs {@code value}, a serialized message of type {@code typeName}, into an {@link Any}. */
public static Any packAny(ByteString value, String typeName) {
return Any.newBuilder().setValue(value).setTypeUrl("type.googleapis.com/" + typeName).build();
}

private Timestamp adaptValueToProtoTimestamp(Object value) {
Expand Down
Loading
Loading