From 7d2a871f046c25bb03d75128408a6daeeb000a99 Mon Sep 17 00:00:00 2001 From: ABHILAKSHYA DOBHAL Date: Sat, 8 Mar 2025 18:12:12 +0530 Subject: [PATCH 1/4] HADOOP-19315: [Java] Improve trustedPackages initialization in SpecificDatumReader This commit addresses an issue in Avro's SpecificDatumReader where the trustedPackages list was not properly initialized in every constructor. Although the issue is tracked under HADOOP-19315 in the Hadoop JIRA board, the underlying problem exists in Avro. Changes include: - Modifying the static initialization block to combine both user-defined packages from the org.apache.avro.SERIALIZABLE_PACKAGES system property and the default packages ("java.lang,java.math,java.io,java.net,org.apache.avro.reflect"), ensuring that duplicates are removed. - Updating the initializeTrustedPackages() method to call clear() before adding packages, preventing duplicate entries. - Ensuring that initializeTrustedPackages() is invoked in every constructor of SpecificDatumReader so that each instance always has the correct trustedPackages list. This improvement ensures that custom packages specified via the system property are correctly recognized as trusted, preventing security errors during deserialization. --- .../avro/specific/SpecificDatumReader.java | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java b/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java index 29c8ac66567..cb2b30513a2 100644 --- a/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java +++ b/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java @@ -37,30 +37,42 @@ public class SpecificDatumReader extends GenericDatumReader { public static final String[] SERIALIZABLE_PACKAGES; static { - SERIALIZABLE_PACKAGES = System.getProperty("org.apache.avro.SERIALIZABLE_PACKAGES", - "java.lang,java.math,java.io,java.net,org.apache.avro.reflect").split(","); + String defaultPackages = "java.lang,java.math,java.io,java.net,org.apache.avro.reflect"; + + String userDefinedPackages = System.getProperty("org.apache.avro.SERIALIZABLE_PACKAGES", ""); + + // Combine the user-defined packages (if any) with the default packages. + String combined = userProp.isEmpty() ? defaultPackages : userProp + "," + defaultPackages; + + SERIALIZABLE_PACKAGES = Arrays.stream(combined.split(",")) + .distinct() + .toArray(String[]::new); } private final List trustedPackages = new ArrayList<>(); public SpecificDatumReader() { this(null, null, SpecificData.get()); + initializeTrustedPackages(); } /** Construct for reading instances of a class. */ public SpecificDatumReader(Class c) { this(SpecificData.getForClass(c)); setSchema(getSpecificData().getSchema(c)); + initializeTrustedPackages(); } /** Construct where the writer's and reader's schemas are the same. */ public SpecificDatumReader(Schema schema) { this(schema, schema, SpecificData.getForSchema(schema)); + initializeTrustedPackages(); } /** Construct given writer's and reader's schema. */ public SpecificDatumReader(Schema writer, Schema reader) { this(writer, reader, SpecificData.getForSchema(reader)); + initializeTrustedPackages(); } /** @@ -68,7 +80,7 @@ public SpecificDatumReader(Schema writer, Schema reader) { */ public SpecificDatumReader(Schema writer, Schema reader, SpecificData data) { super(writer, reader, data); - trustedPackages.addAll(Arrays.asList(SERIALIZABLE_PACKAGES)); + initializeTrustedPackages(); } /** Construct given a {@link SpecificData}. */ From 0beafc0daea391dbb08081c799417ab8033b57e4 Mon Sep 17 00:00:00 2001 From: ABHILAKSHYA DOBHAL Date: Tue, 11 Mar 2025 19:49:49 +0530 Subject: [PATCH 2/4] AVRO-3985: adding default serializable pacakge to trusted package list --- .../avro/specific/SpecificDatumReader.java | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java b/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java index cb2b30513a2..b6a98f0fe15 100644 --- a/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java +++ b/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java @@ -41,38 +41,37 @@ public class SpecificDatumReader extends GenericDatumReader { String userDefinedPackages = System.getProperty("org.apache.avro.SERIALIZABLE_PACKAGES", ""); - // Combine the user-defined packages (if any) with the default packages. - String combined = userProp.isEmpty() ? defaultPackages : userProp + "," + defaultPackages; + if ("*".equals(userDefinedPackages)) { + SERIALIZABLE_PACKAGES = new String[]{"*"}; + } else { + String combinedPackages = userDefinedPackages.isEmpty() ? defaultPackages : userDefinedPackages + "," + defaultPackages; - SERIALIZABLE_PACKAGES = Arrays.stream(combined.split(",")) - .distinct() - .toArray(String[]::new); + SERIALIZABLE_PACKAGES = Arrays.stream(combinedPackages.split(",")) + .distinct() + .toArray(String[]::new); + } } private final List trustedPackages = new ArrayList<>(); public SpecificDatumReader() { this(null, null, SpecificData.get()); - initializeTrustedPackages(); } /** Construct for reading instances of a class. */ public SpecificDatumReader(Class c) { this(SpecificData.getForClass(c)); setSchema(getSpecificData().getSchema(c)); - initializeTrustedPackages(); } /** Construct where the writer's and reader's schemas are the same. */ public SpecificDatumReader(Schema schema) { this(schema, schema, SpecificData.getForSchema(schema)); - initializeTrustedPackages(); } /** Construct given writer's and reader's schema. */ public SpecificDatumReader(Schema writer, Schema reader) { this(writer, reader, SpecificData.getForSchema(reader)); - initializeTrustedPackages(); } /** @@ -80,7 +79,7 @@ public SpecificDatumReader(Schema writer, Schema reader) { */ public SpecificDatumReader(Schema writer, Schema reader, SpecificData data) { super(writer, reader, data); - initializeTrustedPackages(); + trustedPackages.addAll(Arrays.asList(SERIALIZABLE_PACKAGES)); } /** Construct given a {@link SpecificData}. */ From bdfff4963609e7d3de87d7ae2fcbe65c3ee76c93 Mon Sep 17 00:00:00 2001 From: ABHILAKSHYA DOBHAL Date: Sat, 22 Mar 2025 15:08:02 +0530 Subject: [PATCH 3/4] added-documentation-for-default-packages --- .../apache/avro/specific/SpecificDatumReader.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java b/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java index b6a98f0fe15..a5e0f759f10 100644 --- a/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java +++ b/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java @@ -36,11 +36,17 @@ public class SpecificDatumReader extends GenericDatumReader { public static final String[] SERIALIZABLE_PACKAGES; - static { - String defaultPackages = "java.lang,java.math,java.io,java.net,org.apache.avro.reflect"; - - String userDefinedPackages = System.getProperty("org.apache.avro.SERIALIZABLE_PACKAGES", ""); + private static final String DEFAULT_PACKAGES = "java.lang,java.math,java.io,java.net,org.apache.avro.reflect"; + static { + String userDefinedPackages = System.getProperty("org.apache.avro.SERIALIZABLE_PACKAGES", DEFAULT_PACKAGES); + + /* + * Note: + * - There are some packages that has already been considered trustable by Avro. + * - If a user defines custom packages but does not include these default ones, they may face + * errors when deserializing objects that rely on them. + */ if ("*".equals(userDefinedPackages)) { SERIALIZABLE_PACKAGES = new String[]{"*"}; } else { From 03937253676298eb5de4c339ddb8e098110b17d2 Mon Sep 17 00:00:00 2001 From: ABHILAKSHYA DOBHAL Date: Sat, 22 Mar 2025 15:09:40 +0530 Subject: [PATCH 4/4] added-documentation-for-default-packages --- .../java/org/apache/avro/specific/SpecificDatumReader.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java b/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java index a5e0f759f10..a84e3e41fb1 100644 --- a/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java +++ b/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java @@ -50,9 +50,7 @@ public class SpecificDatumReader extends GenericDatumReader { if ("*".equals(userDefinedPackages)) { SERIALIZABLE_PACKAGES = new String[]{"*"}; } else { - String combinedPackages = userDefinedPackages.isEmpty() ? defaultPackages : userDefinedPackages + "," + defaultPackages; - - SERIALIZABLE_PACKAGES = Arrays.stream(combinedPackages.split(",")) + SERIALIZABLE_PACKAGES = Arrays.stream(userDefinedPackages.split(",")) .distinct() .toArray(String[]::new); }