From 6c771e1c483325f953b0ef6b8fbc76d02aa69632 Mon Sep 17 00:00:00 2001 From: Jasper Potts <1466205+jasperpotts@users.noreply.github.com> Date: Thu, 29 Oct 2026 18:48:09 -0700 Subject: [PATCH 1/4] feat: zero-copy views, part 2: oneofs, repeated fields and wrapper types Extends pbj.views (docs/fast-codegen-plan.md section 6.5) so block items and the messages they contain get views: - oneofs: the view records the alternative seen last; xKind() returns the model's kind enum, and the other alternatives' accessors return defaults; - repeated fields: wrapping validates and counts the elements and records the first one; xCount() and a reusable cursor (for (var it = view.x(); it.next(); ) ...) read unpacked elements and packed runs alike, with child views for messages; - google.protobuf wrapper types: hasX() and a boxed x(), null when absent, as in the model. Messages with maps or repeated wrapper types still have no view. Generated tests check every oneof alternative and repeated element against the model. BlockItemViewBench, in a jmhMax source set built only with -PpbjPerformance=max, routes block items through a view and through parsing. Fixes #1006 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com> --- .../pbj/compiler/impl/LookupHelper.java | 63 +- .../impl/generators/TestGenerator.java | 41 +- .../impl/generators/ViewGenerator.java | 547 +++++++++++++++--- pbj-integration-tests/build.gradle.kts | 5 + .../integration/jmh/BlockItemViewBench.java | 113 ++++ 5 files changed, 669 insertions(+), 100 deletions(-) create mode 100644 pbj-integration-tests/src/jmhMax/java/com/hedera/pbj/integration/jmh/BlockItemViewBench.java diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/LookupHelper.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/LookupHelper.java index 425cbb0d1..039527ef2 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/LookupHelper.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/LookupHelper.java @@ -331,8 +331,8 @@ public boolean hasReverseWriter(String modelPackage, String completeClass) { /** * Whether a message gets a zero-copy view class: it has {@code pbj.views} (from its comments, its file's, or the - * project), every field is singular (repeated fields, maps and oneofs are not supported yet), and every message-typed - * field's type, if generated in this build, gets a view too. + * project), it has no maps or repeated {@code google.protobuf} wrapper types (not supported yet), and every message-typed + * field's type is generated in this build and gets a view too. * * @param message the message * @return true if a view is generated for it @@ -362,7 +362,7 @@ public boolean hasView(String modelPackage, String completeClass) { return message != null && hasView(messageDefinitions.get(message)); } - /** The {@code google.protobuf} wrapper types, which views do not support yet. */ + /** The {@code google.protobuf} wrapper types, which views read as boxed values. */ private static final Set WRAPPER_TYPES = Set.of( "StringValue", "BoolValue", @@ -376,35 +376,50 @@ public boolean hasView(String modelPackage, String completeClass) { private boolean viewSupports(String name, MessageDefContext message) { for (MessageElementContext element : message.messageBody().messageElement()) { - if (element.oneof() != null || element.mapField() != null) { - return false; - } - final var field = element.field(); - if (field == null || field.fieldName() == null) { - continue; - } - if (field.REPEATED() != null) { - return false; - } - final var type = field.type_().messageType(); - if (type == null) { - continue; - } - if (WRAPPER_TYPES.contains(type.messageName().getText())) { + if (element.mapField() != null) { return false; } - final String nestedName = getFullyQualifiedProtoName(messageFiles.get(name), type); - if (enumNames.contains(nestedName)) { - continue; + if (element.oneof() != null) { + for (var alternative : element.oneof().oneofField()) { + if (!viewSupportsType(name, alternative.type_())) { + return false; + } + } } - final MessageDefContext nested = messageDefinitions.get(nestedName); - if (nested == null || !messagesByModelClass.containsValue(nestedName) || !hasView(nested)) { - return false; + final var field = element.field(); + if (field != null && field.fieldName() != null) { + if (!viewSupportsType(name, field.type_())) { + return false; + } + // repeated wrapper types are not supported + final var type = field.type_().messageType(); + if (field.REPEATED() != null + && type != null + && WRAPPER_TYPES.contains(type.messageName().getText())) { + return false; + } } } return true; } + /** Whether a field type is supported by views: anything but messages without views. */ + private boolean viewSupportsType(String name, Protobuf3Parser.Type_Context typeContext) { + final var type = typeContext.messageType(); + if (type == null) { + return true; + } + if (WRAPPER_TYPES.contains(type.messageName().getText())) { + return true; + } + final String nestedName = getFullyQualifiedProtoName(messageFiles.get(name), type); + if (enumNames.contains(nestedName)) { + return true; + } + final MessageDefContext nested = messageDefinitions.get(nestedName); + return nested != null && messagesByModelClass.containsValue(nestedName) && hasView(nested); + } + /** Whether a message has {@code pbj.fast_parse}, from its comments, its file's, or the project. */ private boolean fastParse(String name, MessageDefContext message) { return option(name, message, PerformanceOptions::fastParse); diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/TestGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/TestGenerator.java index c5c1e9ad6..86e8dc1c7 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/TestGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/TestGenerator.java @@ -212,13 +212,50 @@ private static String generateViewTest( final StringBuilder checks = new StringBuilder(); for (final Field field : fields) { final String name = field.nameCamelFirstLower(); + if (field instanceof OneOfField oneOf) { + checks.append("assertEquals(modelObj.%s().kind(), view.%sKind());%n".formatted(name, name)); + for (final Field alternative : oneOf.fields()) { + final String alt = alternative.nameCamelFirstLower(); + final String check = + alternative.type() == Field.FieldType.MESSAGE && !alternative.optionalValueType() + ? "assertEquals(modelObj.%s(), view.%s().toModel());".formatted(alt, alt) + : "assertEquals(modelObj.%s(), view.%s());".formatted(alt, alt); + checks.append("if (modelObj.%s().kind() == %s.%s) {%n %s%n}%n" + .formatted( + name, + oneOf.getEnumClassRef(), + Common.camelToUpperSnake(alternative.name()), + check)); + } + continue; + } + if (field.repeated()) { + checks.append( + """ + { + final List elements = new ArrayList<>(); + for (var it = view.$name(); it.next(); ) { + elements.add($element); + } + assertEquals(modelObj.$getter(), elements); + assertEquals(elements.size(), view.$nameCount()); + } + """.replace("$getter", field.type() == Field.FieldType.ENUM ? name + "ProtoOrdinals" : name) + .replace( + "$element", + field.type() == Field.FieldType.MESSAGE ? "it.value().toModel()" : "it.value()") + .replace("$name", name)); + continue; + } checks.append( switch (field.type()) { case ENUM -> "assertEquals(modelObj.%sProtoOrdinal(), view.%sProtoOrdinal());%n".formatted(name, name); case MESSAGE -> - "assertEquals(modelObj.%s(), view.%s() == null ? null : view.%s().toModel());%n" - .formatted(name, name, name); + field.optionalValueType() + ? "assertEquals(modelObj.%s(), view.%s());%n".formatted(name, name) + : "assertEquals(modelObj.%s(), view.%s() == null ? null : view.%s().toModel());%n" + .formatted(name, name, name); default -> "assertEquals(modelObj.%s(), view.%s());%n".formatted(name, name); }); } diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ViewGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ViewGenerator.java index 9991fed55..dc4421dee 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ViewGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ViewGenerator.java @@ -6,6 +6,7 @@ import com.hedera.pbj.compiler.impl.Common; import com.hedera.pbj.compiler.impl.ContextualLookupHelper; import com.hedera.pbj.compiler.impl.Field; +import com.hedera.pbj.compiler.impl.OneOfField; import com.hedera.pbj.compiler.impl.SingleField; import com.hedera.pbj.compiler.impl.grammar.Protobuf3Parser.MessageDefContext; import java.util.Arrays; @@ -16,14 +17,23 @@ * Generates the zero-copy view of a message with {@code pbj.views}: a reusable flyweight, nested in the model class as * {@code View}, that reads fields in place from encoded bytes. {@code wrap} indexes the message in one pass, with an * in-order fast path and the same validation of tags, wire types, lengths and bools as parsing: numbers are decoded - * into primitive fields once, and strings, bytes and nested messages are recorded as offsets and lengths. Nested - * messages are read through child views the parent creates once and re-wraps. Wrapping and reading numbers, enums, - * nested views and the offsets and lengths of strings and bytes allocate nothing. + * into primitive fields once, and strings, bytes and nested messages are recorded as offsets and lengths. Oneofs record + * which alternative was seen last. Repeated fields record their first element and count, and are read with a reusable + * cursor. Nested messages are read through child views created once and re-wrapped. Wrapping and reading numbers, + * enums, nested views and the offsets and lengths of strings and bytes allocate nothing. */ final class ViewGenerator { private ViewGenerator() {} + /** The code generated for the view, built field by field. */ + private record Parts( + StringBuilder state, + StringBuilder resets, + StringBuilder fastPath, + StringBuilder cases, + StringBuilder accessors) {} + /** * Generate the view class for a message, or nothing if it has no view. * @@ -43,53 +53,20 @@ static String generateView( if (!lookupHelper.getLookupHelper().hasView(msgDef)) { return ""; } - final StringBuilder state = new StringBuilder(); - final StringBuilder resets = new StringBuilder(); - final StringBuilder fastPath = new StringBuilder(); - final StringBuilder cases = new StringBuilder(); - final StringBuilder accessors = new StringBuilder(); - for (final Field f : fields) { - final SingleField field = (SingleField) f; - final String name = field.nameCamelFirstLower(); - final int wireType = field.type().wireType(); - final String record; - if (wireType == Common.TYPE_LENGTH_DELIMITED) { - state.append("private int %sOffset = -1;%nprivate int %sLength;%n".formatted(name, name)); - resets.append("%sOffset = -1;%n".formatted(name)); - record = - "%sLength = cursor.length(Codec.DEFAULT_MAX_SIZE, \"%s\");%n%sOffset = cursor.position();%ncursor.skip(%sLength);%n" - .formatted(name, field.name(), name, name); + final Parts parts = new Parts( + new StringBuilder(), + new StringBuilder(), + new StringBuilder(), + new StringBuilder(), + new StringBuilder()); + for (final Field field : fields) { + if (field instanceof OneOfField oneOf) { + oneOf(oneOf, parts); + } else if (field.repeated()) { + repeated((SingleField) field, parts); } else { - // numbers are decoded once, while wrapping; absent fields keep their default, 0 - state.append("private long %sValue;%n".formatted(name)); - resets.append("%sValue = 0;%n".formatted(name)); - record = "%sValue = %s;%n" - .formatted( - name, - switch (field.type()) { - case BOOL -> "cursor.bool() ? 1 : 0"; - case FIXED32, SFIXED32, FLOAT -> "cursor.fixed32()"; - case FIXED64, SFIXED64, DOUBLE -> "cursor.fixed64()"; - default -> "cursor.varint()"; - }); - } - final int[] tagBytes = Common.tagBytes(field.fieldNumber(), wireType); - if (tagBytes.length <= 2) { - fastPath.append("if (cursor.at(%s)) {%n%s}%n" - .formatted( - Arrays.stream(tagBytes) - .mapToObj(b -> "(byte) 0x%02X".formatted(b)) - .collect(Collectors.joining(", ")), - record.indent(DEFAULT_INDENT))); + singular((SingleField) field, null, parts); } - cases.append("case %d /* [%s] field=%d [%s] */ -> {%n%s}%n" - .formatted( - Common.getTag(wireType, field.fieldNumber()), - field.type(), - field.fieldNumber(), - field.name(), - record.indent(DEFAULT_INDENT))); - accessors.append(accessor(field, name)); } // spotless:off return """ @@ -97,9 +74,10 @@ record = "%sValue = %s;%n" * A reusable, zero-copy view of an encoded {@link $model}: reads its fields in place, without parsing it into * a model object. {@link #wrap} indexes the encoded message, validating it as parsing does; accessors return * fields' defaults when they are absent. Nested messages are returned as child views, created once and - * re-wrapped, so after the first use wrapping and reading numbers, enums, nested views and the offsets and - * lengths of strings and bytes allocate nothing. A view is valid until it is wrapped again, and reads the - * wrapped bytes directly, so they must not change. Not thread safe. + * re-wrapped, and repeated fields are read with reusable cursors, so after the first use wrapping and + * reading numbers, enums, nested views and the offsets and lengths of strings and bytes allocate nothing. + * A view is valid until it is wrapped again, and reads the wrapped bytes directly, so they must not change. + * Not thread safe. */ public static final class View { private final ArrayCursor cursor = new ArrayCursor(new byte[0], 0, 0); @@ -162,6 +140,7 @@ private View index(final int depth) throws ParseException { // in-order fast path, then anything else in any order, as parsing does $fastPath while (cursor.hasRemaining()) { + final int tagStart = cursor.position(); final int tag = cursor.tag(); switch (tag) { $cases @@ -190,45 +169,402 @@ private View index(final int depth) throws ParseException { $accessors } """ - .replace("$state", state.toString().indent(DEFAULT_INDENT)) - .replace("$resets", resets.toString().indent(DEFAULT_INDENT * 2)) - .replace("$fastPath", fastPath.toString().indent(DEFAULT_INDENT * 3)) - .replace("$cases", cases.toString().indent(DEFAULT_INDENT * 5)) - .replace("$accessors", accessors.toString().indent(DEFAULT_INDENT)) + .replace("$state", parts.state().toString().indent(DEFAULT_INDENT)) + .replace("$resets", parts.resets().toString().indent(DEFAULT_INDENT * 2)) + .replace("$fastPath", parts.fastPath().toString().indent(DEFAULT_INDENT * 3)) + .replace("$cases", parts.cases().toString().indent(DEFAULT_INDENT * 5)) + .replace("$accessors", parts.accessors().toString().indent(DEFAULT_INDENT)) .replace("$model", modelClass) .replace("$schema", schemaClass) .indent(DEFAULT_INDENT); // spotless:on } - /** The accessors for one field. */ - private static String accessor(final SingleField field, final String name) { + /** {@code if (cursor.at(…))} or {@code while}, for a tag of at most two bytes; empty for longer tags. */ + private static String fastPath(final String keyword, final int fieldNumber, final int wireType, final String body) { + final int[] tagBytes = Common.tagBytes(fieldNumber, wireType); + if (tagBytes.length > 2) { + return ""; + } + return "%s (cursor.at(%s)) {%n%s}%n" + .formatted( + keyword, + Arrays.stream(tagBytes) + .mapToObj(b -> "(byte) 0x%02X".formatted(b)) + .collect(Collectors.joining(", ")), + body.indent(DEFAULT_INDENT)); + } + + private static String caseCode(final SingleField field, final int wireType, final String body) { + return "case %d /* [%s] field=%d [%s] */ -> {%n%s}%n" + .formatted( + Common.getTag(wireType, field.fieldNumber()), + field.type(), + field.fieldNumber(), + field.name(), + body.indent(DEFAULT_INDENT)); + } + + /** An expression decoding one non-delimited value from {@code cursor} into a long. */ + private static String decode(final Field.FieldType type, final String cursor) { + return switch (type) { + case BOOL -> cursor + ".bool() ? 1 : 0"; + case FIXED32, SFIXED32, FLOAT -> cursor + ".fixed32()"; + case FIXED64, SFIXED64, DOUBLE -> cursor + ".fixed64()"; + default -> cursor + ".varint()"; + }; + } + + /** A statement reading one non-delimited value from {@code cursor}, to validate and skip it. */ + private static String skipValue(final Field.FieldType type, final String cursor) { + return type == Field.FieldType.BOOL ? cursor + ".bool()" : decode(type, cursor); + } + + /** A singular field, or a oneof alternative when {@code oneOfKind} names the oneof's kind field. */ + private static void singular(final SingleField field, final String oneOfKind, final Parts parts) { + final String name = field.nameCamelFirstLower(); + final int wireType = field.optionalValueType() + ? Common.TYPE_LENGTH_DELIMITED + : field.type().wireType(); + String record; + if (wireType == Common.TYPE_LENGTH_DELIMITED) { + parts.state().append("private int %sOffset = -1;%nprivate int %sLength;%n".formatted(name, name)); + parts.resets().append("%sOffset = -1;%n".formatted(name)); + record = + "%sLength = cursor.length(Codec.DEFAULT_MAX_SIZE, \"%s\");%n%sOffset = cursor.position();%ncursor.skip(%sLength);%n" + .formatted(name, field.name(), name, name); + } else { + // numbers are decoded once, while wrapping; absent fields keep their default, 0 + parts.state().append("private long %sValue;%n".formatted(name)); + parts.resets().append("%sValue = 0;%n".formatted(name)); + record = "%sValue = %s;%n".formatted(name, decode(field.type(), "cursor")); + } + if (oneOfKind != null) { + record += "%s = %d;%n".formatted(oneOfKind, field.fieldNumber()); + } else { + parts.fastPath().append(fastPath("if", field.fieldNumber(), wireType, record)); + } + parts.cases().append(caseCode(field, wireType, record)); + final String guard = oneOfKind == null ? "" : "%s == %d".formatted(oneOfKind, field.fieldNumber()); + parts.accessors() + .append(field.optionalValueType() ? wrapperAccessor(field, name, guard) : accessor(field, name, guard)); + } + + /** A oneof: each alternative is recorded like a singular field, and the oneof remembers the last one seen. */ + private static void oneOf(final OneOfField oneOf, final Parts parts) { + final String kind = oneOf.nameCamelFirstLower() + "Kind"; + parts.state().append("private int %s;%n".formatted(kind)); + parts.resets().append("%s = 0;%n".formatted(kind)); + final StringBuilder kinds = new StringBuilder(); + for (final Field alternative : oneOf.fields()) { + singular((SingleField) alternative, kind, parts); + kinds.append("case %d -> %s.%s;%n" + .formatted( + alternative.fieldNumber(), + oneOf.getEnumClassRef(), + Common.camelToUpperSnake(alternative.name()))); + } + parts.accessors().append(""" + + /** @return which alternative of {@code $name} is set, as in the model */ + public $enum $kind() { + return switch ($kind) { + $kinds default -> $enum.UNSET; + }; + } + """.replace("$kinds", kinds.toString().indent(DEFAULT_INDENT * 2)) + .replace("$enum", oneOf.getEnumClassRef()) + .replace("$kind", kind) + .replace("$name", oneOf.name())); + } + + /** + * A repeated field: wrapping validates and counts the elements and records where the first one is; a reusable + * cursor reads them in order, from unpacked elements and packed runs alike. + */ + private static void repeated(final SingleField field, final Parts parts) { + final String name = field.nameCamelFirstLower(); + final int wireType = field.type().wireType(); + final boolean delimited = wireType == Common.TYPE_LENGTH_DELIMITED; + final int elementTag = Common.getTag(wireType, field.fieldNumber()); + final int packedTag = Common.getTag(Common.TYPE_LENGTH_DELIMITED, field.fieldNumber()); + parts.state() + .append("private int %sFirst = -1;%nprivate int %sCount;%nprivate %sCursor %sCursor;%n" + .formatted(name, name, field.nameCamelFirstUpper(), name)); + parts.resets().append("%sFirst = -1;%n%sCount = 0;%n".formatted(name, name)); + final String element = delimited + ? "cursor.skip(cursor.length(Codec.DEFAULT_MAX_SIZE, \"%s\"));%n".formatted(field.name()) + : "%s;%n".formatted(skipValue(field.type(), "cursor")); + final String unpacked = """ + if ($nameFirst < 0) { + $nameFirst = $tagStart; + } + $nameCount++; + $element""".replace("$element", element); + final int tagLength = Common.tagBytes(field.fieldNumber(), wireType).length; + parts.fastPath() + .append(fastPath( + "while", + field.fieldNumber(), + wireType, + unpacked.replace("$tagStart", "cursor.position() - " + tagLength) + .replace("$name", name))); + parts.cases() + .append(caseCode( + field, + wireType, + unpacked.replace("$tagStart", "tagStart").replace("$name", name))); + if (!delimited) { + final String packed = + """ + if ($nameFirst < 0) { + $nameFirst = $tagStart; + } + final int packedLength = cursor.length(Codec.DEFAULT_MAX_SIZE, "$field"); + final int packedEnd = cursor.position() + packedLength; + while (cursor.position() < packedEnd) { + $decode; + $nameCount++; + } + if (cursor.position() != packedEnd) { + throw new java.nio.BufferOverflowException(); + } + """.replace("$decode", skipValue(field.type(), "cursor")).replace("$field", field.name()); + final int packedTagLength = Common.tagBytes(field.fieldNumber(), Common.TYPE_LENGTH_DELIMITED).length; + parts.fastPath() + .append(fastPath( + "if", + field.fieldNumber(), + Common.TYPE_LENGTH_DELIMITED, + packed.replace("$tagStart", "cursor.position() - " + packedTagLength) + .replace("$name", name))); + parts.cases() + .append(caseCode( + field, + Common.TYPE_LENGTH_DELIMITED, + packed.replace("$tagStart", "tagStart").replace("$name", name))); + } + final String read = + switch (field.type()) { + case MESSAGE -> """ + length = c.length(Codec.DEFAULT_MAX_SIZE, "$field"); + offset = c.position(); + c.skip(length); + if (child == null) { + child = new $elem.View(); + } + child.wrap(in, offset, length, depth + 1); + """.replace("$elem", field.javaFieldTypeBase()); + case STRING, BYTES -> """ + length = c.length(Codec.DEFAULT_MAX_SIZE, "$field"); + offset = c.position(); + c.skip(length); + """; + default -> "value = %s;%n".formatted(decode(field.type(), "c")); + }; + final String packedStep = delimited ? "" : """ + if (packedEnd >= 0) { + if (c.position() < packedEnd) { + value = $decode; + return true; + } + packedEnd = -1; + } + """.replace("$decode", decode(field.type(), "c")); + final String packedTagCase = delimited ? "" : """ + if (tag == $packedTag) { + final int packedLength = c.length(Codec.DEFAULT_MAX_SIZE, "$field"); + packedEnd = c.position() + packedLength; + continue; + } + """.replace("$packedTag", Integer.toString(packedTag)); + // spotless:off + parts.accessors().append(""" + + /** @return the number of {@code $field} elements */ + public int $nameCount() { + return $nameCount; + } + + /** + * @return a cursor over the {@code $field} elements, positioned before the first: call + * {@code next()} to move to each one. The same cursor is reset and returned on each call. + */ + public $NameCursor $name() { + if ($nameCursor == null) { + $nameCursor = new $NameCursor(); + } + return $nameCursor.reset(); + } + + /** A reusable cursor over the {@code $field} elements. */ + public final class $NameCursor { + private final ArrayCursor c = new ArrayCursor(new byte[0], 0, 0); + private int remaining; + private int packedEnd = -1; + private long value; + private int offset; + private int length; + private $childType child; + + private $NameCursor reset() { + remaining = $nameCount; + packedEnd = -1; + if ($nameFirst >= 0) { + c.reset(in, $nameFirst, end); + } + return this; + } + + /** + * Move to the next element. + * + * @return false when there are no more elements + * @throws ParseException if a nested message is malformed + */ + public boolean next() throws ParseException { + if (remaining == 0) { + return false; + } + remaining--; + try { + while (true) { + $packedStep + final int tag = c.tag(); + if (tag == $elementTag) { + $read + return true; + } + $packedTagCase + c.unknownField(tag, false, false, false, null, Codec.DEFAULT_MAX_SIZE); + } + } catch (final ParseException e) { + throw e; + } catch (final Exception e) { + throw new ParseException(e); + } + } + $elementAccessors + } + """ + .replace("$packedStep", packedStep.indent(DEFAULT_INDENT * 4)) + .replace("$packedTagCase", packedTagCase.indent(DEFAULT_INDENT * 4)) + .replace("$read", read.indent(DEFAULT_INDENT * 5)) + .replace("$elementAccessors", elementAccessors(field).indent(DEFAULT_INDENT)) + .replace("$childType", field.type() == Field.FieldType.MESSAGE ? field.javaFieldTypeBase() + ".View" : "Object") + .replace("$elementTag", Integer.toString(elementTag)) + .replace("$Name", field.nameCamelFirstUpper()) + .replace("$name", name) + .replace("$field", field.name())); + // spotless:on + } + + /** Accessors for the current element of a repeated field's cursor. */ + private static String elementAccessors(final SingleField field) { + final String doc = "\n/** @return the current element */\n"; + return switch (field.type()) { + case INT32, UINT32, FIXED32, SFIXED32, ENUM -> + doc + "public int value() {%n return (int) value;%n}%n".formatted(); + case SINT32 -> + doc + "public int value() {%n return (int) ((value >>> 1) ^ -(value & 1));%n}%n".formatted(); + case INT64, UINT64, FIXED64, SFIXED64 -> doc + "public long value() {%n return value;%n}%n".formatted(); + case SINT64 -> doc + "public long value() {%n return (value >>> 1) ^ -(value & 1);%n}%n".formatted(); + case BOOL -> doc + "public boolean value() {%n return value != 0;%n}%n".formatted(); + case FLOAT -> + doc + "public float value() {%n return Float.intBitsToFloat((int) value);%n}%n".formatted(); + case DOUBLE -> doc + "public double value() {%n return Double.longBitsToDouble(value);%n}%n".formatted(); + case STRING, BYTES -> + """ + + /** @return the offset of the current element in the wrapped array */ + public int valueOffset() { + return offset; + } + + /** @return the length in bytes of the current element */ + public int valueLength() { + return length; + } + + /** + * @param other bytes to compare with + * @return true if the current element is exactly {@code other}, compared without allocating + */ + public boolean valueEquals(@NonNull final byte[] other) { + return java.util.Arrays.equals(in, offset, offset + length, other, 0, other.length); + } + + /** @return the current element, $returns */ + public $type value() { + return $value; + } + """.replace( + "$returns", + field.type() == Field.FieldType.STRING + ? "decoded as a new String" + : "as a view of the wrapped bytes") + .replace("$type", field.type() == Field.FieldType.STRING ? "String" : "Bytes") + .replace( + "$value", + field.type() == Field.FieldType.STRING + ? "new String(in, offset, length, java.nio.charset.StandardCharsets.UTF_8)" + : "Bytes.wrap(in, offset, length)"); + case MESSAGE -> """ + + /** @return a view of the current element, re-wrapped by each {@code next()} */ + public $type.View value() { + return child; + } + """.replace("$type", field.javaFieldTypeBase()); + default -> throw new IllegalArgumentException("Not supported by views: " + field); + }; + } + + /** The accessors for one singular field or oneof alternative, which return defaults unless {@code guard} holds. */ + private static String accessor(final SingleField field, final String name, final String guard) { final String doc = "%n/** @return {@code %s}, or its default when absent */%n".formatted(field.name()); final String v = name + "Value"; + final String when = guard.isEmpty() ? "" : guard + " ? "; + final String otherwise = guard.isEmpty() ? "" : " : 0"; return switch (field.type()) { case INT32, UINT32, FIXED32, SFIXED32 -> - doc + "public int %s() {%n return (int) %s;%n}%n".formatted(name, v); + doc + "public int %s() {%n return %s(int) %s%s;%n}%n".formatted(name, when, v, otherwise); case SINT32 -> - doc + "public int %s() {%n return (int) ((%s >>> 1) ^ -(%s & 1));%n}%n".formatted(name, v, v); - case INT64, UINT64, FIXED64, SFIXED64 -> doc + "public long %s() {%n return %s;%n}%n".formatted(name, v); - case SINT64 -> doc + "public long %s() {%n return (%s >>> 1) ^ -(%s & 1);%n}%n".formatted(name, v, v); - case BOOL -> doc + "public boolean %s() {%n return %s != 0;%n}%n".formatted(name, v); + doc + + "public int %s() {%n return %s(int) ((%s >>> 1) ^ -(%s & 1))%s;%n}%n" + .formatted(name, when, v, v, otherwise); + case INT64, UINT64, FIXED64, SFIXED64 -> + doc + "public long %s() {%n return %s%s%s;%n}%n".formatted(name, when, v, otherwise); + case SINT64 -> + doc + + "public long %s() {%n return %s(%s >>> 1) ^ -(%s & 1)%s;%n}%n" + .formatted(name, when, v, v, otherwise); + case BOOL -> + doc + + "public boolean %s() {%n return %s%s != 0;%n}%n" + .formatted(name, guard.isEmpty() ? "" : guard + " && ", v); case FLOAT -> - doc + "public float %s() {%n return Float.intBitsToFloat((int) %s);%n}%n".formatted(name, v); + doc + + "public float %s() {%n return %sFloat.intBitsToFloat((int) %s)%s;%n}%n" + .formatted(name, when, v, otherwise); case DOUBLE -> - doc + "public double %s() {%n return Double.longBitsToDouble(%s);%n}%n".formatted(name, v); + doc + + "public double %s() {%n return %sDouble.longBitsToDouble(%s)%s;%n}%n" + .formatted(name, when, v, otherwise); case ENUM -> doc + """ public int $nameProtoOrdinal() { - return (int) $nameValue; + return $when(int) $nameValue$otherwise; } /** @return {@code $field} as an enum constant, {@code UNRECOGNIZED} for an unknown ordinal */ public $type $name() { return $type.fromProtobufOrdinal($nameProtoOrdinal()); } - """.replace("$type", Common.snakeToCamel(field.messageType(), true)) + """.replace("$when", when) + .replace("$otherwise", otherwise) + .replace("$type", Common.snakeToCamel(field.messageType(), true)) .replace("$name", name) .replace("$field", field.name()); case STRING, BYTES -> @@ -236,12 +572,12 @@ private static String accessor(final SingleField field, final String name) { /** @return the offset of {@code $field} in {@link #$nameArray()}, 0 when absent */ public int $nameOffset() { - return $nameOffset < 0 ? 0 : $nameOffset; + return $absent ? 0 : $nameOffset; } /** @return the length in bytes of {@code $field}, 0 when absent */ public int $nameLength() { - return $nameOffset < 0 ? 0 : $nameLength; + return $absent ? 0 : $nameLength; } /** @return the wrapped array, which holds {@code $field}; do not modify it */ @@ -260,9 +596,10 @@ private static String accessor(final SingleField field, final String name) { /** @return {@code $field}, $returns */ public $type $name() { - return $nameOffset < 0 ? $default : $value; + return $absent ? $default : $value; } - """.replace( + """.replace("$absent", absent(name, guard)) + .replace( "$returns", field.type() == Field.FieldType.STRING ? "decoded as a new String" @@ -283,7 +620,7 @@ private static String accessor(final SingleField field, final String name) { /** @return true if {@code $field} is present */ public boolean has$Name() { - return $nameOffset >= 0; + return !($absent); } /** @@ -291,7 +628,7 @@ private static String accessor(final SingleField field, final String name) { * @throws ParseException if the nested message is malformed */ public $type.View $name() throws ParseException { - if ($nameOffset < 0) { + if ($absent) { return null; } if ($nameView == null) { @@ -299,11 +636,73 @@ private static String accessor(final SingleField field, final String name) { } return $nameView.wrap(in, $nameOffset, $nameLength, depth + 1); } - """.replace("$type", field.javaFieldTypeBase()) + """.replace("$absent", absent(name, guard)) + .replace("$type", field.javaFieldTypeBase()) .replace("$Name", field.nameCamelFirstUpper()) .replace("$name", name) .replace("$field", field.name()); default -> throw new IllegalArgumentException("Not supported by views yet: " + field); }; } + + /** + * The accessors for a {@code google.protobuf} wrapper type: like the model, null when absent and the default value + * when the wrapper is empty. The value is decoded with the view's own cursor, which is free once wrapping is done. + */ + private static String wrapperAccessor(final SingleField field, final String name, final String guard) { + final String[] typeReadDefault = + switch (field.messageType()) { + case "StringValue" -> new String[] {"String", "cursor.string(Codec.DEFAULT_MAX_SIZE)", "\"\""}; + case "BytesValue" -> new String[] {"Bytes", "cursor.bytes(Codec.DEFAULT_MAX_SIZE)", "Bytes.EMPTY"}; + case "Int32Value", "UInt32Value" -> new String[] {"Integer", "(int) cursor.varint()", "0"}; + case "Int64Value", "UInt64Value" -> new String[] {"Long", "cursor.varint()", "0L"}; + case "FloatValue" -> new String[] {"Float", "Float.intBitsToFloat(cursor.fixed32())", "0f"}; + case "DoubleValue" -> new String[] {"Double", "Double.longBitsToDouble(cursor.fixed64())", "0d"}; + case "BoolValue" -> new String[] {"Boolean", "cursor.varint() != 0", "false"}; + default -> throw new IllegalArgumentException("Not a wrapper type: " + field); + }; + return """ + + /** @return true if {@code $field} is present */ + public boolean has$Name() { + return !($absent); + } + + /** + * @return {@code $field}: null when absent, as in the model + * @throws ParseException if the wrapper is malformed + */ + public $type $name() throws ParseException { + if ($absent) { + return null; + } + if ($nameLength == 0) { + return $default; + } + try { + cursor.reset(in, $nameOffset, $nameOffset + $nameLength); + // the wrapper's one field: its tag, then the value + cursor.tag(); + return $read; + $catch + } + """.replace( + "$catch", + field.messageType().equals("StringValue") + || field.messageType().equals("BytesValue") + ? "} catch (final ParseException e) {\n throw e;\n } catch (final Exception e) {\n throw new ParseException(e);\n }" + : "} catch (final RuntimeException e) {\n throw new ParseException(e);\n }") + .replace("$absent", absent(name, guard)) + .replace("$type", typeReadDefault[0]) + .replace("$read", typeReadDefault[1]) + .replace("$default", typeReadDefault[2]) + .replace("$Name", field.nameCamelFirstUpper()) + .replace("$name", name) + .replace("$field", field.name()); + } + + /** The condition for a delimited field being absent: no offset recorded, or another oneof alternative set. */ + private static String absent(final String name, final String guard) { + return guard.isEmpty() ? name + "Offset < 0" : "!(" + guard + ") || " + name + "Offset < 0"; + } } diff --git a/pbj-integration-tests/build.gradle.kts b/pbj-integration-tests/build.gradle.kts index 8420cc300..4e7f8214e 100644 --- a/pbj-integration-tests/build.gradle.kts +++ b/pbj-integration-tests/build.gradle.kts @@ -101,6 +101,11 @@ require(pbjPerformance == "default" || pbjPerformance == "max") { "pbjPerformance must be \"default\" or \"max\", not \"$pbjPerformance\"" } +// Benchmarks of APIs that exist only with every option on, such as views of HAPI messages +if (pbjPerformance == "max") { + sourceSets.named("jmh") { java.srcDir("src/jmhMax/java") } +} + // Tests can tell which configuration they run in tasks.withType().configureEach { systemProperties["pbj.performance"] = pbjPerformance } diff --git a/pbj-integration-tests/src/jmhMax/java/com/hedera/pbj/integration/jmh/BlockItemViewBench.java b/pbj-integration-tests/src/jmhMax/java/com/hedera/pbj/integration/jmh/BlockItemViewBench.java new file mode 100644 index 000000000..273014bfb --- /dev/null +++ b/pbj-integration-tests/src/jmhMax/java/com/hedera/pbj/integration/jmh/BlockItemViewBench.java @@ -0,0 +1,113 @@ +// SPDX-License-Identifier: Apache-2.0 +package com.hedera.pbj.integration.jmh; + +import com.hedera.hapi.block.stream.Block; +import com.hedera.hapi.block.stream.BlockItem; +import com.hedera.hapi.block.stream.output.StateChange; +import com.hedera.hapi.block.stream.output.StateChanges; +import com.hedera.pbj.runtime.ParseException; +import com.hedera.pbj.runtime.io.buffer.Bytes; +import java.io.BufferedInputStream; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; +import java.util.concurrent.TimeUnit; +import java.util.zip.GZIPInputStream; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OperationsPerInvocation; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; + +/** + * Routing block items, per item: read the item's kind, and for state changes the consensus time and the id and + * operation of every change; for signed transactions their length. Through a reusable zero-copy view, against parsing + * each item into model objects. Only built with {@code -PpbjPerformance=max}, where HAPI messages have views. + */ +@SuppressWarnings("unused") +@State(Scope.Thread) +@Fork(3) +@Warmup(iterations = 3, time = 1) +@Measurement(iterations = 5, time = 1) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@BenchmarkMode(Mode.AverageTime) +public class BlockItemViewBench { + private static final int ITEMS = 67_869; + private static final List ITEM_BYTES = new ArrayList<>(); + + static { + try (var in = new BufferedInputStream(new GZIPInputStream(Objects.requireNonNull( + BlockItemViewBench.class.getResourceAsStream("/000000000000000000000000000000030221.blk.gz"))))) { + for (final BlockItem item : + Block.PROTOBUF.parse(Bytes.wrap(in.readAllBytes())).items()) { + ITEM_BYTES.add(BlockItem.PROTOBUF.toBytes(item)); + } + } catch (IOException e) { + throw new UncheckedIOException(e); + } catch (ParseException e) { + throw new IllegalStateException(e); + } + if (ITEM_BYTES.size() != ITEMS) { + throw new IllegalStateException("Update ITEMS to " + ITEM_BYTES.size()); + } + } + + private final BlockItem.View view = new BlockItem.View(); + + @Benchmark + @OperationsPerInvocation(ITEMS) + public void routeWithView(final Blackhole bh) throws ParseException { + for (final Bytes bytes : ITEM_BYTES) { + final BlockItem.View item = view.wrap(bytes); + long sum = item.itemKind().ordinal(); + switch (item.itemKind()) { + case STATE_CHANGES -> { + final StateChanges.View changes = item.stateChanges(); + if (changes.hasConsensusTimestamp()) { + sum += changes.consensusTimestamp().seconds(); + } + for (var it = changes.stateChanges(); it.next(); ) { + sum += it.value().stateId() + + it.value().changeOperationKind().ordinal(); + } + } + case SIGNED_TRANSACTION -> sum += item.signedTransactionLength(); + default -> {} + } + bh.consume(sum); + } + } + + @Benchmark + @OperationsPerInvocation(ITEMS) + public void routeWithParse(final Blackhole bh) throws ParseException { + for (final Bytes bytes : ITEM_BYTES) { + final BlockItem item = BlockItem.PROTOBUF.parse(bytes); + long sum = item.item().kind().ordinal(); + switch (item.item().kind()) { + case STATE_CHANGES -> { + final StateChanges changes = item.stateChangesOrThrow(); + if (changes.consensusTimestamp() != null) { + sum += changes.consensusTimestamp().seconds(); + } + for (final StateChange change : changes.stateChanges()) { + sum += change.stateId() + + change.changeOperation().kind().ordinal(); + } + } + case SIGNED_TRANSACTION -> + sum += item.signedTransactionOrThrow().length(); + default -> {} + } + bh.consume(sum); + } + } +} From 75d1213a96b3bea0c0184c0d49a22073f30b7240 Mon Sep 17 00:00:00 2001 From: Jasper Potts <1466205+jasperpotts@users.noreply.github.com> Date: Thu, 29 Oct 2026 23:26:58 -0700 Subject: [PATCH 2/4] fix: views validate with the caller's limits and strict mode, and reuse child views - wrap(bytes, strictMode, maxDepth, maxSize) and wrap(in, offset, length, strictMode, maxDepth, maxSize) validate as parse does with the same arguments, and pass them to child views and repeated field cursors. Strict mode rejects unknown fields and repeated singular messages. - Repeated string elements are checked to be valid UTF-8 while wrapping. - A nested accessor returns its child view without indexing it again if it is still wrapped over the field since the parent was wrapped. - LookupHelper.hasView is a greatest fixed point, so a cycle of messages through one without a view gives none of them a view, whichever is looked at first. The build says why a view is denied to a message that asks for one in its comments. - A message with views that declares its own nested type named View is a clear compiler error instead of a duplicate class. - Removes the unused, unchecked static ArrayCursor helpers. - Documents views, and the reuse of child views, in the usage guide. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com> --- docs/usage-guide.md | 41 +++ .../pbj/compiler/impl/LookupHelper.java | 189 +++++++++----- .../impl/generators/ViewGenerator.java | 173 ++++++++++--- .../pbj/compiler/ViewGenerationTest.java | 134 ++++++++++ .../pbj/runtime/io/buffer/ArrayCursor.java | 40 --- .../src/main/proto/viewChecks.proto | 55 ++++ .../integration/test/ViewValidationTest.java | 239 ++++++++++++++++++ 7 files changed, 733 insertions(+), 138 deletions(-) create mode 100644 pbj-core/pbj-compiler/src/test/java/com/hedera/pbj/compiler/ViewGenerationTest.java create mode 100644 pbj-integration-tests/src/main/proto/viewChecks.proto create mode 100644 pbj-integration-tests/src/test/java/com/hedera/pbj/integration/test/ViewValidationTest.java diff --git a/docs/usage-guide.md b/docs/usage-guide.md index 27eb8778d..e2d29c30f 100644 --- a/docs/usage-guide.md +++ b/docs/usage-guide.md @@ -360,6 +360,47 @@ HelloRequest msg = HelloRequest.PROTOBUF.parse( Strict mode throws `UnknownFieldException` on unrecognized fields — useful for validation. +### Zero-Copy Views + +With `pbj.views`, a model gets a nested `View` class that reads fields in place from the encoded bytes, without +parsing them into objects. A view is reusable: wrap it again for the next message. + +```java +final Order.View view = new Order.View(); +for (Bytes bytes : orders) { + view.wrap(bytes); // or wrap(bytes, strictMode, maxDepth, maxSize), as for parse() + total += view.amount(); + for (var line = view.lines(); line.next(); ) { + count += line.value().quantity(); + } +} +``` + +`wrap` validates the message as `parse` does, with the same limits and strict mode. Nested messages, repeated message +elements and wrapper type values are validated when they are read, so a malformed one throws from its accessor or +cursor rather than from `wrap`. Messages with maps, or that contain a message without a view, get no view; the build +says why for messages that ask for one in their comments. + +**Views and cursors are reused, so what they return changes under you.** A nested accessor returns the same child view +every time, re-wrapped when the parent is, and a repeated field's cursor re-wraps one child view for each element. +Keeping a reference and reading it later reads whatever the view was last wrapped over: + +```java +// Wrong: every element of the list is the same child view, wrapped over the last line +List lines = new ArrayList<>(); +for (var line = view.lines(); line.next(); ) { + lines.add(line.value()); +} + +// Right: copy what you need, or parse the element into a model object +List lines = new ArrayList<>(); +for (var line = view.lines(); line.next(); ) { + lines.add(line.value().toModel()); +} +``` + +The wrapped bytes must not change while a view reads them. + ### Streaming I/O Use `ReadableStreamingData` and `WritableStreamingData` for file or network I/O: diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/LookupHelper.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/LookupHelper.java index 039527ef2..b56f6871d 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/LookupHelper.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/LookupHelper.java @@ -19,6 +19,7 @@ import java.nio.file.FileSystems; import java.nio.file.Path; import java.util.ArrayDeque; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; @@ -326,8 +327,15 @@ public boolean hasReverseWriter(String modelPackage, String completeClass) { return message != null && hasReverseWriter(messageDefinitions.get(message)); } - /** Whether each message gets a view, by fully qualified proto name, computed on demand. */ - private final Map views = new HashMap<>(); + /** The messages that get a view, by fully qualified proto name, computed on first use. */ + private Set views; + + /** + * Why each message that asks for a view in its own or its file's comments gets none, by fully qualified proto name. + * Messages that have {@code pbj.views} only from the project are left out, so turning everything on doesn't report + * every message with a map. + */ + private final Map viewDenials = new HashMap<>(); /** * Whether a message gets a zero-copy view class: it has {@code pbj.views} (from its comments, its file's, or the @@ -338,88 +346,151 @@ public boolean hasReverseWriter(String modelPackage, String completeClass) { * @return true if a view is generated for it */ public boolean hasView(MessageDefContext message) { - final String name = getFullyQualifiedProtoNameForContext(message); - final Boolean known = views.get(name); - if (known != null) { - return known; - } - // provisionally true, so messages that contain each other don't recurse forever - views.put(name, Boolean.TRUE); - final boolean result = option(name, message, PerformanceOptions::views) && viewSupports(name, message); - views.put(name, result); - return result; + return views().contains(getFullyQualifiedProtoNameForContext(message)); } /** - * Whether the model of a message generated in this build has a view, so another view can return it. + * Why a message that has {@code pbj.views} gets no view. * - * @param modelPackage the Java package of the message's model class - * @param completeClass the model's complete class name, including outer classes - * @return true if the message is generated in this build with a view + * @param message the message + * @return the reason, or null if the message gets a view or does not ask for one */ - public boolean hasView(String modelPackage, String completeClass) { - final String message = messagesByModelClass.get(modelPackage + "." + completeClass); - return message != null && hasView(messageDefinitions.get(message)); + public String viewDenial(MessageDefContext message) { + views(); + return viewDenials.get(getFullyQualifiedProtoNameForContext(message)); } - /** The {@code google.protobuf} wrapper types, which views read as boxed values. */ - private static final Set WRAPPER_TYPES = Set.of( - "StringValue", - "BoolValue", - "Int32Value", - "UInt32Value", - "Int64Value", - "UInt64Value", - "FloatValue", - "DoubleValue", - "BytesValue"); + /** + * Compute which messages get views, as a greatest fixed point: start with every message that asks for a view and + * supports one on its own, then drop those with a message-typed field whose type was dropped, until nothing + * changes. Messages that contain each other, directly or through others, keep their views only if all of them can. + */ + private Set views() { + if (views != null) { + return views; + } + final Set generated = new HashSet<>(messagesByModelClass.values()); + // each candidate's message-typed fields: field name and type, which must get views too + final Map> candidates = new HashMap<>(); + for (Map.Entry entry : messageDefinitions.entrySet()) { + final String name = entry.getKey(); + if (generated.contains(name) && option(name, entry.getValue(), PerformanceOptions::views)) { + final Map nested = viewNestedTypes(name, entry.getValue(), generated); + if (nested != null) { + candidates.put(name, nested); + } + } + } + boolean changed = true; + while (changed) { + changed = false; + for (var it = candidates.entrySet().iterator(); it.hasNext(); ) { + final var candidate = it.next(); + for (Map.Entry field : candidate.getValue().entrySet()) { + if (!candidates.containsKey(field.getValue())) { + viewDenials.put( + candidate.getKey(), + "field '%s' has type %s, which gets no view" + .formatted(field.getKey(), field.getValue())); + it.remove(); + changed = true; + break; + } + } + } + } + viewDenials + .keySet() + .removeIf(denied -> !PerformanceOptions.resolve( + levels(denied, messageDefinitions.get(denied)).subList(0, 2), + PerformanceOptions::views, + true, + false)); + views = candidates.keySet(); + return views; + } - private boolean viewSupports(String name, MessageDefContext message) { + /** + * The message-typed fields of a message that asks for a view, by field name, with their types' proto names; or + * null, with the reason recorded, if the message can't have a view whatever its nested types get. + */ + private Map viewNestedTypes(String name, MessageDefContext message, Set generated) { + final Map nested = new HashMap<>(); for (MessageElementContext element : message.messageBody().messageElement()) { if (element.mapField() != null) { - return false; + viewDenials.put( + name, + "map field '%s' (maps are not supported by views)" + .formatted(element.mapField().mapName().getText())); + return null; } + final List> fields = new ArrayList<>(); if (element.oneof() != null) { for (var alternative : element.oneof().oneofField()) { - if (!viewSupportsType(name, alternative.type_())) { - return false; - } + fields.add(Map.entry(alternative.fieldName().getText(), alternative.type_())); } } final var field = element.field(); if (field != null && field.fieldName() != null) { - if (!viewSupportsType(name, field.type_())) { - return false; - } // repeated wrapper types are not supported final var type = field.type_().messageType(); if (field.REPEATED() != null && type != null && WRAPPER_TYPES.contains(type.messageName().getText())) { - return false; + viewDenials.put( + name, + "repeated wrapper type field '%s' (not supported by views)" + .formatted(field.fieldName().getText())); + return null; } + fields.add(Map.entry(field.fieldName().getText(), field.type_())); + } + for (Map.Entry f : fields) { + final var type = f.getValue().messageType(); + if (type == null || WRAPPER_TYPES.contains(type.messageName().getText())) { + continue; + } + final String nestedName = getFullyQualifiedProtoName(messageFiles.get(name), type); + if (enumNames.contains(nestedName)) { + continue; + } + if (!generated.contains(nestedName)) { + viewDenials.put( + name, + "field '%s' has type %s, which is not generated in this build" + .formatted(f.getKey(), nestedName)); + return null; + } + nested.put(f.getKey(), nestedName); } } - return true; + return nested; } - /** Whether a field type is supported by views: anything but messages without views. */ - private boolean viewSupportsType(String name, Protobuf3Parser.Type_Context typeContext) { - final var type = typeContext.messageType(); - if (type == null) { - return true; - } - if (WRAPPER_TYPES.contains(type.messageName().getText())) { - return true; - } - final String nestedName = getFullyQualifiedProtoName(messageFiles.get(name), type); - if (enumNames.contains(nestedName)) { - return true; - } - final MessageDefContext nested = messageDefinitions.get(nestedName); - return nested != null && messagesByModelClass.containsValue(nestedName) && hasView(nested); + /** + * Whether the model of a message generated in this build has a view, so another view can return it. + * + * @param modelPackage the Java package of the message's model class + * @param completeClass the model's complete class name, including outer classes + * @return true if the message is generated in this build with a view + */ + public boolean hasView(String modelPackage, String completeClass) { + final String message = messagesByModelClass.get(modelPackage + "." + completeClass); + return message != null && hasView(messageDefinitions.get(message)); } + /** The {@code google.protobuf} wrapper types, which views read as boxed values. */ + private static final Set WRAPPER_TYPES = Set.of( + "StringValue", + "BoolValue", + "Int32Value", + "UInt32Value", + "Int64Value", + "UInt64Value", + "FloatValue", + "DoubleValue", + "BytesValue"); + /** Whether a message has {@code pbj.fast_parse}, from its comments, its file's, or the project. */ private boolean fastParse(String name, MessageDefContext message) { return option(name, message, PerformanceOptions::fastParse); @@ -427,14 +498,18 @@ private boolean fastParse(String name, MessageDefContext message) { /** Resolve a boolean fast code generation option for a message, from its comments, its file's, or the project. */ private boolean option(String name, MessageDefContext message, Function option) { + return PerformanceOptions.resolve(levels(name, message), option, true, false); + } + + /** The options that apply to a message, in order of precedence: its comments, its file's, and the project's. */ + private List levels(String name, MessageDefContext message) { final File file = messageFiles.get(name); - final List levels = List.of( + return List.of( PerformanceOptions.fromComments(message.optionComment(), "message " + name), file == null ? PerformanceOptions.NONE : fileOptions.getOrDefault(file.getAbsolutePath(), PerformanceOptions.NONE), projectOptions); - return PerformanceOptions.resolve(levels, option, true, false); } /** Read a boolean PBJ message option from its special comments, defaulting to false. */ diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ViewGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ViewGenerator.java index 91cdc7202..04b841629 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ViewGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ViewGenerator.java @@ -7,8 +7,10 @@ import com.hedera.pbj.compiler.impl.ContextualLookupHelper; import com.hedera.pbj.compiler.impl.Field; import com.hedera.pbj.compiler.impl.OneOfField; +import com.hedera.pbj.compiler.impl.PbjCompilerException; import com.hedera.pbj.compiler.impl.SingleField; import com.hedera.pbj.compiler.impl.grammar.Protobuf3Parser.MessageDefContext; +import com.hedera.pbj.compiler.impl.grammar.Protobuf3Parser.MessageElementContext; import java.util.Arrays; import java.util.List; import java.util.stream.Collectors; @@ -51,8 +53,23 @@ static String generateView( final String schemaClass, final ContextualLookupHelper lookupHelper) { if (!lookupHelper.getLookupHelper().hasView(msgDef)) { + final String denial = lookupHelper.getLookupHelper().viewDenial(msgDef); + if (denial != null) { + System.err.printf("PBJ: no view for message %s: %s%n", modelClass, denial); + } return ""; } + for (final MessageElementContext element : msgDef.messageBody().messageElement()) { + final String nested = element.messageDef() != null + ? element.messageDef().messageName().getText() + : element.enumDef() != null ? element.enumDef().enumName().getText() : null; + if ("View".equals(nested)) { + throw new PbjCompilerException(("Message %s has pbj.views but declares its own nested type named View, " + + "which would clash with the generated view class: rename it, or turn pbj.views off " + + "for this message with a <<>> comment") + .formatted(modelClass)); + } + } final Parts parts = new Parts( new StringBuilder(), new StringBuilder(), @@ -72,34 +89,61 @@ static String generateView( return """ /** * A reusable, zero-copy view of an encoded {@link $model}: reads its fields in place, without parsing it into - * a model object. {@link #wrap} indexes the encoded message, validating it as parsing does; accessors return - * fields' defaults when they are absent. Nested messages are returned as child views, created once and - * re-wrapped, and repeated fields are read with reusable cursors, so after the first use wrapping and - * reading numbers, enums, nested views and the offsets and lengths of strings and bytes allocate nothing. - * A view is valid until it is wrapped again, and reads the wrapped bytes directly, so they must not change. - * Not thread safe. + * a model object. Accessors return fields' defaults when they are absent. Nested messages are returned as + * child views, created once and re-wrapped, and repeated fields are read with reusable cursors, so after the + * first use wrapping and reading numbers, enums, nested views and the offsets and lengths of strings and + * bytes allocate nothing. + * + *

