steveloughran commented on code in PR #3562:
URL: https://github.com/apache/parquet-java/pull/3562#discussion_r4111952552
##########
parquet-variant/src/main/java/org/apache/parquet/variant/VariantUtil.java:
##########
@@ -880,6 +910,161 @@ static HashMap<String, Integer> getMetadataMap(ByteBuffer
metadata) {
return result;
}
+ /**
+ * Bounds-checks a single Variant value node against its buffer slot.
Performs no recursion
+ * into nested children: child nodes are checked on demand when callers
descend into them.
+ *
+ * <p>Cost: O(1) for primitives and short strings, O(numElements) for
objects and arrays.
+ * Validation of nested structures is deferred so that opening a large
well-formed Variant
+ * is not penalized by sub-trees the caller never inspects.
+ *
+ * <p>Note that as validation is against the total bytebuffer capacity, a
variant can expand into
+ * the buffer space nominally allocated its siblings. This is not something
checked
+ * for; it is memory safe.
+ * @param valueBuffer the variant value buffer (position/limit define the
extent of this node's slot)
+ * @param dictSize the metadata dictionary size, used to bound object field
ids
+ * @throws IllegalArgumentException if the value header or container table
does not fit within
+ * the buffer slot, or if any object field id is out of range
+ */
+ static void validateValueShallow(final ByteBuffer valueBuffer, final int
dictSize) {
+ int pos = valueBuffer.position();
+ long slot = (long) valueBuffer.limit() - pos;
+ Preconditions.checkArgument(slot > 0, "variant value is empty");
+ int header = valueBuffer.get(pos) & 0xFF;
+ int basicType = header & BASIC_TYPE_MASK;
+ int typeInfo = (header >> BASIC_TYPE_BITS) & PRIMITIVE_TYPE_MASK;
+ switch (basicType) {
+ case SHORT_STR:
+ Preconditions.checkArgument(1L + typeInfo <= slot, "variant short
string extends past buffer");
+ return;
+ case OBJECT:
+ validateContainerShallow(valueBuffer, typeInfo, pos, slot, dictSize,
true);
+ return;
+ case ARRAY:
+ validateContainerShallow(valueBuffer, typeInfo, pos, slot, dictSize,
false);
+ return;
+ default:
+ validatePrimitiveShallow(valueBuffer, typeInfo, pos, slot);
+ }
+ }
+
+ /**
+ * Shallow validation of a container.
+ *
+ * @param valueBuffer buffer with the variant data
+ * @param typeInfo type information from the metadata
+ * @param pos buffer read position
+ * @param length length of slot for this container.
+ * @param dictSize dictionary size
+ * @param isObject is this an object?
+ */
+ private static void validateContainerShallow(
+ final ByteBuffer valueBuffer,
+ final int typeInfo,
+ final int pos,
+ final long length,
+ final int dictSize,
+ final boolean isObject) {
+ boolean largeSize;
+ int idSize;
+ if (isObject) {
+ largeSize = ((typeInfo >> 4) & 0x1) != 0;
+ idSize = ((typeInfo >> 2) & 0x3) + 1;
+ } else {
+ largeSize = ((typeInfo >> 2) & 0x1) != 0;
+ idSize = 0;
+ }
+ int offsetSize = (typeInfo & 0x3) + 1;
+ int sizeBytes = largeSize ? U32_SIZE : 1;
+ Preconditions.checkArgument(1L + sizeBytes <= length, "variant container
header truncated");
+ long numElements = readUnsignedLittleEndian(valueBuffer, pos + 1,
sizeBytes);
+ long idStart = 1L + sizeBytes;
+ long idBytes = isObject ? numElements * idSize : 0L;
+ long offsetStart = idStart + idBytes;
+ long offsetBytes = (numElements + 1) * offsetSize;
+ long dataStart = offsetStart + offsetBytes;
+ Preconditions.checkArgument(
+ dataStart <= length, "variant container offset table extends past
buffer: numElements=%s", numElements);
+ long dataLen = length - dataStart;
+ if (isObject) {
+ for (int i = 0; i < numElements; i++) {
+ int id = readUnsignedLittleEndian(valueBuffer, pos + (int) idStart + i
* idSize, idSize);
+ Preconditions.checkArgument(
+ id < dictSize, "variant object key id %s out of range
(dictSize=%s)", id, dictSize);
+ }
+ }
+ // Each child offset must lie within the data region. Children may overlap
or leave gaps;
+ // the trailing terminator offset is range-checked for the same reason.
+ for (int i = 0; i <= numElements; i++) {
Review Comment:
You do take the hit on the base node, but there shouldn't be any rescanning
inside a nested variant structure.
otherwise, apart from a "trusted/untrusted" flag, which is dangerous as the
trust boundary becomes that how that flag is set, can you suggest other
approaches?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
