damansingh1313 commented on code in PR #17852:
URL: https://github.com/apache/iceberg/pull/17852#discussion_r3886400947
##########
core/src/main/java/org/apache/iceberg/avro/GenericAvroReader.java:
##########
@@ -30,13 +31,39 @@
import org.apache.avro.io.Decoder;
import org.apache.iceberg.common.DynClasses;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet;
import org.apache.iceberg.types.Type;
import org.apache.iceberg.types.Types;
import org.apache.iceberg.util.Pair;
public class GenericAvroReader<T>
implements DatumReader<T>, SupportsRowPosition, SupportsCustomRecords {
+ // Restricts record-name-based class resolution to a known-safe set. Record
names come from
+ // Avro schemas, which can originate from untrusted input (e.g.
AvroEncoderUtil.decode);
+ // without this check, any class name that happens to match something
already on the classpath
+ // would be loaded and constructed. See
https://github.com/apache/iceberg/issues/17802.
+ //
+ // NOTE: Kafka Connect's classes are listed by name (not Class literal)
because iceberg-core
+ // cannot depend on iceberg-kafka-connect-events. Keep this in sync with
that module's
+ // AvroUtil.FIELD_ID_TO_CLASS if either changes.
+ private static final Set<String> ALLOWED_RECORD_CLASSES =
Review Comment:
For the current 8 Kafka Connect classes, this is already covered —
TestEventSerialization round-trips each one with a strict recursive comparison,
so a missing/renamed entry fails CI today (that module's actually in our
regular build scope, unlike flink, where we found the real gap above).
The open case is a future class added to kafka-connect-events with no
corresponding test — nothing can fully guarantee that at compile time given the
module boundary. The WARN log proposed above would catch that too, though: it
fires for any allowlist gap, Kafka Connect or otherwise, and names the exact
missing class. I'd lean on that rather than a dedicated cross-module test, but
happy to add one if you'd still want the extra guarantee.
--
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]