{@link #wrap} indexes this message and validates it as parsing does, with the same limits and strict + * mode: tags, wire types, lengths, bools, UTF-8 strings, unknown fields and, in strict mode, repeated + * singular messages. Nested messages, repeated message elements and wrapper type values are validated + * when they are read, so a malformed one throws from its accessor or cursor rather than from {@code wrap}. + * + *

A view is valid until it is wrapped again, and reads the wrapped bytes directly, so they must not + * change. Child views and cursors are reused: the view a nested accessor or a cursor's {@code value()} + * returns changes on the next call, so copy what you need, or call {@code toModel()}, before reading + * further. Not thread safe. */ public static final class View { private final ArrayCursor cursor = new ArrayCursor(new byte[0], 0, 0); private byte[] in; private int start; private int end; - private int depth; + private boolean strictMode; + private int maxDepth; + private int maxSize; $state /** - * Wrap an encoded $model. + * Wrap an encoded $model, with the defaults of {@link Codec#parse(Bytes)}. * * @param bytes the encoded message * @return this view * @throws ParseException if the message is malformed */ public View wrap(@NonNull final Bytes bytes) throws ParseException { + return wrap(bytes, false, Codec.DEFAULT_MAX_DEPTH, Codec.DEFAULT_MAX_SIZE); + } + + /** + * Wrap an encoded $model, validating it as {@link Codec#parse} does with the same arguments. + * + * @param bytes the encoded message + * @param strictMode when {@code true}, unknown fields and repeated singular messages are errors + * @param maxDepth the maximum depth of nested messages + * @param maxSize the maximum length of a field, nested message or unknown field + * @return this view + * @throws ParseException if the message is malformed + */ + public View wrap( + @NonNull final Bytes bytes, final boolean strictMode, final int maxDepth, final int maxSize) + throws ParseException { cursor.reset(bytes); - return index(0); + return index(strictMode, maxDepth, maxSize); } /** - * Wrap an encoded $model in {@code in[offset, offset + length)}. + * Wrap an encoded $model in {@code in[offset, offset + length)}, with the defaults of + * {@link Codec#parse(Bytes)}. * * @param in the bytes * @param offset where the message starts @@ -108,33 +152,58 @@ public View wrap(@NonNull final Bytes bytes) throws ParseException { * @throws ParseException if the message is malformed */ public View wrap(@NonNull final byte[] in, final int offset, final int length) throws ParseException { - return wrap(in, offset, length, 0); + return wrap(in, offset, length, false, Codec.DEFAULT_MAX_DEPTH, Codec.DEFAULT_MAX_SIZE); } /** - * Wrap an encoded $model nested {@code depth} messages deep, for the views that contain it. + * Wrap an encoded $model in {@code in[offset, offset + length)}, validating it as {@link Codec#parse} + * does with the same arguments. Views also use it for the messages they contain. * * @param in the bytes * @param offset where the message starts * @param length the message's length - * @param depth how deep the message is nested + * @param strictMode when {@code true}, unknown fields and repeated singular messages are errors + * @param maxDepth the maximum depth of nested messages below this one + * @param maxSize the maximum length of a field, nested message or unknown field * @return this view * @throws ParseException if the message is malformed or nested too deep */ - public View wrap(@NonNull final byte[] in, final int offset, final int length, final int depth) + public View wrap( + @NonNull final byte[] in, + final int offset, + final int length, + final boolean strictMode, + final int maxDepth, + final int maxSize) throws ParseException { cursor.reset(in, offset, offset + length); - return index(depth); + return index(strictMode, maxDepth, maxSize); + } + + /** + * Whether this view wraps exactly {@code in[offset, offset + length)}, so a containing view can return + * it again without indexing it again. + * + * @param in the bytes + * @param offset where the message starts + * @param length the message's length + * @return true if this view is valid and wraps those bytes + */ + public boolean wraps(@NonNull final byte[] in, final int offset, final int length) { + return this.in == in && start == offset && end == offset + length; } - private View index(final int depth) throws ParseException { - if (depth > Codec.DEFAULT_MAX_DEPTH) { + private View index(final boolean strictMode, final int maxDepth, final int maxSize) + throws ParseException { + if (maxDepth < 0) { throw new ParseException("Reached maximum allowed depth of nested messages"); } - this.depth = depth; - in = cursor.array(); - start = cursor.position(); - end = cursor.limit(); + // invalid until indexing succeeds + in = null; + this.strictMode = strictMode; + this.maxDepth = maxDepth; + this.maxSize = maxSize; + final int start = cursor.position(); $resets try { // in-order fast path, then anything else in any order, as parsing does @@ -146,7 +215,7 @@ private View index(final int depth) throws ParseException { $cases default -> cursor.unknownField( tag, $schema.getField(tag >>> 3) != null, - false, false, null, Codec.DEFAULT_MAX_SIZE); + strictMode, false, null, maxSize); } } } catch (final ParseException e) { @@ -154,6 +223,9 @@ private View index(final int depth) throws ParseException { } catch (final Exception e) { throw new ParseException(e); } + this.start = start; + end = cursor.limit(); + in = cursor.array(); return this; } @@ -231,20 +303,31 @@ private static void singular(final SingleField field, final String oneOfKind, fi parts.state().append("private int %sOffset = -1;%nprivate int %sLength;%n".formatted(name, name)); parts.resets().append("%sOffset = -1;%n".formatted(name)); // strings are checked to be valid UTF-8 while wrapping, as parsing does - record = - "%sLength = cursor.%s(Codec.DEFAULT_MAX_SIZE, \"%s\");%n%sOffset = cursor.position();%ncursor.skip(%sLength);%n" - .formatted( - name, - field.type() == Field.FieldType.STRING ? "stringLength" : "length", - field.name(), - name, - name); + record = "%sLength = cursor.%s(maxSize, \"%s\");%n%sOffset = cursor.position();%ncursor.skip(%sLength);%n" + .formatted( + name, + field.type() == Field.FieldType.STRING ? "stringLength" : "length", + field.name(), + name, + name); } else { // numbers are decoded once, while wrapping; absent fields keep their default, 0 parts.state().append("private long %sValue;%n".formatted(name)); parts.resets().append("%sValue = 0;%n".formatted(name)); record = "%sValue = %s;%n".formatted(name, decode(field.type(), "cursor")); } + if (field.type() == Field.FieldType.MESSAGE && !field.optionalValueType()) { + // as in parsing, strict mode rejects a singular message, or a oneof with a message alternative, seen twice + record = """ + if (strictMode && %s) { + throw new ParseException("A non-repeated %s encountered more than once"); + } + """.formatted( + oneOfKind == null ? name + "Offset >= 0" : oneOfKind + " != 0", + oneOfKind == null ? field.name() : field.parent().name()) + + record; + parts.resets().append("%sViewCurrent = false;%n".formatted(name)); + } if (oneOfKind != null) { record += "%s = %d;%n".formatted(oneOfKind, field.fieldNumber()); } else { @@ -299,7 +382,8 @@ private static void repeated(final SingleField field, final Parts parts) { .formatted(name, name, field.nameCamelFirstUpper(), name)); parts.resets().append("%sFirst = -1;%n%sCount = 0;%n".formatted(name, name)); final String element = delimited - ? "cursor.skip(cursor.length(Codec.DEFAULT_MAX_SIZE, \"%s\"));%n".formatted(field.name()) + ? "cursor.skip(cursor.%s(maxSize, \"%s\"));%n" + .formatted(field.type() == Field.FieldType.STRING ? "stringLength" : "length", field.name()) : "%s;%n".formatted(skipValue(field.type(), "cursor")); final String unpacked = """ if ($nameFirst < 0) { @@ -326,7 +410,7 @@ private static void repeated(final SingleField field, final Parts parts) { if ($nameFirst < 0) { $nameFirst = $tagStart; } - final int packedLength = cursor.length(Codec.DEFAULT_MAX_SIZE, "$field"); + final int packedLength = cursor.length(maxSize, "$field"); final int packedEnd = cursor.position() + packedLength; while (cursor.position() < packedEnd) { $decode; @@ -353,16 +437,16 @@ private static void repeated(final SingleField field, final Parts parts) { final String read = switch (field.type()) { case MESSAGE -> """ - length = c.length(Codec.DEFAULT_MAX_SIZE, "$field"); + length = c.length(maxSize, "$field"); offset = c.position(); c.skip(length); if (child == null) { child = new $elem.View(); } - child.wrap(in, offset, length, depth + 1); + child.wrap(in, offset, length, strictMode, maxDepth - 1, maxSize); """.replace("$elem", field.javaFieldTypeBase()); case STRING, BYTES -> """ - length = c.length(Codec.DEFAULT_MAX_SIZE, "$field"); + length = c.length(maxSize, "$field"); offset = c.position(); c.skip(length); """; @@ -379,7 +463,7 @@ private static void repeated(final SingleField field, final Parts parts) { """.replace("$decode", decode(field.type(), "c")); final String packedTagCase = delimited ? "" : """ if (tag == $packedTag) { - final int packedLength = c.length(Codec.DEFAULT_MAX_SIZE, "$field"); + final int packedLength = c.length(maxSize, "$field"); packedEnd = c.position() + packedLength; continue; } @@ -442,7 +526,7 @@ public boolean next() throws ParseException { return true; } $packedTagCase - c.unknownField(tag, false, false, false, null, Codec.DEFAULT_MAX_SIZE); + c.unknownField(tag, false, false, false, null, maxSize); } } catch (final ParseException e) { throw e; @@ -623,6 +707,7 @@ private static String accessor(final SingleField field, final String name, final """ private $type.View $nameView; + private boolean $nameViewCurrent; /** @return true if {@code $field} is present */ public boolean has$Name() { @@ -630,7 +715,8 @@ private static String accessor(final SingleField field, final String name, final } /** - * @return a view of {@code $field}, or null when absent; the same child view is re-wrapped on each call + * @return a view of {@code $field}, or null when absent; the same child view, wrapped again only when this + * view has been * @throws ParseException if the nested message is malformed */ public $type.View $name() throws ParseException { @@ -639,8 +725,13 @@ private static String accessor(final SingleField field, final String name, final } if ($nameView == null) { $nameView = new $type.View(); + } else if ($nameViewCurrent && $nameView.wraps(in, $nameOffset, $nameLength)) { + // already wrapped over this field since this view was wrapped: don't index it again + return $nameView; } - return $nameView.wrap(in, $nameOffset, $nameLength, depth + 1); + $nameView.wrap(in, $nameOffset, $nameLength, strictMode, maxDepth - 1, maxSize); + $nameViewCurrent = true; + return $nameView; } """.replace("$absent", absent(name, guard)) .replace("$type", field.javaFieldTypeBase()) @@ -658,8 +749,8 @@ private static String accessor(final SingleField field, final String name, final private static String wrapperAccessor(final SingleField field, final String name, final String guard) { final String[] typeReadDefault = switch (field.messageType()) { - case "StringValue" -> new String[] {"String", "cursor.string(Codec.DEFAULT_MAX_SIZE)", "\"\""}; - case "BytesValue" -> new String[] {"Bytes", "cursor.bytes(Codec.DEFAULT_MAX_SIZE)", "Bytes.EMPTY"}; + case "StringValue" -> new String[] {"String", "cursor.string(maxSize)", "\"\""}; + case "BytesValue" -> new String[] {"Bytes", "cursor.bytes(maxSize)", "Bytes.EMPTY"}; case "Int32Value", "UInt32Value" -> new String[] {"Integer", "(int) cursor.varint()", "0"}; case "Int64Value", "UInt64Value" -> new String[] {"Long", "cursor.varint()", "0L"}; case "FloatValue" -> new String[] {"Float", "Float.intBitsToFloat(cursor.fixed32())", "0f"}; diff --git a/pbj-core/pbj-compiler/src/test/java/com/hedera/pbj/compiler/ViewGenerationTest.java b/pbj-core/pbj-compiler/src/test/java/com/hedera/pbj/compiler/ViewGenerationTest.java new file mode 100644 index 000000000..890324b39 --- /dev/null +++ b/pbj-core/pbj-compiler/src/test/java/com/hedera/pbj/compiler/ViewGenerationTest.java @@ -0,0 +1,134 @@ +// SPDX-License-Identifier: Apache-2.0 +package com.hedera.pbj.compiler; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.hedera.pbj.compiler.impl.PbjCompilerException; +import com.hedera.pbj.compiler.impl.PerformanceOptions; +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.Set; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** Which messages get views and array parsers when messages contain each other, and why views are denied. */ +class ViewGenerationTest { + @TempDir + Path directory; + + /** Compile a proto file with every option on, and return what the compiler printed to standard error. */ + private String compile(final String proto) throws Exception { + final Path file = directory.resolve("cycles.proto"); + Files.writeString(file, proto); + final PrintStream err = System.err; + final ByteArrayOutputStream printed = new ByteArrayOutputStream(); + System.setErr(new PrintStream(printed, true, StandardCharsets.UTF_8)); + try { + PbjCompiler.compileFilesIn( + List.of(file.toFile()), + Set.of(), + Set.of(directory.toFile()), + directory.resolve("out").toFile(), + directory.resolve("out").toFile(), + null, + false, + false, + Set.of(), + Set.of(), + PerformanceOptions.of(null, null, null, null, "max", "test project")); + } finally { + System.setErr(err); + } + return printed.toString(StandardCharsets.UTF_8); + } + + private boolean hasView(final String message) throws Exception { + return Files.readString(directory.resolve("out/example/" + message + ".java")) + .contains("public static final class View"); + } + + private String codec(final String message) throws Exception { + return Files.readString(directory.resolve("out/example/codec/" + message + "ProtoCodec.java")); + } + + @Test + void messagesThatContainEachOtherGetViewsAndArrayParsers() throws Exception { + compile(""" + syntax = "proto3"; + package example; + message Self { Self next = 1; int32 value = 2; } + message PairA { PairB b = 1; } + message PairB { PairA a = 1; repeated PairA as = 2; } + message TriX { TriY y = 1; } + message TriY { TriZ z = 1; } + message TriZ { oneof choice { TriX x = 1; string text = 2; } } + """); + for (final String message : List.of("Self", "PairA", "PairB", "TriX", "TriY", "TriZ")) { + assertTrue(hasView(message), message); + assertTrue(codec(message).contains("parseArray("), message); + } + // nested array parsers in the cycles are called directly + assertTrue(codec("Self").contains("SelfProtoCodec.parseArray("), codec("Self")); + assertTrue(codec("PairA").contains("PairBProtoCodec.parseArray("), codec("PairA")); + assertTrue(codec("TriZ").contains("TriXProtoCodec.parseArray("), codec("TriZ")); + } + + @Test + void aCycleThroughAMessageWithoutAViewGetsNoViews() throws Exception { + // CycleB can't have a view because of Counts' map, so neither can CycleA, which contains CycleB, nor CycleC, + // which contains CycleA, whichever message is looked at first + // views requested in the file's comments, so the compiler reports why they are denied + final String printed = compile(""" + syntax = "proto3"; + // <<>> + package example; + message CycleA { CycleB b = 1; } + message CycleB { CycleC c = 1; Counts counts = 2; } + message CycleC { CycleA a = 1; int32 value = 2; } + message Counts { map counts = 1; } + message Fine { int32 value = 1; } + """); + for (final String message : List.of("CycleA", "CycleB", "CycleC", "Counts")) { + assertFalse(hasView(message), message); + } + assertTrue(hasView("Fine")); + // the compiler says why + assertTrue( + printed.contains("no view for message Counts: map field 'counts' (maps are not supported by views)"), + printed); + assertTrue(printed.contains("no view for message CycleB: field 'counts' has type example.Counts"), printed); + assertTrue(printed.contains("no view for message CycleC: field 'a' has type example.CycleA"), printed); + } + + @Test + void viewsDeniedOnlyByTheProjectSettingAreNotReported() throws Exception { + final String printed = compile(""" + syntax = "proto3"; + package example; + message Counts { map counts = 1; } + """); + assertFalse(hasView("Counts")); + assertFalse(printed.contains("no view"), printed); + } + + @Test + void aNestedTypeNamedViewIsAClearError() { + final Exception e = assertThrows(Exception.class, () -> compile(""" + syntax = "proto3"; + package example; + message Clash { + message View { int32 value = 1; } + View view = 1; + } + """)); + final Throwable cause = e instanceof PbjCompilerException ? e : e.getCause(); + assertTrue(cause instanceof PbjCompilerException, e.toString()); + assertTrue(cause.getMessage().contains("declares its own nested type named View"), cause.getMessage()); + } +} diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/ArrayCursor.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/ArrayCursor.java index 134c5d87b..c8fdcdab3 100644 --- a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/ArrayCursor.java +++ b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/ArrayCursor.java @@ -131,46 +131,6 @@ public byte[] array() { return in; } - /** - * Read an unsigned varint at an index already validated by a cursor, without bounds checks beyond the array's. - * - * @param in the bytes - * @param offset the varint's first byte - * @return the value - */ - public static long varintAt(@NonNull final byte[] in, final int offset) { - long value = 0; - for (int shift = 0, i = offset; ; shift += 7) { - final byte b = in[i++]; - value |= (long) (b & 0x7F) << shift; - if (b >= 0) { - return value; - } - } - } - - /** - * Read a little-endian 32-bit value at an index. - * - * @param in the bytes - * @param q the value's first byte - * @return the value - */ - public static int fixed32At(@NonNull final byte[] in, final int q) { - return (in[q] & 0xFF) | (in[q + 1] & 0xFF) << 8 | (in[q + 2] & 0xFF) << 16 | (in[q + 3] & 0xFF) << 24; - } - - /** - * Read a little-endian 64-bit value at an index. - * - * @param in the bytes - * @param q the value's first byte - * @return the value - */ - public static long fixed64At(@NonNull final byte[] in, final int q) { - return (fixed32At(in, q) & 0xFFFFFFFFL) | ((long) fixed32At(in, q + 4)) << 32; - } - /** @return the number of bytes read since this cursor was created */ public int consumed() { return p - start; diff --git a/pbj-integration-tests/src/main/proto/viewChecks.proto b/pbj-integration-tests/src/main/proto/viewChecks.proto new file mode 100644 index 000000000..fe01f838f --- /dev/null +++ b/pbj-integration-tests/src/main/proto/viewChecks.proto @@ -0,0 +1,55 @@ +// SPDX-License-Identifier: Apache-2.0 +syntax = "proto3"; + +package proto; + +option java_package = "com.hedera.pbj.test.proto.java"; +option java_multiple_files = true; + +/** + * Messages for ViewValidationTest: views validate as parsing does, with the caller's limits and strict mode, and are + * denied to messages that can't have them. + */ + +// <<>> +message ViewCheckOuter { + ViewCheckInner inner = 1; + oneof choice { + ViewCheckInner inner_choice = 2; + string text_choice = 3; + } + repeated sint64 numbers = 4; + repeated string names = 5; + string name = 6; + repeated ViewCheckInner inners = 7; +} + +// <<>> +message ViewCheckInner { + int32 value = 1; + ViewCheckLeaf leaf = 2; +} + +// <<>> +message ViewCheckLeaf { + int32 value = 1; +} + +/** Asks for a view, but maps are not supported by views. */ +// <<>> +message ViewCheckWithMap { + map counts = 1; +} + +/** Turns views off. */ +// <<>> +message ViewCheckOptedOut { + int32 value = 1; +} + +/** Asks for a view, but contains a message that has none. */ +// <<>> +message ViewCheckDenied { + ViewCheckOptedOut opted_out = 1; + int32 value = 2; +} diff --git a/pbj-integration-tests/src/test/java/com/hedera/pbj/integration/test/ViewValidationTest.java b/pbj-integration-tests/src/test/java/com/hedera/pbj/integration/test/ViewValidationTest.java new file mode 100644 index 000000000..71da347cb --- /dev/null +++ b/pbj-integration-tests/src/test/java/com/hedera/pbj/integration/test/ViewValidationTest.java @@ -0,0 +1,239 @@ +// SPDX-License-Identifier: Apache-2.0 +package com.hedera.pbj.integration.test; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.hedera.pbj.runtime.Codec; +import com.hedera.pbj.runtime.ParseException; +import com.hedera.pbj.runtime.io.buffer.Bytes; +import com.hedera.pbj.test.proto.java.pbj.integration.tests.ViewCheckDenied; +import com.hedera.pbj.test.proto.java.pbj.integration.tests.ViewCheckInner; +import com.hedera.pbj.test.proto.java.pbj.integration.tests.ViewCheckLeaf; +import com.hedera.pbj.test.proto.java.pbj.integration.tests.ViewCheckOuter; +import com.hedera.pbj.test.proto.java.pbj.integration.tests.ViewCheckWithMap; +import java.io.ByteArrayOutputStream; +import java.util.ArrayList; +import java.util.List; +import org.junit.jupiter.api.Test; + +/** Views validate as parsing does, with the caller's limits and strict mode, and reuse child views safely. */ +final class ViewValidationTest { + private static final int VARINT = 0; + private static final int DELIMITED = 2; + + /** Encoded fields, built by hand to make malformed or unusual messages. */ + private static final class Fields { + private final ByteArrayOutputStream out = new ByteArrayOutputStream(); + + Fields varint(final long value) { + long v = value; + while ((v & ~0x7FL) != 0) { + out.write((int) (v & 0x7F) | 0x80); + v >>>= 7; + } + out.write((int) v); + return this; + } + + Fields tag(final int field, final int wireType) { + return varint(((long) field << 3) | wireType); + } + + Fields delimited(final int field, final byte[] value) { + tag(field, DELIMITED).varint(value.length); + out.writeBytes(value); + return this; + } + + Fields delimited(final int field, final Bytes value) { + return delimited(field, value.toByteArray()); + } + + Bytes bytes() { + return Bytes.wrap(out.toByteArray()); + } + } + + private static Bytes inner(final int value) { + return ViewCheckInner.PROTOBUF.toBytes(ViewCheckInner.newBuilder() + .value(value) + .leaf(new ViewCheckLeaf(value * 10)) + .build()); + } + + @Test + void strictModeRejectsUnknownFields() throws Exception { + final Bytes bytes = new Fields().tag(1000, VARINT).varint(1).bytes(); + assertEquals(ViewCheckOuter.DEFAULT, ViewCheckOuter.PROTOBUF.parse(bytes)); + new ViewCheckOuter.View().wrap(bytes); + assertThrows(ParseException.class, () -> ViewCheckOuter.PROTOBUF.parseStrict(bytes)); + assertThrows(ParseException.class, () -> new ViewCheckOuter.View() + .wrap(bytes, true, Codec.DEFAULT_MAX_DEPTH, Codec.DEFAULT_MAX_SIZE)); + } + + @Test + void strictModeRejectsARepeatedSingularMessage() throws Exception { + final Bytes twice = + new Fields().delimited(1, inner(1)).delimited(1, inner(2)).bytes(); + // not strict: the last one wins, as in parsing + final ViewCheckOuter.View view = new ViewCheckOuter.View().wrap(twice); + assertEquals( + ViewCheckOuter.PROTOBUF.parse(twice).inner().value(), + view.inner().value()); + assertEquals(2, view.inner().value()); + assertThrows(ParseException.class, () -> ViewCheckOuter.PROTOBUF.parseStrict(twice)); + assertThrows(ParseException.class, () -> new ViewCheckOuter.View() + .wrap(twice, true, Codec.DEFAULT_MAX_DEPTH, Codec.DEFAULT_MAX_SIZE)); + + // a oneof whose alternative is a message, after another alternative + final Bytes oneOfTwice = new Fields() + .delimited(3, "text".getBytes()) + .delimited(2, inner(3)) + .bytes(); + assertEquals(3, new ViewCheckOuter.View().wrap(oneOfTwice).innerChoice().value()); + assertThrows(ParseException.class, () -> ViewCheckOuter.PROTOBUF.parseStrict(oneOfTwice)); + assertThrows(ParseException.class, () -> new ViewCheckOuter.View() + .wrap(oneOfTwice, true, Codec.DEFAULT_MAX_DEPTH, Codec.DEFAULT_MAX_SIZE)); + } + + @Test + void maxSizeLimitsFieldsAndRepeatedElements() throws Exception { + // a 100-byte ViewCheckInner: its value field 50 times + final Fields hundredBytes = new Fields(); + for (int i = 0; i < 50; i++) { + hundredBytes.tag(1, VARINT).varint(1); + } + final Bytes longName = new Fields().delimited(6, new byte[100]).bytes(); + final Bytes longString = new Fields().delimited(5, new byte[100]).bytes(); + final Bytes longMessage = + new Fields().delimited(7, hundredBytes.bytes()).bytes(); + for (final Bytes bytes : List.of(longName, longString, longMessage)) { + ViewCheckOuter.PROTOBUF.parse(bytes.toReadableSequentialData(), false, false, Codec.DEFAULT_MAX_DEPTH, 200); + new ViewCheckOuter.View().wrap(bytes, false, Codec.DEFAULT_MAX_DEPTH, 200); + assertThrows( + ParseException.class, + () -> ViewCheckOuter.PROTOBUF.parse( + bytes.toReadableSequentialData(), false, false, Codec.DEFAULT_MAX_DEPTH, 50)); + assertThrows(ParseException.class, () -> new ViewCheckOuter.View() + .wrap(bytes, false, Codec.DEFAULT_MAX_DEPTH, 50)); + } + } + + @Test + void maxDepthLimitsNestedViews() throws Exception { + final Bytes bytes = new Fields().delimited(1, inner(4)).bytes(); + ViewCheckOuter.PROTOBUF.parse(bytes.toReadableSequentialData(), false, false, 2, Codec.DEFAULT_MAX_SIZE); + assertThrows( + ParseException.class, + () -> ViewCheckOuter.PROTOBUF.parse( + bytes.toReadableSequentialData(), false, false, 1, Codec.DEFAULT_MAX_SIZE)); + assertEquals( + 40, + new ViewCheckOuter.View() + .wrap(bytes, false, 2, Codec.DEFAULT_MAX_SIZE) + .inner() + .leaf() + .value()); + // nested messages are validated when they are read + final ViewCheckInner.View inner = new ViewCheckOuter.View() + .wrap(bytes, false, 1, Codec.DEFAULT_MAX_SIZE) + .inner(); + assertThrows(ParseException.class, inner::leaf); + } + + @Test + void packedAndUnpackedElementsMixInOneRepeatedField() throws Exception { + // packed [1, -2], then 3 unpacked, then packed [-4], then 5 unpacked + final Bytes bytes = new Fields() + .delimited(4, new Fields().varint(2).varint(3).bytes()) + .tag(4, VARINT) + .varint(6) + .delimited(4, new Fields().varint(7).bytes()) + .tag(4, VARINT) + .varint(10) + .bytes(); + assertEquals( + List.of(1L, -2L, 3L, -4L, 5L), + ViewCheckOuter.PROTOBUF.parse(bytes).numbers()); + final ViewCheckOuter.View view = new ViewCheckOuter.View().wrap(bytes); + assertEquals(5, view.numbersCount()); + final List numbers = new ArrayList<>(); + for (final var it = view.numbers(); it.next(); ) { + numbers.add(it.value()); + } + assertEquals(List.of(1L, -2L, 3L, -4L, 5L), numbers); + } + + @Test + void malformedUtf8InRepeatedStringsIsRejected() { + final Bytes bytes = new Fields() + .delimited(5, "fine".getBytes()) + .delimited(5, new byte[] {(byte) 0xFF}) + .bytes(); + assertThrows(ParseException.class, () -> ViewCheckOuter.PROTOBUF.parse(bytes)); + assertThrows(ParseException.class, () -> new ViewCheckOuter.View().wrap(bytes)); + } + + @Test + void nestedViewsAreIndexedOnceAndFollowTheirParent() throws Exception { + final byte[] buffer = new Fields().delimited(1, inner(5)).bytes().toByteArray(); + final ViewCheckOuter.View view = new ViewCheckOuter.View().wrap(buffer, 0, buffer.length); + final ViewCheckInner.View first = view.inner(); + assertSame(first, view.inner()); + assertEquals(5, view.inner().value()); + + // the caller re-wraps the child view: the parent wraps it again over its own field + first.wrap(inner(6)); + assertEquals(6, first.value()); + assertEquals(5, view.inner().value()); + + // the same buffer, reused for another message of the same layout: re-wrapping the parent indexes the child + // again + final byte[] other = new Fields().delimited(1, inner(7)).bytes().toByteArray(); + assertEquals(buffer.length, other.length); + System.arraycopy(other, 0, buffer, 0, buffer.length); + view.wrap(buffer, 0, buffer.length); + assertEquals(7, view.inner().value()); + assertEquals(70, view.inner().leaf().value()); + } + + @Test + void messagesThatCannotHaveViewsGetNone() { + for (final Class model : List.of(ViewCheckWithMap.class, ViewCheckDenied.class)) { + assertFalse( + List.of(model.getDeclaredClasses()).stream() + .anyMatch(c -> c.getSimpleName().equals("View")), + model.getName()); + } + assertTrue(List.of(ViewCheckOuter.class.getDeclaredClasses()).stream() + .anyMatch(c -> c.getSimpleName().equals("View"))); + } + + @Test + void aViewRoundTripsToTheModel() throws Exception { + final ViewCheckOuter outer = ViewCheckOuter.newBuilder() + .inner(new ViewCheckInner(1, new ViewCheckLeaf(2))) + .textChoice("text") + .numbers(List.of(3L, -4L)) + .names(List.of("a", "é")) + .name("name") + .inners(List.of(new ViewCheckInner(5, null), ViewCheckInner.DEFAULT)) + .build(); + final Bytes bytes = ViewCheckOuter.PROTOBUF.toBytes(outer); + final ViewCheckOuter.View view = new ViewCheckOuter.View().wrap(bytes); + assertEquals(outer, view.toModel()); + assertEquals("text", view.textChoice()); + assertNotNull(view.inner()); + final List names = new ArrayList<>(); + for (final var it = view.names(); it.next(); ) { + names.add(it.value()); + } + assertArrayEquals(new String[] {"a", "é"}, names.toArray()); + } +} From cf92ac54de267f04574695def0627c8afe48c041 Mon Sep 17 00:00:00 2001 From: Jasper Potts <1466205+jasperpotts@users.noreply.github.com> Date: Fri, 30 Oct 2026 08:47:04 -0700 Subject: [PATCH 3/4] docs: views of repeated fields, oneofs and wrapper types, and wrap with limits Co-Authored-By: Claude Opus 5.5 Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com> --- docs/codec-protobuf.md | 42 +++++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/docs/codec-protobuf.md b/docs/codec-protobuf.md index e45cf9aca..484216a2a 100644 --- a/docs/codec-protobuf.md +++ b/docs/codec-protobuf.md @@ -276,30 +276,46 @@ in the integration tests compares both writers on HAPI block items. ### Zero-Copy Views (`pbj.views`) The model gets a nested, reusable `View` class that reads fields in place from encoded bytes instead of parsing them -into objects: +into objects. [usage-guide.md](usage-guide.md#zero-copy-views) shows how to use one and the reuse pitfall. ```java final Order.View view = new Order.View(); for (Bytes bytes : orders) { - view.wrap(bytes); // or wrap(byte[], offset, length) + view.wrap(bytes); // or wrap(bytes, strictMode, maxDepth, maxSize), as for parse() total += view.amount(); // numbers and enums are decoded once, by wrap if (view.hasCustomer()) { names.add(view.customer().name()); // a child view, re-wrapped with the parent } + for (var line = view.lines(); line.next(); ) { + count += line.value().quantity(); + } } ``` `wrap` indexes the message in one pass, with the same in-order fast path and the same checks of tags, wire types, -lengths, bools and UTF-8 as parsing; a malformed message throws `ParseException`. Numbers are decoded into fields of -the view, and strings, bytes and nested messages are recorded as offsets and lengths. String and bytes fields also have -`xxxArray()`, `xxxOffset()`, `xxxLength()` and `xxxEquals(byte[])`, which read them without allocating; `xxx()` builds a -`String` or `Bytes`. An enum field's `xxxProtoOrdinal()` returns the raw number. `toModel()` parses the wrapped bytes -into a model object. Wrapping and reading numbers, enums, nested views and offsets allocate nothing. - -A view and its child views are reused, so a child view returned earlier reads whatever its parent was last wrapped -over. The wrapped bytes must not change while a view reads them. +lengths, bools and UTF-8 as parsing; a malformed message throws `ParseException`. `wrap(bytes, strictMode, maxDepth, +maxSize)` applies the same limits and strict mode as `parse` with those arguments, and passes them down to nested +views. Nested messages, repeated message elements and wrapper type values are validated when they are read, so a +malformed one throws from its accessor or cursor rather than from `wrap`. -A message gets a view only if every field is a singular scalar, string, bytes, enum or message, and every message -field's type also gets a view. Messages with repeated fields, maps, oneofs or `google.protobuf` wrapper types get no -view. +What a view has for each kind of field: +| Field | Accessors | +|---|---| +| Number, bool | `xxx()`, decoded by `wrap` | +| Enum | `xxx()`, and `xxxProtoOrdinal()` for the raw number | +| String, bytes | `xxx()` builds a `String` or `Bytes`; `xxxArray()`, `xxxOffset()`, `xxxLength()` and `xxxEquals(byte[])` read it without allocating | +| Message | `xxx()` returns a child view, created once and re-wrapped with the parent | +| `google.protobuf` wrapper type | `xxx()` returns the value, or null when absent | +| Repeated | `xxxCount()`, and `xxx()` returns a reusable cursor: `next()` moves to each element, `value()` reads it (and `valueOffset()`, `valueLength()`, `valueEquals(byte[])` for strings and bytes) | +| Oneof | `xxxKind()` returns which alternative is set; each alternative has the accessors of a singular field | + +Message and wrapper type fields also have `hasXxx()`. `toModel()` parses the wrapped bytes into a model object. Wrapping +and reading numbers, enums, nested views, cursors and offsets allocate nothing. + +A view and its child views and cursors are reused, so a child view returned earlier reads whatever its parent was +last wrapped over. The wrapped bytes must not change while a view reads them. + +A message gets a view only if it has no maps or repeated wrapper types, and every message field's type also gets a +view (messages may contain each other). When a message asks for a view in its own or its file's comments and gets +none, the build prints why. A message with a view that declares its own nested message or enum named `View` fails the build. From 1b1ed9e4e750ac7be08df4f095a727ab4d503e9b Mon Sep 17 00:00:00 2001 From: Jasper Potts <1466205+jasperpotts@users.noreply.github.com> Date: Fri, 30 Oct 2026 12:19:35 -0700 Subject: [PATCH 4/4] fix: views validate nested messages in strict mode and never hand out the wrapped array Strict mode is fail fast, but a strict wrap only validated the top-level message: unknown fields, depth and UTF-8 inside nested messages, repeated message elements and wrapper values were only checked if they were read, so a strict wrap accepted input parseStrict rejects. In strict mode wrap now validates all of them, at any depth, before returning. Non-strict views stay lazy, and the docs now say plainly that a non-strict wrap does not validate untrusted input. Bytes accessors returned Bytes.wrap over the wrapped array, and xxxArray() returned the array itself, so a view over a plain array handed out bytes that changed when the caller reused it, and a view over Bytes exposed the Bytes' mutable backing array. Views now remember the Bytes they wrap: bytes fields, wrapper bytes and toModel() share it, or copy from a plain array, as the array parser does. xxxArray(), xxxOffset() and valueOffset() are replaced by xxxCopyTo() and valueCopyTo(), which copy without allocating. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com> --- docs/codec-protobuf.md | 14 +- docs/usage-guide.md | 7 +- .../impl/generators/ViewGenerator.java | 138 +++++++++++++---- .../pbj/runtime/io/buffer/ArrayCursor.java | 59 +++++++ .../src/main/proto/viewChecks.proto | 5 + .../integration/test/ViewValidationTest.java | 145 ++++++++++++++++++ 6 files changed, 329 insertions(+), 39 deletions(-) diff --git a/docs/codec-protobuf.md b/docs/codec-protobuf.md index e54f72263..46f1fab34 100644 --- a/docs/codec-protobuf.md +++ b/docs/codec-protobuf.md @@ -304,8 +304,10 @@ for (Bytes bytes : orders) { `wrap` indexes the message in one pass, with the same in-order fast path and the same checks of tags, wire types, lengths, bools and UTF-8 as parsing; a malformed message throws `ParseException`. `wrap(bytes, strictMode, maxDepth, maxSize)` applies the same limits and strict mode as `parse` with those arguments, and passes them down to nested -views. Nested messages, repeated message elements and wrapper type values are validated when they are read, so a -malformed one throws from its accessor or cursor rather than from `wrap`. +views. In strict mode `wrap` also validates every nested message, repeated message element and wrapper type value, at +any depth, so it fails fast on whatever `parseStrict` rejects. Otherwise they are validated when they are read, so a +malformed one throws from its accessor or cursor rather than from `wrap`, and one that is never read is never +validated: **a non-strict `wrap` is not a validation of untrusted input**; use strict mode or `parse` for that. What a view has for each kind of field: @@ -313,17 +315,19 @@ What a view has for each kind of field: |---|---| | Number, bool | `xxx()`, decoded by `wrap` | | Enum | `xxx()`, and `xxxProtoOrdinal()` for the raw number | -| String, bytes | `xxx()` builds a `String` or `Bytes`; `xxxArray()`, `xxxOffset()`, `xxxLength()` and `xxxEquals(byte[])` read it without allocating | +| String, bytes | `xxx()` builds a `String` or `Bytes` (a view of the wrapped `Bytes`, or a copy when wrapping an array); `xxxLength()`, `xxxEquals(byte[])` and `xxxCopyTo(byte[], int)` read it without allocating | | Message | `xxx()` returns a child view, created once and re-wrapped with the parent | | `google.protobuf` wrapper type | `xxx()` returns the value, or null when absent | -| Repeated | `xxxCount()`, and `xxx()` returns a reusable cursor: `next()` moves to each element, `value()` reads it (and `valueOffset()`, `valueLength()`, `valueEquals(byte[])` for strings and bytes) | +| Repeated | `xxxCount()`, and `xxx()` returns a reusable cursor: `next()` moves to each element, `value()` reads it (and `valueLength()`, `valueEquals(byte[])`, `valueCopyTo(byte[], int)` for strings and bytes) | | Oneof | `xxxKind()` returns which alternative is set; each alternative has the accessors of a singular field | Message and wrapper type fields also have `hasXxx()`. `toModel()` parses the wrapped bytes into a model object. Wrapping and reading numbers, enums, nested views, cursors and offsets allocate nothing. A view and its child views and cursors are reused, so a child view returned earlier reads whatever its parent was -last wrapped over. The wrapped bytes must not change while a view reads them. +last wrapped over. The wrapped bytes must not change while a view reads them. Views never hand out the wrapped array: +`Bytes` they return, and `toModel()`, share the wrapped `Bytes`, which is immutable, or copy from a wrapped array, so +they stay the same when a caller reuses its array. A message gets a view only if it has no maps or repeated wrapper types, and every message field's type also gets a view (messages may contain each other). When a message asks for a view in its own or its file's comments and gets diff --git a/docs/usage-guide.md b/docs/usage-guide.md index fc8eb32d8..3b9050de7 100644 --- a/docs/usage-guide.md +++ b/docs/usage-guide.md @@ -389,9 +389,10 @@ for (Bytes bytes : orders) { } ``` -`wrap` validates the message as `parse` does, with the same limits and strict mode. Nested messages, repeated message -elements and wrapper type values are validated when they are read, so a malformed one throws from its accessor or -cursor rather than from `wrap`. Messages with maps, or that contain a message without a view, get no view; the build +`wrap` validates the message as `parse` does, with the same limits and strict mode. In strict mode it validates +nested messages, repeated message elements and wrapper type values too, at any depth, before returning. Otherwise they +are validated when they are read, so a malformed one throws from its accessor or cursor rather than from `wrap`, and +one that is never read is never validated: to validate untrusted input, wrap it in strict mode or parse it. Messages with maps, or that contain a message without a view, get no view; the build says why for messages that ask for one in their comments. **Views and cursors are reused, so what they return changes under you.** A nested accessor returns the same child view diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ViewGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ViewGenerator.java index 04b841629..ce39eb7ed 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ViewGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ViewGenerator.java @@ -34,7 +34,8 @@ private record Parts( StringBuilder resets, StringBuilder fastPath, StringBuilder cases, - StringBuilder accessors) {} + StringBuilder accessors, + StringBuilder validations) {} /** * Generate the view class for a message, or nothing if it has no view. @@ -75,6 +76,7 @@ static String generateView( new StringBuilder(), new StringBuilder(), new StringBuilder(), + new StringBuilder(), new StringBuilder()); for (final Field field : fields) { if (field instanceof OneOfField oneOf) { @@ -96,17 +98,23 @@ static String generateView( * *

{@link #wrap} indexes this message and validates it as parsing does, with the same limits and strict * mode: tags, wire types, lengths, bools, UTF-8 strings, unknown fields and, in strict mode, repeated - * singular messages. Nested messages, repeated message elements and wrapper type values are validated - * when they are read, so a malformed one throws from its accessor or cursor rather than from {@code wrap}. + * singular messages. In strict mode it also validates every nested message, repeated message element and + * wrapper type value, at any depth, so it fails fast on anything strict parsing rejects. Otherwise they + * are only validated when they are read, and a malformed one throws from its accessor or cursor: a + * non-strict {@code wrap} does not validate nested messages that are never read, so use strict mode, or + * parse, to validate untrusted input. * *

A view is valid until it is wrapped again, and reads the wrapped bytes directly, so they must not - * change. Child views and cursors are reused: the view a nested accessor or a cursor's {@code value()} - * returns changes on the next call, so copy what you need, or call {@code toModel()}, before reading - * further. Not thread safe. + * change while it is used. Bytes it returns are views of the wrapped {@link Bytes}, which is immutable, + * or copies when it wraps a plain array. Child views and cursors are reused: the view a nested accessor + * or a cursor's {@code value()} returns changes on the next call, so copy what you need, or call + * {@code toModel()}, before reading further. Not thread safe. */ public static final class View { private final ArrayCursor cursor = new ArrayCursor(new byte[0], 0, 0); private byte[] in; + /** The Bytes holding {@code in}, if this view wraps one, so bytes fields need not be copied */ + private Bytes source; private int start; private int end; private boolean strictMode; @@ -157,8 +165,34 @@ public View wrap(@NonNull final byte[] in, final int offset, final int length) t /** * Wrap an encoded $model in {@code in[offset, offset + length)}, validating it as {@link Codec#parse} - * does with the same arguments. Views also use it for the messages they contain. + * does with the same arguments. Bytes fields are returned as copies, since the array could change. + * + * @param in the bytes + * @param offset where the message starts + * @param length the message's length + * @param strictMode when {@code true}, unknown fields and repeated singular messages are errors + * @param maxDepth the maximum depth of nested messages below this one + * @param maxSize the maximum length of a field, nested message or unknown field + * @return this view + * @throws ParseException if the message is malformed or nested too deep + */ + public View wrap( + @NonNull final byte[] in, + final int offset, + final int length, + final boolean strictMode, + final int maxDepth, + final int maxSize) + throws ParseException { + return wrap(null, in, offset, length, strictMode, maxDepth, maxSize); + } + + /** + * Wrap an encoded $model in {@code in[offset, offset + length)}, part of {@code source} if it is not + * null, validating it as {@link Codec#parse} does with the same arguments. Views use it for the + * messages they contain, so bytes fields are views of {@code source} rather than copies. * + * @param source the {@link Bytes} holding the message, or null if it is part of a plain array * @param in the bytes * @param offset where the message starts * @param length the message's length @@ -167,8 +201,10 @@ public View wrap(@NonNull final byte[] in, final int offset, final int length) t * @param maxSize the maximum length of a field, nested message or unknown field * @return this view * @throws ParseException if the message is malformed or nested too deep + * @throws IllegalArgumentException if {@code source} does not hold the message */ public View wrap( + @Nullable final Bytes source, @NonNull final byte[] in, final int offset, final int length, @@ -176,7 +212,7 @@ public View wrap( final int maxDepth, final int maxSize) throws ParseException { - cursor.reset(in, offset, offset + length); + cursor.reset(source, in, offset, offset + length); return index(strictMode, maxDepth, maxSize); } @@ -225,10 +261,24 @@ private View index(final boolean strictMode, final int maxDepth, final int maxSi } this.start = start; end = cursor.limit(); + source = cursor.source(); in = cursor.array(); + if (strictMode) { + // strict mode fails fast: validate nested messages now, not when they are read + try { + validateNested(); + } catch (final ParseException e) { + in = null; + throw e; + } + } return this; } + /** Validates every nested message, repeated message element and wrapper value, at any depth. */ + private void validateNested() throws ParseException { + $validations } + /** * Parse the wrapped message into a model object. * @@ -236,7 +286,7 @@ private View index(final boolean strictMode, final int maxDepth, final int maxSi * @throws ParseException if the message is malformed */ public $model toModel() throws ParseException { - return $model.PROTOBUF.parse(Bytes.wrap(in, start, end - start)); + return $model.PROTOBUF.parse(ArrayCursor.bytes(source, in, start, end - start)); } $accessors } @@ -246,6 +296,7 @@ private View index(final boolean strictMode, final int maxDepth, final int maxSi .replace("$fastPath", parts.fastPath().toString().indent(DEFAULT_INDENT * 3)) .replace("$cases", parts.cases().toString().indent(DEFAULT_INDENT * 5)) .replace("$accessors", parts.accessors().toString().indent(DEFAULT_INDENT)) + .replace("$validations", parts.validations().toString().indent(DEFAULT_INDENT * 2)) .replace("$model", modelClass) .replace("$schema", schemaClass) .indent(DEFAULT_INDENT); @@ -337,6 +388,10 @@ record += "%s = %d;%n".formatted(oneOfKind, field.fieldNumber()); final String guard = oneOfKind == null ? "" : "%s == %d".formatted(oneOfKind, field.fieldNumber()); parts.accessors() .append(field.optionalValueType() ? wrapperAccessor(field, name, guard) : accessor(field, name, guard)); + if (field.type() == Field.FieldType.MESSAGE) { + // reading a nested message or wrapper value validates it; absent ones are skipped by the accessor + parts.validations().append("%s();%n".formatted(name)); + } } /** A oneof: each alternative is recorded like a singular field, and the oneof remembers the last one seen. */ @@ -443,7 +498,7 @@ private static void repeated(final SingleField field, final Parts parts) { if (child == null) { child = new $elem.View(); } - child.wrap(in, offset, length, strictMode, maxDepth - 1, maxSize); + child.wrap(source, in, offset, length, strictMode, maxDepth - 1, maxSize); """.replace("$elem", field.javaFieldTypeBase()); case STRING, BYTES -> """ length = c.length(maxSize, "$field"); @@ -452,6 +507,12 @@ private static void repeated(final SingleField field, final Parts parts) { """; default -> "value = %s;%n".formatted(decode(field.type(), "c")); }; + if (field.type() == Field.FieldType.MESSAGE) { + // moving the cursor to each element wraps, and so validates, it + parts.validations() + .append("for (final var elements = %s(); elements.next(); ) {%n // validated by next()%n}%n" + .formatted(name)); + } final String packedStep = delimited ? "" : """ if (packedEnd >= 0) { if (c.position() < packedEnd) { @@ -566,16 +627,23 @@ private static String elementAccessors(final SingleField field) { case STRING, BYTES -> """ - /** @return the offset of the current element in the wrapped array */ - public int valueOffset() { - return offset; - } - /** @return the length in bytes of the current element */ public int valueLength() { return length; } + /** + * Copy the current element's bytes, without allocating. + * + * @param dst the array to copy to + * @param dstOffset where in {@code dst} to copy to + * @return the number of bytes copied, {@link #valueLength()} + */ + public int valueCopyTo(@NonNull final byte[] dst, final int dstOffset) { + System.arraycopy(in, offset, dst, dstOffset, length); + return length; + } + /** * @param other bytes to compare with * @return true if the current element is exactly {@code other}, compared without allocating @@ -592,13 +660,13 @@ public boolean valueEquals(@NonNull final byte[] other) { "$returns", field.type() == Field.FieldType.STRING ? "decoded as a new String" - : "as a view of the wrapped bytes") + : "as a view of the wrapped Bytes, or a copy when wrapping an array") .replace("$type", field.type() == Field.FieldType.STRING ? "String" : "Bytes") .replace( "$value", field.type() == Field.FieldType.STRING ? "new String(in, offset, length, java.nio.charset.StandardCharsets.UTF_8)" - : "Bytes.wrap(in, offset, length)"); + : "ArrayCursor.bytes(source, in, offset, length)"); case MESSAGE -> """ /** @return a view of the current element, re-wrapped by each {@code next()} */ @@ -660,19 +728,24 @@ private static String accessor(final SingleField field, final String name, final case STRING, BYTES -> """ - /** @return the offset of {@code $field} in {@link #$nameArray()}, 0 when absent */ - public int $nameOffset() { - return $absent ? 0 : $nameOffset; - } - /** @return the length in bytes of {@code $field}, 0 when absent */ public int $nameLength() { return $absent ? 0 : $nameLength; } - /** @return the wrapped array, which holds {@code $field}; do not modify it */ - public byte[] $nameArray() { - return in; + /** + * Copy the bytes of {@code $field}, without allocating. + * + * @param dst the array to copy to + * @param dstOffset where in {@code dst} to copy to + * @return the number of bytes copied, {@link #$nameLength()} + */ + public int $nameCopyTo(@NonNull final byte[] dst, final int dstOffset) { + final int length = $nameLength(); + if (length > 0) { + System.arraycopy(in, $nameOffset, dst, dstOffset, length); + } + return length; } /** @@ -680,8 +753,11 @@ private static String accessor(final SingleField field, final String name, final * @return true if {@code $field} is exactly {@code other}, compared without allocating */ public boolean $nameEquals(@NonNull final byte[] other) { - return java.util.Arrays.equals( - in, $nameOffset(), $nameOffset() + $nameLength(), other, 0, other.length); + final int length = $nameLength(); + return length == 0 + ? other.length == 0 + : java.util.Arrays.equals( + in, $nameOffset, $nameOffset + length, other, 0, other.length); } /** @return {@code $field}, $returns */ @@ -693,14 +769,14 @@ private static String accessor(final SingleField field, final String name, final "$returns", field.type() == Field.FieldType.STRING ? "decoded as a new String" - : "as a view of the wrapped bytes") + : "as a view of the wrapped Bytes, or a copy when wrapping an array") .replace("$type", field.type() == Field.FieldType.STRING ? "String" : "Bytes") .replace("$default", field.type() == Field.FieldType.STRING ? "\"\"" : "Bytes.EMPTY") .replace( "$value", field.type() == Field.FieldType.STRING ? "new String(in, $nameOffset, $nameLength, java.nio.charset.StandardCharsets.UTF_8)" - : "Bytes.wrap(in, $nameOffset, $nameLength)") + : "ArrayCursor.bytes(source, in, $nameOffset, $nameLength)") .replace("$name", name) .replace("$field", field.name()); case MESSAGE -> @@ -729,7 +805,7 @@ private static String accessor(final SingleField field, final String name, final // already wrapped over this field since this view was wrapped: don't index it again return $nameView; } - $nameView.wrap(in, $nameOffset, $nameLength, strictMode, maxDepth - 1, maxSize); + $nameView.wrap(source, in, $nameOffset, $nameLength, strictMode, maxDepth - 1, maxSize); $nameViewCurrent = true; return $nameView; } @@ -777,7 +853,7 @@ private static String wrapperAccessor(final SingleField field, final String name return $default; } try { - cursor.reset(in, $nameOffset, $nameOffset + $nameLength); + cursor.reset(source, in, $nameOffset, $nameOffset + $nameLength); // the wrapper's one field: its tag, then the value cursor.tag(); return $read; diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/ArrayCursor.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/ArrayCursor.java index c8fdcdab3..3cd29ccd8 100644 --- a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/ArrayCursor.java +++ b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/ArrayCursor.java @@ -122,6 +122,65 @@ public ArrayCursor reset(@NonNull final Bytes bytes) { return this; } + /** + * Point this cursor at {@code in[offset, limit)}, part of {@code source} if it is not null, so {@link #bytes} + * returns views of {@code source} instead of copies. Views use it for the messages they contain. + * + * @param source the {@link Bytes} holding {@code in[offset, limit)}, or null if they are a plain array + * @param in the bytes + * @param offset index of the first byte to read + * @param limit index after the last byte to read + * @return this cursor + * @throws IllegalArgumentException if {@code source} does not hold {@code in[offset, limit)} + */ + public ArrayCursor reset( + @Nullable final Bytes source, @NonNull final byte[] in, final int offset, final int limit) { + reset(in, offset, limit); + if (source != null) { + checkHolds(source, in, offset, limit - offset); + this.source = source; + } + return this; + } + + /** @return the {@link Bytes} this cursor reads, or null if it reads a plain array */ + public @Nullable Bytes source() { + return source; + } + + /** + * The bytes {@code in[offset, offset + length)}, as the stream parser returns them: a view of {@code source}, which + * is immutable, or a copy if they are part of a plain array, which could change. + * + * @param source the {@link Bytes} holding the bytes, or null if they are part of a plain array + * @param in the array holding the bytes + * @param offset index of the first byte + * @param length the number of bytes + * @return the bytes + * @throws IllegalArgumentException if {@code source} does not hold the bytes + */ + public static @NonNull Bytes bytes( + @Nullable final Bytes source, @NonNull final byte[] in, final int offset, final int length) { + if (length == 0) { + return Bytes.EMPTY; + } + if (source != null) { + checkHolds(source, in, offset, length); + return source.getBytes(offset - source.backingOffset(), length); + } + return Bytes.wrap(Arrays.copyOfRange(in, offset, offset + length)); + } + + private static void checkHolds( + @NonNull final Bytes source, @NonNull final byte[] in, final int offset, final int length) { + if (source.backingArray() != in + || offset < source.backingOffset() + || length < 0 + || offset + length > source.backingOffset() + source.length()) { + throw new IllegalArgumentException("The bytes are not part of the given source"); + } + } + /** * The array this cursor reads. Views use it to decode fields at the offsets they recorded; it must not be modified. * diff --git a/pbj-integration-tests/src/main/proto/viewChecks.proto b/pbj-integration-tests/src/main/proto/viewChecks.proto index fe01f838f..8c635ccde 100644 --- a/pbj-integration-tests/src/main/proto/viewChecks.proto +++ b/pbj-integration-tests/src/main/proto/viewChecks.proto @@ -6,6 +6,8 @@ package proto; option java_package = "com.hedera.pbj.test.proto.java"; option java_multiple_files = true; +import "google/protobuf/wrappers.proto"; + /** * Messages for ViewValidationTest: views validate as parsing does, with the caller's limits and strict mode, and are * denied to messages that can't have them. @@ -22,6 +24,9 @@ message ViewCheckOuter { repeated string names = 5; string name = 6; repeated ViewCheckInner inners = 7; + bytes payload = 8; + repeated bytes payloads = 9; + google.protobuf.StringValue memo = 10; } // <<>> diff --git a/pbj-integration-tests/src/test/java/com/hedera/pbj/integration/test/ViewValidationTest.java b/pbj-integration-tests/src/test/java/com/hedera/pbj/integration/test/ViewValidationTest.java index 71da347cb..8f6810790 100644 --- a/pbj-integration-tests/src/test/java/com/hedera/pbj/integration/test/ViewValidationTest.java +++ b/pbj-integration-tests/src/test/java/com/hedera/pbj/integration/test/ViewValidationTest.java @@ -147,6 +147,151 @@ void maxDepthLimitsNestedViews() throws Exception { assertThrows(ParseException.class, inner::leaf); } + @Test + void strictWrapEnforcesMaxDepthBeforeAnythingIsRead() { + final Bytes bytes = new Fields().delimited(1, inner(4)).bytes(); + assertThrows( + ParseException.class, + () -> ViewCheckOuter.PROTOBUF.parse( + bytes.toReadableSequentialData(), true, false, 1, Codec.DEFAULT_MAX_SIZE)); + assertThrows( + ParseException.class, () -> new ViewCheckOuter.View().wrap(bytes, true, 1, Codec.DEFAULT_MAX_SIZE)); + } + + /** Messages strict parsing rejects only because of something nested, which a non-strict view reads lazily. */ + private static List nestedStrictViolations() { + final byte[] unknownField = + new Fields().tag(1000, VARINT).varint(1).bytes().toByteArray(); + final Bytes innerWithUnknown = + new Fields().tag(1, VARINT).varint(5).bytes().append(Bytes.wrap(unknownField)); + final Bytes leafWithUnknown = new Fields() + .delimited(2, new Fields().tag(1, VARINT).varint(6).bytes().append(Bytes.wrap(unknownField))) + .bytes(); + return List.of( + // an unknown field in a nested message, and in one nested two deep + new Fields().delimited(1, innerWithUnknown).bytes(), + new Fields().delimited(1, leafWithUnknown).bytes(), + // in a oneof alternative, and in the second element of a repeated message + new Fields().delimited(2, innerWithUnknown).bytes(), + new Fields() + .delimited(7, inner(1)) + .delimited(7, innerWithUnknown) + .bytes(), + // malformed UTF-8 in a wrapper type value + new Fields() + .delimited( + 10, + new Fields() + .delimited(1, new byte[] {(byte) 0xFF}) + .bytes()) + .bytes()); + } + + @Test + void strictWrapValidatesNestedMessagesAtAnyDepth() throws Exception { + for (final Bytes bytes : nestedStrictViolations()) { + assertThrows(ParseException.class, () -> ViewCheckOuter.PROTOBUF.parseStrict(bytes), bytes::toHex); + // strict mode fails fast, from wrap, even though nothing nested has been read + final ViewCheckOuter.View view = new ViewCheckOuter.View(); + assertThrows( + ParseException.class, + () -> view.wrap(bytes, true, Codec.DEFAULT_MAX_DEPTH, Codec.DEFAULT_MAX_SIZE), + bytes::toHex); + final byte[] array = bytes.toByteArray(); + assertThrows( + ParseException.class, + () -> view.wrap(array, 0, array.length, true, Codec.DEFAULT_MAX_DEPTH, Codec.DEFAULT_MAX_SIZE), + bytes::toHex); + } + } + + @Test + void nonStrictWrapLeavesUnreadNestedMessagesToTheirAccessors() throws Exception { + // documented: without strict mode only what is read is validated, as parsing would validate it + for (final Bytes bytes : nestedStrictViolations()) { + new ViewCheckOuter.View().wrap(bytes); + } + } + + @Test + void strictWrapStillReturnsNestedValues() throws Exception { + final ViewCheckOuter outer = ViewCheckOuter.newBuilder() + .inner(new ViewCheckInner(1, new ViewCheckLeaf(2))) + .innerChoice(new ViewCheckInner(3, null)) + .inners(List.of(new ViewCheckInner(4, null), new ViewCheckInner(5, new ViewCheckLeaf(6)))) + .memo("memo") + .build(); + final Bytes bytes = ViewCheckOuter.PROTOBUF.toBytes(outer); + final ViewCheckOuter.View view = + new ViewCheckOuter.View().wrap(bytes, true, Codec.DEFAULT_MAX_DEPTH, Codec.DEFAULT_MAX_SIZE); + assertEquals(2, view.inner().leaf().value()); + assertEquals(3, view.innerChoice().value()); + assertEquals("memo", view.memo()); + final List values = new ArrayList<>(); + for (final var it = view.inners(); it.next(); ) { + values.add(it.value().value()); + } + assertEquals(List.of(4, 5), values); + assertEquals(outer, view.toModel()); + } + + @Test + void bytesFromAViewOverAnArrayAreCopies() throws Exception { + final ViewCheckOuter outer = ViewCheckOuter.newBuilder() + .payload(Bytes.wrap(new byte[] {1, 2, 3})) + .payloads(List.of(Bytes.wrap(new byte[] {4, 5}))) + .build(); + final byte[] array = ViewCheckOuter.PROTOBUF.toBytes(outer).toByteArray(); + final ViewCheckOuter.View view = new ViewCheckOuter.View().wrap(array, 0, array.length); + final Bytes payload = view.payload(); + final var payloads = view.payloads(); + assertTrue(payloads.next()); + final Bytes element = payloads.value(); + final ViewCheckOuter model = view.toModel(); + // the caller reuses its array: what the view returned must not change + java.util.Arrays.fill(array, (byte) 0x7F); + assertEquals(Bytes.wrap(new byte[] {1, 2, 3}), payload); + assertEquals(Bytes.wrap(new byte[] {4, 5}), element); + assertEquals(outer, model); + } + + @Test + void bytesFromAViewOverBytesAreReadWithoutTheArray() throws Exception { + final ViewCheckOuter outer = ViewCheckOuter.newBuilder() + .inner(new ViewCheckInner(1, null)) + .payload(Bytes.wrap(new byte[] {1, 2, 3})) + .payloads(List.of(Bytes.wrap(new byte[] {4, 5}))) + .build(); + final ViewCheckOuter.View view = new ViewCheckOuter.View().wrap(ViewCheckOuter.PROTOBUF.toBytes(outer)); + assertEquals(outer.payload(), view.payload()); + assertEquals(3, view.payloadLength()); + assertTrue(view.payloadEquals(new byte[] {1, 2, 3})); + final byte[] copy = new byte[5]; + assertEquals(3, view.payloadCopyTo(copy, 2)); + assertArrayEquals(new byte[] {0, 0, 1, 2, 3}, copy); + final var payloads = view.payloads(); + assertTrue(payloads.next()); + assertEquals(Bytes.wrap(new byte[] {4, 5}), payloads.value()); + final byte[] elementCopy = new byte[2]; + assertEquals(2, payloads.valueCopyTo(elementCopy, 0)); + assertArrayEquals(new byte[] {4, 5}, elementCopy); + // an absent bytes field + final ViewCheckOuter.View empty = new ViewCheckOuter.View().wrap(Bytes.EMPTY); + assertEquals(Bytes.EMPTY, empty.payload()); + assertTrue(empty.payloadEquals(new byte[0])); + assertEquals(0, empty.payloadCopyTo(new byte[0], 0)); + } + + @Test + void aSourceMustHoldTheWrappedBytes() { + final Bytes bytes = ViewCheckOuter.PROTOBUF.toBytes( + ViewCheckOuter.newBuilder().name("name").build()); + final byte[] other = bytes.toByteArray(); + // a Bytes that does not hold the array cannot be passed off as its source + assertThrows(IllegalArgumentException.class, () -> new ViewCheckOuter.View() + .wrap(bytes, other, 0, other.length, false, Codec.DEFAULT_MAX_DEPTH, Codec.DEFAULT_MAX_SIZE)); + } + @Test void packedAndUnpackedElementsMixInOneRepeatedField() throws Exception { // packed [1, -2], then 3 unpacked, then packed [-4], then 5 unpacked