[ 
https://issues.apache.org/jira/browse/FLINK-40911?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Timo Walther closed FLINK-40911.
--------------------------------
    Fix Version/s: 2.4.0
       Resolution: Fixed

Fixed in master: 374289707dc40a056f0a97914da65a20b19a26da

> Variant builder mishandles a nested variant and a value of exactly 16 MiB
> -------------------------------------------------------------------------
>
>                 Key: FLINK-40911
>                 URL: https://issues.apache.org/jira/browse/FLINK-40911
>             Project: Flink
>          Issue Type: Bug
>          Components: API / Core
>            Reporter: Ramin Gharib
>            Assignee: Ramin Gharib
>            Priority: Major
>             Fix For: 2.4.0
>
>
> h3. Problem 1: embedding a field or element of another variant
> {\{BinaryVariantInternalBuilder.appendVariant}} reads the value through 
> \{{getValue()}} but passes the original \{{getPos()}}. A field or element of 
> another variant, from \{{getField}} or \{{getElement}}, shares its parent's 
> buffer and starts at \{{pos > 0}}. \{{getValue()}} copies that slice to a new 
> array that starts at 0, and the builder then reads the copy at the old 
> position. Depending on the offsets, the result is a \{{MALFORMED_VARIANT}} 
> error or a silently wrong value.
> The object and array builders of the public \{{VariantBuilder}} call this 
> method, so user code hits it too.
> {code:java}
> BinaryVariant source =
>         BinaryVariantInternalBuilder.parseJson(
>                 "\{\"a\":[7,8,9],\"b\":\"hello\",\"c\":{\"d\":true}}", false);
> VariantBuilder builder = Variant.newBuilder();
> // Expected [[7,8,9],"hello",\{"d":true}]. Fails with MALFORMED_VARIANT 
> instead.
> builder.array()
>         .add(source.getField("a"))
>         .add(source.getField("b"))
>         .add(source.getField("c"))
>         .build();
> {code}
> h3. Problem 2: a value of exactly 16 MiB
> The builder asserts that every size, offset and id fits into 3 bytes. An 
> array or object whose content fills exactly 16 MiB breaks that assertion 
> before the size check runs. With assertions enabled, it fails with a bare 
> \{{AssertionError}}. Without them, it fails with 
> \{{VARIANT_SIZE_LIMIT_EXCEPTION}} as intended.
> {code:java}
> BinaryVariantInternalBuilder builder = new 
> BinaryVariantInternalBuilder(false);
> // With its 5-byte header, this binary value fills exactly 16 MiB.
> builder.appendBinary(new byte[BinaryVariantUtil.SIZE_LIMIT - 5]);
> // AssertionError with -ea, VARIANT_SIZE_LIMIT_EXCEPTION without.
> builder.finishWritingArray(0, new ArrayList<>(List.of(0)));
> {code}
> h3. Fix
> * Read the shared buffer at the variant's own position: 
> \{{appendVariantImpl(v.rawValue(), v.getMetadata(), v.getPos())}}. This also 
> saves the copy.
> * Throw \{{VARIANT_SIZE_LIMIT_EXCEPTION}} instead of asserting when a size, 
> offset or id does not fit into 3 bytes. Such a value is always over the size 
> limit.
> Both bugs are in every release with VARIANT, 2.1.0 and later. The fix is the 
> first commit of https://github.com/apache/flink/pull/29369 and can be 
> cherry-picked on its own.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to