diff --git a/common/src/main/java/dev/cel/common/values/ProtoLiteCelValueConverter.java b/common/src/main/java/dev/cel/common/values/ProtoLiteCelValueConverter.java index a154d9603..74b9b4b32 100644 --- a/common/src/main/java/dev/cel/common/values/ProtoLiteCelValueConverter.java +++ b/common/src/main/java/dev/cel/common/values/ProtoLiteCelValueConverter.java @@ -161,11 +161,6 @@ private static boolean isStructLike(@Nullable WellKnownProto wellKnownProto) { return wellKnownProto == null || wellKnownProto == WellKnownProto.FIELD_MASK; } - Object getDefaultCelValue(String protoTypeName, String fieldName) { - MessageLiteDescriptor messageDescriptor = descriptorPool.getDescriptorOrThrow(protoTypeName); - return getDefaultCelValue(messageDescriptor.getByFieldNameOrThrow(fieldName)); - } - Object getDefaultCelValue(FieldLiteDescriptor fieldDescriptor) { return toRuntimeValue(getDefaultValue(fieldDescriptor)); } @@ -176,6 +171,12 @@ Optional findFieldDescriptor(String protoTypeName, int fiel .flatMap(desc -> desc.findByFieldNumber(fieldNumber)); } + Optional findFieldDescriptor(String protoTypeName, String fieldName) { + return descriptorPool + .findDescriptor(protoTypeName) + .flatMap(desc -> desc.findByFieldName(fieldName)); + } + MessageLite parseMessageLite(ByteString bytes, String protoTypeName) { MessageLiteDescriptor descriptor = descriptorPool.getDescriptorOrThrow(protoTypeName); return parseMessageLite(bytes, descriptor); @@ -304,6 +305,11 @@ private Map.Entry readSingleMapEntry( return new AbstractMap.SimpleImmutableEntry<>(key, value); } + /** + * Decodes only {@code fieldDescriptor}'s occurrences in {@code bytes} into its CEL value, + * skipping every other field. Returns null if the field is absent (or a repeated field has only + * empty packed records on the wire). + */ @Nullable Object readSingleField(ByteString bytes, FieldLiteDescriptor fieldDescriptor) throws IOException { CodedInputStream inputStream = bytes.newCodedInput(); @@ -318,8 +324,6 @@ private Map.Entry readSingleMapEntry( int tagWireType = WireFormat.getTagWireType(tag); fieldValue = readFieldValue(tagWireType, inputStream, fieldDescriptor, fieldValue); } - // Only this field is decoded, so unlike readAllFields, a failed conversion can't affect access - // to any other field. return fieldValue == null ? null : resolveFieldValue(finalizeFieldValue(fieldValue)); } @@ -472,49 +476,8 @@ private static FieldLiteDescriptor newFieldDescriptor( protoTypeName); } - /** - * Decodes every known field in {@code bytes}, keyed by field name. Each value must be passed to - * {@link #resolveFieldValue} to obtain its CEL value. - */ - ImmutableMap readAllFields(ByteString bytes, String protoTypeName) - throws IOException { - MessageLiteDescriptor messageDescriptor = descriptorPool.getDescriptorOrThrow(protoTypeName); - if (bytes.isEmpty()) { - return ImmutableMap.of(); - } - return readAllFields(bytes.newCodedInput(), messageDescriptor); - } - - private ImmutableMap readAllFields( - CodedInputStream inputStream, MessageLiteDescriptor messageDescriptor) throws IOException { - Map fieldValues = new LinkedHashMap<>(); - for (int tag = inputStream.readTag(); tag != 0; tag = inputStream.readTag()) { - int tagWireType = WireFormat.getTagWireType(tag); - int fieldNumber = WireFormat.getTagFieldNumber(tag); - FieldLiteDescriptor fieldDescriptor = - messageDescriptor.findByFieldNumber(fieldNumber).orElse(null); - if (fieldDescriptor == null) { - skipWireField(tag, inputStream); - continue; - } - - String fieldName = fieldDescriptor.getFieldName(); - Object fieldValue = - readFieldValue(tagWireType, inputStream, fieldDescriptor, fieldValues.get(fieldName)); - if (fieldValue != null) { - fieldValues.put(fieldName, fieldValue); - } - } - - fieldValues.replaceAll((fieldName, fieldValue) -> finalizeFieldValue(fieldValue)); - return ImmutableMap.copyOf(fieldValues); - } - - /** - * Returns the CEL value of a field decoded by {@link #readAllFields}, completing any conversion - * that was deferred. - */ - Object resolveFieldValue(Object fieldValue) { + /** Returns the CEL value of a scanned field, completing any conversion that was deferred. */ + private Object resolveFieldValue(Object fieldValue) { if (fieldValue instanceof DeferredConversion) { return toRuntimeValue(((DeferredConversion) fieldValue).value); } diff --git a/common/src/main/java/dev/cel/common/values/ProtoMessageLiteValue.java b/common/src/main/java/dev/cel/common/values/ProtoMessageLiteValue.java index fb2af1cb0..b9cec7271 100644 --- a/common/src/main/java/dev/cel/common/values/ProtoMessageLiteValue.java +++ b/common/src/main/java/dev/cel/common/values/ProtoMessageLiteValue.java @@ -18,10 +18,10 @@ import com.google.auto.value.AutoValue; import com.google.auto.value.extension.memoized.Memoized; -import com.google.common.collect.ImmutableMap; import com.google.errorprone.annotations.Immutable; import com.google.protobuf.ByteString; import com.google.protobuf.MessageLite; +import dev.cel.common.exceptions.CelAttributeNotFoundException; import dev.cel.common.types.CelType; import dev.cel.common.types.StructTypeReference; import dev.cel.protobuf.CelLiteDescriptor.FieldLiteDescriptor; @@ -93,16 +93,6 @@ ByteString toByteString() { return bytes != null ? bytes : serializedRawValue(); } - @Memoized - ImmutableMap fieldValues() { - try { - return protoLiteCelValueConverter().readAllFields(toByteString(), celType().name()); - } catch (IOException e) { - throw new IllegalArgumentException( - "Failed to decode proto message of type: " + celType().name(), e); - } - } - @Override public boolean isZeroValue() { ByteString bytes = wireBytes(); @@ -131,28 +121,39 @@ public final int hashCode() { @Override public Object select(String field) { - return find(field) - .orElseGet(() -> protoLiteCelValueConverter().getDefaultCelValue(celType().name(), field)); + Optional fd = findFieldDescriptor(field); + if (!fd.isPresent()) { + throw CelAttributeNotFoundException.of( + String.format("field '%s' is not declared in message '%s'", field, celType().name())); + } + Object fieldValue = readField(fd.get()); + return fieldValue != null + ? fieldValue + : protoLiteCelValueConverter().getDefaultCelValue(fd.get()); } @Override public Optional find(String field) { - return Optional.ofNullable(fieldValues().get(field)) - .map(protoLiteCelValueConverter()::resolveFieldValue); + Optional fd = findFieldDescriptor(field); + if (!fd.isPresent()) { + // Per SelectableValue#find, a field that doesn't exist is reported as absent. + return Optional.empty(); + } + return Optional.ofNullable(readField(fd.get())); } @Override public Object selectByFieldNumber(SelectField field) { - FieldLiteDescriptor fd = findFieldDescriptor(field); - if (fd != null) { - Object known = readField(fd); + Optional fd = findFieldDescriptor(field); + if (fd.isPresent()) { + Object known = readField(fd.get()); if (known != null) { return known; } if (field.defaultValue() != null) { return field.defaultValue(); } - return protoLiteCelValueConverter().getDefaultCelValue(fd); + return protoLiteCelValueConverter().getDefaultCelValue(fd.get()); } try { return protoLiteCelValueConverter().selectByFieldNumber(toByteString(), field); @@ -164,9 +165,9 @@ public Object selectByFieldNumber(SelectField field) { @Override public boolean hasFieldByNumber(SelectField field) { - FieldLiteDescriptor fd = findFieldDescriptor(field); - if (fd != null) { - return hasField(fd); + Optional fd = findFieldDescriptor(field); + if (fd.isPresent()) { + return hasField(fd.get()); } try { return protoLiteCelValueConverter().hasFieldByNumber(toByteString(), field); @@ -178,9 +179,9 @@ public boolean hasFieldByNumber(SelectField field) { @Override public Optional findByFieldNumber(SelectField field) { - FieldLiteDescriptor fd = findFieldDescriptor(field); - if (fd != null) { - return Optional.ofNullable(readField(fd)); + Optional fd = findFieldDescriptor(field); + if (fd.isPresent()) { + return Optional.ofNullable(readField(fd.get())); } try { return protoLiteCelValueConverter().findByFieldNumber(toByteString(), field); @@ -208,10 +209,12 @@ private boolean hasField(FieldLiteDescriptor fd) { } } - private @Nullable FieldLiteDescriptor findFieldDescriptor(SelectField field) { - return protoLiteCelValueConverter() - .findFieldDescriptor(celType().name(), field.fieldNumber()) - .orElse(null); + private Optional findFieldDescriptor(SelectField field) { + return protoLiteCelValueConverter().findFieldDescriptor(celType().name(), field.fieldNumber()); + } + + private Optional findFieldDescriptor(String fieldName) { + return protoLiteCelValueConverter().findFieldDescriptor(celType().name(), fieldName); } static ProtoMessageLiteValue create( diff --git a/common/src/test/java/dev/cel/common/values/ProtoLiteCelValueConverterTest.java b/common/src/test/java/dev/cel/common/values/ProtoLiteCelValueConverterTest.java index 69c4eba81..d416b3aa8 100644 --- a/common/src/test/java/dev/cel/common/values/ProtoLiteCelValueConverterTest.java +++ b/common/src/test/java/dev/cel/common/values/ProtoLiteCelValueConverterTest.java @@ -175,7 +175,7 @@ private enum RepeatedFieldBytesTestCase { // repeated_int64 is declared unpacked in proto2 and packed in proto3. @Test - public void readAllFields_repeatedFields_packedBytesCombinations( + public void readSingleField_repeatedFields_packedBytesCombinations( @TestParameter RepeatedFieldBytesTestCase testCase, @TestParameter({ "cel.expr.conformance.proto2.TestAllTypes", @@ -183,11 +183,12 @@ public void readAllFields_repeatedFields_packedBytesCombinations( }) String messageName) throws Exception { - ImmutableMap fields = - PROTO_LITE_CEL_VALUE_CONVERTER.readAllFields( - ByteString.copyFrom(testCase.bytes), messageName); + FieldLiteDescriptor fd = fieldDescriptor(messageName, TestAllTypes.REPEATED_INT64_FIELD_NUMBER); - assertThat(fields).containsExactly("repeated_int64", ImmutableList.of(1L, 2L, 3L)); + Object result = + PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField(ByteString.copyFrom(testCase.bytes), fd); + + assertThat(result).isEqualTo(ImmutableList.of(1L, 2L, 3L)); } /** @@ -236,16 +237,18 @@ private enum UnknownFieldsTestCase { } @Test - public void readAllFields_unknownFields(@TestParameter UnknownFieldsTestCase testCase) + public void readSingleField_unknownFields_skipped(@TestParameter UnknownFieldsTestCase testCase) throws Exception { TestAllTypes parsedMsg = TestAllTypes.parseFrom(testCase.bytes, ExtensionRegistryLite.getEmptyRegistry()); + FieldLiteDescriptor fd = + fieldDescriptor( + "cel.expr.conformance.proto3.TestAllTypes", TestAllTypes.SINGLE_INT64_FIELD_NUMBER); - ImmutableMap messageFields = - PROTO_LITE_CEL_VALUE_CONVERTER.readAllFields( - ByteString.copyFrom(testCase.bytes), "cel.expr.conformance.proto3.TestAllTypes"); + Object result = + PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField(ByteString.copyFrom(testCase.bytes), fd); - assertThat(messageFields).isEmpty(); + assertThat(result).isNull(); assertThat(TextFormat.printer().printToString(parsedMsg).trim()) .isEqualTo(testCase.formattedOutput); } @@ -270,8 +273,7 @@ public void readAllFields_unknownFields(@TestParameter UnknownFieldsTestCase tes * } */ @Test - @SuppressWarnings("unchecked") - public void readAllFields_unknownFieldsWithValues() throws Exception { + public void readSingleField_unknownFieldsPrecedeTarget_decodesTarget() throws Exception { // Unknown fields precede the known map entries so that decoding must continue past them. byte[] unknownMessageBytes = { -96, -100, 1, 1, -83, -100, 1, 2, 0, 0, 0, -79, -100, 1, 3, 0, 0, 0, 0, 0, 0, 0, -70, -100, 1, @@ -281,10 +283,13 @@ public void readAllFields_unknownFieldsWithValues() throws Exception { }; TestAllTypes parsedMsg = TestAllTypes.parseFrom(unknownMessageBytes, ExtensionRegistryLite.getEmptyRegistry()); + FieldLiteDescriptor fd = + fieldDescriptor( + "cel.expr.conformance.proto3.TestAllTypes", TestAllTypes.MAP_BOOL_DOUBLE_FIELD_NUMBER); - ImmutableMap fields = - PROTO_LITE_CEL_VALUE_CONVERTER.readAllFields( - ByteString.copyFrom(unknownMessageBytes), "cel.expr.conformance.proto3.TestAllTypes"); + Object result = + PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField( + ByteString.copyFrom(unknownMessageBytes), fd); assertThat(TextFormat.printer().printToString(parsedMsg)) .isEqualTo( @@ -309,9 +314,7 @@ public void readAllFields_unknownFieldsWithValues() throws Exception { + " 1: \"bar\"\n" + " 2: 5\n" + "}\n"); - assertThat(fields).containsKey("map_bool_double"); - Map mapBoolDoubleValues = (Map) fields.get("map_bool_double"); - assertThat(mapBoolDoubleValues).containsExactly(true, 1.5d, false, 2.5d).inOrder(); + assertThat((Map) result).containsExactly(true, 1.5d, false, 2.5d).inOrder(); } @Test @@ -345,7 +348,7 @@ public void getDefaultCelValue_nestedMessageWithoutDescriptor_returnsRawProtoMes } @Test - public void readAllFields_nestedMessageWithoutDescriptor_returnsRawProtoMessageLiteValue() + public void readSingleField_nestedMessageWithoutDescriptor_returnsRawProtoMessageLiteValue() throws Exception { // DESCRIPTOR_POOL only registers TestAllTypesCelDescriptor, not NestedTestAllTypesCelDescriptor TestAllTypes msg = @@ -354,22 +357,21 @@ public void readAllFields_nestedMessageWithoutDescriptor_returnsRawProtoMessageL NestedTestAllTypes.newBuilder() .setPayload(TestAllTypes.newBuilder().setSingleInt64(42L))) .build(); + FieldLiteDescriptor fd = + fieldDescriptor( + "cel.expr.conformance.proto3.TestAllTypes", TestAllTypes.ONEOF_TYPE_FIELD_NUMBER); - ImmutableMap fields = - PROTO_LITE_CEL_VALUE_CONVERTER.readAllFields( - msg.toByteString(), "cel.expr.conformance.proto3.TestAllTypes"); + Object result = PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField(msg.toByteString(), fd); - assertThat(fields.keySet()).containsExactly("oneof_type"); - Object fieldValue = fields.get("oneof_type"); - assertThat(fieldValue).isInstanceOf(RawProtoMessageLiteValue.class); - RawProtoMessageLiteValue rawValue = (RawProtoMessageLiteValue) fieldValue; + assertThat(result).isInstanceOf(RawProtoMessageLiteValue.class); + RawProtoMessageLiteValue rawValue = (RawProtoMessageLiteValue) result; assertThat(rawValue.toByteString()).isEqualTo(msg.getOneofType().toByteString()); assertThat(rawValue.protoTypeName()) .isEqualTo("cel.expr.conformance.proto3.NestedTestAllTypes"); } @Test - public void readAllFields_splitSingularSubmessages_mergesAllOccurrences() throws Exception { + public void readSingleField_splitSingularSubmessages_mergesAllOccurrences() throws Exception { ByteArrayOutputStream unknownFieldBaos = new ByteArrayOutputStream(); CodedOutputStream cos = CodedOutputStream.newInstance(unknownFieldBaos); cos.writeInt64(999, 42L); @@ -382,7 +384,6 @@ public void readAllFields_splitSingularSubmessages_mergesAllOccurrences() throws .setOneofType( NestedTestAllTypes.newBuilder() .setPayload(TestAllTypes.newBuilder().setSingleInt32(10))) - .setSingleDuration(Duration.newBuilder().setSeconds(10)) .setSingleNestedMessage(NestedMessage.newBuilder().setBb(99)) .build(); TestAllTypes part2 = @@ -390,23 +391,28 @@ public void readAllFields_splitSingularSubmessages_mergesAllOccurrences() throws .setOneofType( NestedTestAllTypes.newBuilder() .setPayload(TestAllTypes.newBuilder().setSingleString("merged"))) - .setSingleDuration(Duration.newBuilder().setNanos(500)) .setSingleNestedMessage(nestedWithUnknown) .build(); ByteString splitWireBytes = part1.toByteString().concat(part2.toByteString()); + FieldLiteDescriptor nestedMessageFd = + fieldDescriptor( + "cel.expr.conformance.proto3.TestAllTypes", + TestAllTypes.SINGLE_NESTED_MESSAGE_FIELD_NUMBER); + FieldLiteDescriptor oneofTypeFd = + fieldDescriptor( + "cel.expr.conformance.proto3.TestAllTypes", TestAllTypes.ONEOF_TYPE_FIELD_NUMBER); + + ProtoMessageLiteValue nestedMsg = + (ProtoMessageLiteValue) + PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField(splitWireBytes, nestedMessageFd); + RawProtoMessageLiteValue rawSubmessage = + (RawProtoMessageLiteValue) + PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField(splitWireBytes, oneofTypeFd); - ImmutableMap fields = - PROTO_LITE_CEL_VALUE_CONVERTER.readAllFields( - splitWireBytes, "cel.expr.conformance.proto3.TestAllTypes"); - - assertThat(PROTO_LITE_CEL_VALUE_CONVERTER.resolveFieldValue(fields.get("single_duration"))) - .isEqualTo(java.time.Duration.ofSeconds(10, 500)); - ProtoMessageLiteValue nestedMsg = (ProtoMessageLiteValue) fields.get("single_nested_message"); assertThat(nestedMsg.rawValue()).isNull(); assertThat(nestedMsg.select("bb")).isEqualTo(99L); assertThat(nestedMsg.value()) .isEqualTo(NestedMessage.newBuilder().setBb(99).mergeFrom(nestedWithUnknown).build()); - RawProtoMessageLiteValue rawSubmessage = (RawProtoMessageLiteValue) fields.get("oneof_type"); assertThat( NestedTestAllTypes.parseFrom( rawSubmessage.toByteString(), ExtensionRegistryLite.getEmptyRegistry())) @@ -425,22 +431,11 @@ public void parseMessageLite_emptyBytes_returnsDefaultInstanceSingleton() { assertThat(parsed).isSameInstanceAs(TestAllTypes.getDefaultInstance()); } - @Test - public void readAllFields_emptyBytes_returnsEmptyMap() throws Exception { - ImmutableMap fields = - PROTO_LITE_CEL_VALUE_CONVERTER.readAllFields( - ByteString.EMPTY, "cel.expr.conformance.proto3.TestAllTypes"); - - assertThat(fields).isEmpty(); - } - @Test public void readSingleField_emptyBytes_returnsNull() throws Exception { FieldLiteDescriptor fd = - PROTO_LITE_CEL_VALUE_CONVERTER - .findFieldDescriptor( - "cel.expr.conformance.proto3.TestAllTypes", TestAllTypes.SINGLE_INT64_FIELD_NUMBER) - .get(); + fieldDescriptor( + "cel.expr.conformance.proto3.TestAllTypes", TestAllTypes.SINGLE_INT64_FIELD_NUMBER); Object result = PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField(ByteString.EMPTY, fd); @@ -451,10 +446,8 @@ public void readSingleField_emptyBytes_returnsNull() throws Exception { public void readSingleField_absentField_returnsNull() throws Exception { TestAllTypes proto = TestAllTypes.newBuilder().setSingleInt64(42L).build(); FieldLiteDescriptor fd = - PROTO_LITE_CEL_VALUE_CONVERTER - .findFieldDescriptor( - "cel.expr.conformance.proto3.TestAllTypes", TestAllTypes.SINGLE_BOOL_FIELD_NUMBER) - .get(); + fieldDescriptor( + "cel.expr.conformance.proto3.TestAllTypes", TestAllTypes.SINGLE_BOOL_FIELD_NUMBER); Object result = PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField(proto.toByteString(), fd); @@ -469,11 +462,8 @@ public void readSingleField_emptyPackedRepeated_returnsNull() throws Exception { emptyPackedCos.flush(); ByteString emptyPackedBytes = ByteString.copyFrom(emptyPackedOut.toByteArray()); FieldLiteDescriptor repeatedInt32Fd = - PROTO_LITE_CEL_VALUE_CONVERTER - .findFieldDescriptor( - "cel.expr.conformance.proto3.TestAllTypes", - TestAllTypes.REPEATED_INT32_FIELD_NUMBER) - .get(); + fieldDescriptor( + "cel.expr.conformance.proto3.TestAllTypes", TestAllTypes.REPEATED_INT32_FIELD_NUMBER); Object result = PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField(emptyPackedBytes, repeatedInt32Fd); @@ -538,9 +528,7 @@ public void readSingleField_skipsOtherFieldsAndDecodesTarget( .concat(part2.toByteString()) .concat(ByteString.copyFrom(extraWireOut.toByteArray())); FieldLiteDescriptor fd = - PROTO_LITE_CEL_VALUE_CONVERTER - .findFieldDescriptor("cel.expr.conformance.proto3.TestAllTypes", testCase.fieldNumber) - .get(); + fieldDescriptor("cel.expr.conformance.proto3.TestAllTypes", testCase.fieldNumber); Object result = PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField(bytes, fd); @@ -550,10 +538,8 @@ public void readSingleField_skipsOtherFieldsAndDecodesTarget( @Test public void hasSingleField_emptyBytes_returnsFalse() throws Exception { FieldLiteDescriptor singleInt64Fd = - PROTO_LITE_CEL_VALUE_CONVERTER - .findFieldDescriptor( - "cel.expr.conformance.proto3.TestAllTypes", TestAllTypes.SINGLE_INT64_FIELD_NUMBER) - .get(); + fieldDescriptor( + "cel.expr.conformance.proto3.TestAllTypes", TestAllTypes.SINGLE_INT64_FIELD_NUMBER); boolean result = PROTO_LITE_CEL_VALUE_CONVERTER.hasSingleField(ByteString.EMPTY, singleInt64Fd); @@ -570,9 +556,7 @@ public void hasSingleField_emptyPackedRepeatedField_returnsFalse( String messageName) throws Exception { FieldLiteDescriptor repeatedInt32Fd = - PROTO_LITE_CEL_VALUE_CONVERTER - .findFieldDescriptor(messageName, TestAllTypes.REPEATED_INT32_FIELD_NUMBER) - .get(); + fieldDescriptor(messageName, TestAllTypes.REPEATED_INT32_FIELD_NUMBER); ByteArrayOutputStream emptyPackedOut = new ByteArrayOutputStream(); CodedOutputStream emptyPackedCos = CodedOutputStream.newInstance(emptyPackedOut); emptyPackedCos.writeInt64(TestAllTypes.SINGLE_INT64_FIELD_NUMBER, 42L); @@ -589,11 +573,8 @@ public void hasSingleField_emptyPackedRepeatedField_returnsFalse( @Test public void hasSingleField_emptyPackedFollowedByPopulatedPacked_returnsTrue() throws Exception { FieldLiteDescriptor repeatedInt32Fd = - PROTO_LITE_CEL_VALUE_CONVERTER - .findFieldDescriptor( - "cel.expr.conformance.proto3.TestAllTypes", - TestAllTypes.REPEATED_INT32_FIELD_NUMBER) - .get(); + fieldDescriptor( + "cel.expr.conformance.proto3.TestAllTypes", TestAllTypes.REPEATED_INT32_FIELD_NUMBER); ByteArrayOutputStream emptyThenPopulatedOut = new ByteArrayOutputStream(); CodedOutputStream emptyThenPopulatedCos = CodedOutputStream.newInstance(emptyThenPopulatedOut); emptyThenPopulatedCos.writeByteArray(TestAllTypes.REPEATED_INT32_FIELD_NUMBER, new byte[0]); @@ -618,4 +599,8 @@ public void skipWireField_groupWireType_throwsUnsupportedOperationException() { ProtoLiteCelValueConverter.skipWireField( startGroupTag, ByteString.EMPTY.newCodedInput())); } + + private static FieldLiteDescriptor fieldDescriptor(String messageName, int fieldNumber) { + return PROTO_LITE_CEL_VALUE_CONVERTER.findFieldDescriptor(messageName, fieldNumber).get(); + } } diff --git a/common/src/test/java/dev/cel/common/values/ProtoMessageLiteValueTest.java b/common/src/test/java/dev/cel/common/values/ProtoMessageLiteValueTest.java index ef842e0bb..ebabba660 100644 --- a/common/src/test/java/dev/cel/common/values/ProtoMessageLiteValueTest.java +++ b/common/src/test/java/dev/cel/common/values/ProtoMessageLiteValueTest.java @@ -318,6 +318,69 @@ public void create_withMismatchedWireType_throwsOnSelect( assertThat(thrown).hasCauseThat().hasMessageThat().contains("unexpected wire type"); } + @Test + public void select_otherFieldHasMismatchedWireType_decodesOnlySelectedField() { + // single_fixed32 (7) encoded as a varint precedes single_int64 (2) = 42. The malformed field + // fails to decode only if it is selected. + ByteString bytes = + ByteString.copyFrom(new byte[] {0x38, 0x01}) + .concat(TestAllTypes.newBuilder().setSingleInt64(42L).build().toByteString()); + ProtoMessageLiteValue messageLiteValue = + ProtoMessageLiteValue.create( + bytes, "cel.expr.conformance.proto3.TestAllTypes", PROTO_LITE_CEL_VALUE_CONVERTER); + + Object result = messageLiteValue.select("single_int64"); + + assertThat(result).isEqualTo(42L); + } + + @Test + public void select_undeclaredField_throwsCelAttributeNotFoundException() { + ProtoMessageLiteValue messageLiteValue = + ProtoMessageLiteValue.create( + TestAllTypes.getDefaultInstance(), + "cel.expr.conformance.proto3.TestAllTypes", + PROTO_LITE_CEL_VALUE_CONVERTER); + + CelAttributeNotFoundException thrown = + assertThrows( + CelAttributeNotFoundException.class, () -> messageLiteValue.select("undeclared_field")); + + assertThat(thrown) + .hasMessageThat() + .isEqualTo( + "field 'undeclared_field' is not declared in message" + + " 'cel.expr.conformance.proto3.TestAllTypes'"); + } + + @SuppressWarnings("ImmutableEnumChecker") // Test only + private enum FindFieldTestCase { + PRESENT("single_string", Optional.of("foo")), + ABSENT("single_int64", Optional.empty()), + UNDECLARED("undeclared_field", Optional.empty()); + + private final String fieldName; + private final Optional expected; + + FindFieldTestCase(String fieldName, Optional expected) { + this.fieldName = fieldName; + this.expected = expected; + } + } + + @Test + public void find_returnsValueOnlyIfFieldIsPresent(@TestParameter FindFieldTestCase testCase) { + ProtoMessageLiteValue messageLiteValue = + ProtoMessageLiteValue.create( + TestAllTypes.newBuilder().setSingleString("foo").build(), + "cel.expr.conformance.proto3.TestAllTypes", + PROTO_LITE_CEL_VALUE_CONVERTER); + + Optional result = messageLiteValue.find(testCase.fieldName); + + assertThat(result).isEqualTo(testCase.expected); + } + @SuppressWarnings("ImmutableEnumChecker") // Test only private enum SelectFieldTestCase { BOOL("single_bool", true), diff --git a/protobuf/src/main/java/dev/cel/protobuf/CelLiteDescriptor.java b/protobuf/src/main/java/dev/cel/protobuf/CelLiteDescriptor.java index fcee6215a..14973505c 100644 --- a/protobuf/src/main/java/dev/cel/protobuf/CelLiteDescriptor.java +++ b/protobuf/src/main/java/dev/cel/protobuf/CelLiteDescriptor.java @@ -85,6 +85,10 @@ public Optional findByFieldNumber(int fieldNumber) { return Optional.ofNullable(fieldNumberToFieldDescriptors.get(fieldNumber)); } + public Optional findByFieldName(String fieldName) { + return Optional.ofNullable(fieldNameToFieldDescriptors.get(fieldName)); + } + public FieldLiteDescriptor getByFieldNameOrThrow(String fieldName) { return Objects.requireNonNull(fieldNameToFieldDescriptors.get(fieldName)); } diff --git a/protobuf/src/test/java/dev/cel/protobuf/CelLiteDescriptorTest.java b/protobuf/src/test/java/dev/cel/protobuf/CelLiteDescriptorTest.java index 95dacd6ef..90969a17d 100644 --- a/protobuf/src/test/java/dev/cel/protobuf/CelLiteDescriptorTest.java +++ b/protobuf/src/test/java/dev/cel/protobuf/CelLiteDescriptorTest.java @@ -26,6 +26,7 @@ import dev.cel.protobuf.CelLiteDescriptor.FieldLiteDescriptor.JavaType; import dev.cel.protobuf.CelLiteDescriptor.MessageLiteDescriptor; import java.util.Map; +import java.util.Optional; import org.junit.Test; import org.junit.runner.RunWith; @@ -68,6 +69,32 @@ public void fieldDescriptor_getByFieldNumber() { assertThat(fieldLiteDescriptor.getFieldName()).isEqualTo("single_string"); } + @Test + public void findByFieldName_declaredField_returnsDescriptor() { + MessageLiteDescriptor testAllTypesDescriptor = + TEST_ALL_TYPES_CEL_LITE_DESCRIPTOR + .getProtoTypeNamesToDescriptors() + .get("cel.expr.conformance.proto3.TestAllTypes"); + + Optional fieldLiteDescriptor = + testAllTypesDescriptor.findByFieldName("single_string"); + + assertThat(fieldLiteDescriptor.map(FieldLiteDescriptor::getFieldNumber)).hasValue(14); + } + + @Test + public void findByFieldName_undeclaredField_returnsEmpty() { + MessageLiteDescriptor testAllTypesDescriptor = + TEST_ALL_TYPES_CEL_LITE_DESCRIPTOR + .getProtoTypeNamesToDescriptors() + .get("cel.expr.conformance.proto3.TestAllTypes"); + + Optional fieldLiteDescriptor = + testAllTypesDescriptor.findByFieldName("undeclared_field"); + + assertThat(fieldLiteDescriptor).isEmpty(); + } + @Test public void fieldDescriptor_scalarField() { MessageLiteDescriptor testAllTypesDescriptor =