Skip to content

Tighten OptimizedSelectTraversal to only work on struct-like interfaces #1250

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
Open

Tighten OptimizedSelectTraversal to only work on struct-like interfaces #1250

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
4 changes: 4 additions & 0 deletions common/src/main/java/dev/cel/common/values/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,8 @@ java_library(
],
deps = [
":base_proto_cel_value_converter",
":optimized_selectable",
":select_field",
":values",
"//:auto_value",
"//common/annotations",
Expand Down Expand Up @@ -351,6 +353,8 @@ cel_android_library(
],
deps = [
":base_proto_cel_value_converter_android",
":optimized_selectable_android",
":select_field_android",
":values_android",
"//:auto_value",
"//common/annotations",
Expand Down
27 changes: 14 additions & 13 deletions common/src/main/java/dev/cel/common/values/OptimizedSelectTraversal.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@
import java.util.Optional;

/**
* Walks a sequence of {@link SelectField} selections, dispatching each field over {@link
* OptimizedSelectable} or {@link SelectableValue}.
* Walks a sequence of {@link SelectField} selections over a struct target.
*
* <p>Each hop resolves by field number through {@link OptimizedSelectable} when the target
* implements it, and otherwise by field name through {@link StructValue}. Any other target,
* including maps and optional values, raises {@link CelAttributeNotFoundException}.
*
* <p>CEL Library Internals. Do Not Use.
*/
Expand All @@ -41,8 +44,6 @@ public static Object qualify(Object target, ImmutableList<SelectField> fields) {

/**
* Presence tests the terminal field of {@code fields}, navigating through all preceding fields.
*
* <p>Absence of any intermediate field short-circuits to {@code false}.
*/
public static boolean hasField(Object target, ImmutableList<SelectField> fields) {
if (fields.isEmpty()) {
Expand All @@ -64,7 +65,7 @@ public static boolean hasField(Object target, ImmutableList<SelectField> fields)
return hasTerminalField(current, fields.get(terminalIndex));
}

// SelectableValue is only ever instantiated with String keys in the select path.
// StructValue is only ever instantiated with String keys in the select path.
@SuppressWarnings("unchecked")
private static Object qualifyField(Object target, SelectField field) {
if (target instanceof ErrorValue) {
Expand All @@ -73,8 +74,8 @@ private static Object qualifyField(Object target, SelectField field) {
if (target instanceof OptimizedSelectable) {
return ((OptimizedSelectable) target).selectByFieldNumber(field);
}
if (target instanceof SelectableValue) {
SelectableValue<String> selectable = (SelectableValue<String>) target;
if (target instanceof StructValue) {
StructValue<String, ?> selectable = (StructValue<String, ?>) target;
if (field.defaultValue() != null) {
return selectable
.find(field.fieldName())
Expand All @@ -86,7 +87,7 @@ private static Object qualifyField(Object target, SelectField field) {
throw CelAttributeNotFoundException.forFieldResolution(field.fieldName());
}

// SelectableValue is only ever instantiated with String keys in the select path.
// StructValue is only ever instantiated with String keys in the select path.
@SuppressWarnings("unchecked")
private static Optional<Object> navigateField(Object target, SelectField field) {
if (target instanceof ErrorValue) {
Expand All @@ -95,13 +96,13 @@ private static Optional<Object> navigateField(Object target, SelectField field)
if (target instanceof OptimizedSelectable) {
return ((OptimizedSelectable) target).findByFieldNumber(field);
}
if (target instanceof SelectableValue) {
return ((SelectableValue<String>) target).find(field.fieldName()).map(Object.class::cast);
if (target instanceof StructValue) {
return ((StructValue<String, ?>) target).find(field.fieldName()).map(Object.class::cast);
}
throw CelAttributeNotFoundException.forFieldResolution(field.fieldName());
}

// SelectableValue is only ever instantiated with String keys in the select path.
// StructValue is only ever instantiated with String keys in the select path.
@SuppressWarnings("unchecked")
private static boolean hasTerminalField(Object target, SelectField field) {
if (target instanceof ErrorValue) {
Expand All @@ -110,8 +111,8 @@ private static boolean hasTerminalField(Object target, SelectField field) {
if (target instanceof OptimizedSelectable) {
return ((OptimizedSelectable) target).hasFieldByNumber(field);
}
if (target instanceof SelectableValue) {
return ((SelectableValue<String>) target).find(field.fieldName()).isPresent();
if (target instanceof StructValue) {
return ((StructValue<String, ?>) target).find(field.fieldName()).isPresent();
}
throw CelAttributeNotFoundException.forFieldResolution(field.fieldName());
}
Expand Down
74 changes: 53 additions & 21 deletions common/src/main/java/dev/cel/common/values/ProtoLiteCelValueConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import static com.google.common.base.Preconditions.checkNotNull;

import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Defaults;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
Expand All @@ -26,6 +25,7 @@
import com.google.common.collect.Multimaps;
import com.google.common.primitives.UnsignedLong;
import com.google.errorprone.annotations.Immutable;
import com.google.protobuf.ByteString;
import com.google.protobuf.CodedInputStream;
import com.google.protobuf.ExtensionRegistryLite;
import com.google.protobuf.MessageLite;
Expand Down Expand Up @@ -62,13 +62,20 @@
@Immutable
@Internal
public final class ProtoLiteCelValueConverter extends BaseProtoCelValueConverter {
static final String MAP_KEY_FIELD_NAME = "key";
static final String MAP_VALUE_FIELD_NAME = "value";

private final CelLiteDescriptorPool descriptorPool;

public static ProtoLiteCelValueConverter newInstance(
CelLiteDescriptorPool celLiteDescriptorPool) {
return new ProtoLiteCelValueConverter(celLiteDescriptorPool);
}

boolean hasDescriptor(String protoTypeName) {
return descriptorPool.findDescriptor(protoTypeName).isPresent();
}

private static Object readPrimitiveField(
CodedInputStream inputStream, FieldLiteDescriptor fieldDescriptor) throws IOException {
switch (fieldDescriptor.getProtoFieldType()) {
Expand Down Expand Up @@ -155,22 +162,45 @@ private MessageLite.Builder getDefaultMessageBuilder(String protoTypeName) {

Object getDefaultCelValue(String protoTypeName, String fieldName) {
MessageLiteDescriptor messageDescriptor = descriptorPool.getDescriptorOrThrow(protoTypeName);
FieldLiteDescriptor fieldDescriptor = messageDescriptor.getByFieldNameOrThrow(fieldName);

Object defaultValue = getDefaultValue(fieldDescriptor);
return getDefaultCelValue(messageDescriptor.getByFieldNameOrThrow(fieldName));
}

return toRuntimeValue(defaultValue);
Object getDefaultCelValue(FieldLiteDescriptor fieldDescriptor) {
return toRuntimeValue(getDefaultValue(fieldDescriptor));
}

public Optional<FieldLiteDescriptor> findFieldDescriptor(String protoTypeName, int fieldNumber) {
Optional<FieldLiteDescriptor> findFieldDescriptor(String protoTypeName, int fieldNumber) {
return descriptorPool
.findDescriptor(protoTypeName)
.flatMap(desc -> desc.findByFieldNumber(fieldNumber));
}

public Optional<Object> findDefaultCelValue(String protoTypeName, int fieldNumber) {
return findFieldDescriptor(protoTypeName, fieldNumber)
.map(fieldDescriptor -> toRuntimeValue(getDefaultValue(fieldDescriptor)));
Optional<Object> tryDecodeWellKnownProto(ByteString bytes, String protoTypeName) {
Optional<WellKnownProto> wellKnownProto = WellKnownProto.getByTypeName(protoTypeName);
if (!wellKnownProto.isPresent()) {
return Optional.empty();
}

return descriptorPool
.findDescriptor(protoTypeName)
.map(
descriptor ->
decodeWellKnownProto(bytes, protoTypeName, descriptor, wellKnownProto.get()));
}

private Object decodeWellKnownProto(
ByteString bytes,
String protoTypeName,
MessageLiteDescriptor descriptor,
WellKnownProto wellKnownProto) {
try {
MessageLite.Builder builder = descriptor.newMessageBuilder();
builder.mergeFrom(bytes, ExtensionRegistryLite.getEmptyRegistry());
return fromWellKnownProto(builder.build(), wellKnownProto);
} catch (IOException e) {
throw new IllegalArgumentException(
"Failed to decode well-known proto of type: " + protoTypeName, e);
}
}

@Override
Expand Down Expand Up @@ -276,16 +306,21 @@ private ImmutableList<Object> readPackedRepeatedFields(

private Map.Entry<Object, Object> readSingleMapEntry(
CodedInputStream inputStream, FieldLiteDescriptor fieldDescriptor) throws IOException {
String entryTypeName = fieldDescriptor.getFieldProtoTypeName();
ImmutableMap<String, Object> singleMapEntry =
readAllFields(inputStream.readByteArray(), fieldDescriptor.getFieldProtoTypeName())
.values();
Object key = checkNotNull(singleMapEntry.get("key"));
Object value = checkNotNull(singleMapEntry.get("value"));
readAllFields(inputStream.readByteArray(), entryTypeName).values();
Object key = singleMapEntry.get(MAP_KEY_FIELD_NAME);
if (key == null) {
key = getDefaultCelValue(entryTypeName, MAP_KEY_FIELD_NAME);
}
Object value = singleMapEntry.get(MAP_VALUE_FIELD_NAME);
if (value == null) {
value = getDefaultCelValue(entryTypeName, MAP_VALUE_FIELD_NAME);
}

return new AbstractMap.SimpleEntry<>(key, value);
}

@VisibleForTesting
MessageFields readAllFields(byte[] bytes, String protoTypeName) throws IOException {
MessageLiteDescriptor messageDescriptor = descriptorPool.getDescriptorOrThrow(protoTypeName);
CodedInputStream inputStream = CodedInputStream.newInstance(bytes);
Expand Down Expand Up @@ -360,19 +395,16 @@ MessageFields readAllFields(byte[] bytes, String protoTypeName) throws IOExcepti
if (fieldDescriptor.getEncodingType().equals(EncodingType.LIST)) {
String fieldName = fieldDescriptor.getFieldName();
List<Object> repeatedValues =
repeatedFieldValues.computeIfAbsent(
fieldNumber,
(unused) -> {
List<Object> newList = new ArrayList<>();
fieldValues.put(fieldName, newList);
return newList;
});
repeatedFieldValues.computeIfAbsent(fieldNumber, (unused) -> new ArrayList<>());

if (payload instanceof Collection) {
repeatedValues.addAll((Collection<?>) payload);
} else {
repeatedValues.add(payload);
}
if (!repeatedValues.isEmpty()) {
fieldValues.put(fieldName, repeatedValues);
}
} else {
fieldValues.put(fieldDescriptor.getFieldName(), payload);
}
Expand Down
Loading
Loading