Skip to content

Commit 6889a39

Browse files
author
Roger Riggs
committed
8268826: Cleanup Override in Context-Specific Deserialization Filters
Reviewed-by: dfuchs, bchristi
1 parent f791fdf commit 6889a39

13 files changed

Lines changed: 211 additions & 103 deletions

‎src/java.base/share/classes/java/io/ObjectInputFilter.java‎

Lines changed: 41 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import static java.io.ObjectInputFilter.Status.*;
4646
import static java.lang.System.Logger.Level.TRACE;
4747
import static java.lang.System.Logger.Level.DEBUG;
48+
import static java.lang.System.Logger.Level.ERROR;
4849

4950
/**
5051
* Filter classes, array lengths, and graph metrics during deserialization.
@@ -308,10 +309,12 @@ public interface ObjectInputFilter {
308309
* <li>Otherwise, return {@code otherStatus}.</li>
309310
* </ul>
310311
* <p>
311-
* Example, to create a filter that will allow any class loaded from the platform classloader.
312+
* Example, to create a filter that will allow any class loaded from the platform
313+
* or bootstrap classloaders.
312314
* <pre><code>
313-
* ObjectInputFilter f = allowFilter(cl -> cl.getClassLoader() == ClassLoader.getPlatformClassLoader()
314-
* || cl.getClassLoader() == null, Status.UNDECIDED);
315+
* ObjectInputFilter f
316+
* = allowFilter(cl -> cl.getClassLoader() == ClassLoader.getPlatformClassLoader() ||
317+
* cl.getClassLoader() == null, Status.UNDECIDED);
315318
* </code></pre>
316319
*
317320
* @param predicate a predicate to test a non-null Class
@@ -527,23 +530,17 @@ enum Status {
527530
* The syntax for the property value is the same as for the
528531
* {@link #createFilter(String) createFilter} method.
529532
*
530-
* <p> If only `jdk.serialFilter` is set and not `jdk.serialFilterFactory` the builtin
531-
* filter factory, compatible with previous versions, is set and can not be replaced,
532-
* see below to override the builtin filter factory.
533533
* <p>
534534
* If the Java virtual machine is started with the system property
535535
* {@systemProperty jdk.serialFilterFactory} or the {@link java.security.Security} property
536536
* of the same name, its value names the class to configure the JVM-wide deserialization
537-
* filter factory or the special value `OVERRIDE`.
537+
* filter factory.
538538
* If the system property is not defined, and the {@link java.security.Security} property
539539
* {@code jdk.serialFilterFactory} is defined then it is used to configure the filter factory.
540-
*
541-
* If the value is `OVERRIDE`, the filter factory can be set by the application before
542-
* the first deserialization using {@link Config#setSerialFilterFactory(BinaryOperator)};
543540
* If it remains unset, the filter factory is a builtin filter factory compatible
544541
* with previous versions.
545542
*
546-
* <p>If not `OVERRIDE`, the class must be public, must have a public zero-argument constructor, implement the
543+
* <p>The class must be public, must have a public zero-argument constructor, implement the
547544
* {@link BinaryOperator {@literal BinaryOperator<ObjectInputFilter>}} interface, provide its implementation and
548545
* be accessible via the {@linkplain ClassLoader#getSystemClassLoader() application class loader}.
549546
* If the filter factory constructor is not invoked successfully, an {@link ExceptionInInitializerError}
@@ -579,11 +576,6 @@ final class Config {
579576
*/
580577
private static final String SERIAL_FILTER_FACTORY_PROPNAME = "jdk.serialFilterFactory";
581578

582-
/**
583-
* The property name to enable tracing of filters.
584-
*/
585-
private static final String SERIAL_FILTER_TRACE_PROPNAME = "jdk.serialFilterTrace";
586-
587579
/**
588580
* Current static filter.
589581
*/
@@ -599,34 +591,30 @@ final class Config {
599591
* Boolean to indicate that the filter factory can not be set or replaced.
600592
* - an ObjectInputStream has already been created using the current filter factory
601593
* - has been set on the command line
602-
* - jdk.serialFilter is set and jdk.serialFilterFactory is unset, the builtin can not be replaced
603594
* @see Config#setSerialFilterFactory(BinaryOperator)
604595
*/
605596
private static final AtomicBoolean filterFactoryNoReplace = new AtomicBoolean();
606597

607598
/**
608-
* Debug: Logger
599+
* Debug and Trace Logger
609600
*/
610601
private static final System.Logger configLog;
611602

612-
/**
613-
* True when tracing of filters is enabled.
614-
*/
615-
private static final boolean traceFilters;
616-
617603
static {
618604
/*
619605
* Initialize the configuration containing the filter factory, static filter, and logger.
620606
* <ul>
607+
* <li>The logger is created.
621608
* <li>The property 'jdk.serialFilter" is read, either as a system property or a security property,
622609
* and if set, defines the configured static JVM-wide filter and is logged.
623610
* <li>The property jdk.serialFilterFactory is read, either as a system property or a security property,
624611
* and if set, defines the initial filter factory and is logged.
625-
* <li>The property jdk.serialFilterTrace, is read, and if set enables tracing of filters.
626-
* <li>If either property is defined or tracing is enabled, the logger is created.
627612
* </ul>
628613
*/
629614

615+
// Initialize the logger.
616+
configLog = System.getLogger("java.io.serialization");
617+
630618
// Get the values of the system properties, if they are defined
631619
String factoryClassName = StaticProperty.jdkSerialFilterFactory();
632620
if (factoryClassName == null) {
@@ -642,12 +630,6 @@ final class Config {
642630
Security.getProperty(SERIAL_FILTER_PROPNAME));
643631
}
644632

645-
traceFilters = GetBooleanAction.privilegedGetProperty(SERIAL_FILTER_TRACE_PROPNAME);
646-
647-
// Initialize the logger if either filter factory or filter property is set
648-
configLog = (filterString != null || factoryClassName != null || traceFilters)
649-
? System.getLogger("java.io.serialization") : null;
650-
651633
// Initialize the static filter if the jdk.serialFilter is present
652634
ObjectInputFilter filter = null;
653635
if (filterString != null) {
@@ -656,53 +638,40 @@ final class Config {
656638
try {
657639
filter = createFilter(filterString);
658640
} catch (RuntimeException re) {
659-
configLog.log(System.Logger.Level.ERROR,
641+
configLog.log(ERROR,
660642
"Error configuring filter: {0}", re);
661643
}
662644
}
663645
serialFilter = filter;
664646

665647
// Initialize the filter factory if the jdk.serialFilterFactory is defined
666648
// otherwise use the builtin filter factory.
667-
if (factoryClassName == null || "OVERRIDE".equals(factoryClassName)) {
649+
if (factoryClassName == null) {
668650
serialFilterFactory = new BuiltinFilterFactory();
669-
if (serialFilter != null && factoryClassName == null) {
670-
// Ensure backward compatibility, unless factory is explicitly allowed to override
671-
// Do not allow factory to be overridden by Config.setSerialFilterFactory
672-
filterFactoryNoReplace.set(true);
673-
}
674-
675651
} else {
676-
configLog.log(DEBUG,
677-
"Creating deserialization filter factory for {0}", factoryClassName);
678652
try {
679653
// Load using the system class loader, the named class may be an application class.
680-
// The static initialization of the class or constructor may create a race
681-
// if either calls Config.setSerialFilterFactory; the command line configured
682-
// Class should not be overridden.
654+
// Cause Config.setSerialFilterFactory to throw {@link IllegalStateException}
655+
// if Config.setSerialFilterFactory is called as a side effect of the
656+
// static initialization of the class or constructor.
657+
filterFactoryNoReplace.set(true);
658+
683659
Class<?> factoryClass = Class.forName(factoryClassName, true,
684660
ClassLoader.getSystemClassLoader());
685661
@SuppressWarnings("unchecked")
686-
BinaryOperator<ObjectInputFilter> f =
662+
BinaryOperator<ObjectInputFilter> factory =
687663
(BinaryOperator<ObjectInputFilter>)
688664
factoryClass.getConstructor().newInstance(new Object[0]);
689-
if (serialFilterFactory != null) {
690-
// Init cycle if Config.setSerialFilterFactory called from class initialization
691-
configLog.log(System.Logger.Level.ERROR,
692-
"FilterFactory provided on the command line can not be overridden");
693-
// Do not continue if configuration not initialized
694-
throw new ExceptionInInitializerError(
695-
"FilterFactory provided on the command line can not be overridden");
696-
}
697-
serialFilterFactory = f;
698-
filterFactoryNoReplace.set(true);
665+
configLog.log(DEBUG,
666+
"Creating deserialization filter factory for {0}", factoryClassName);
667+
serialFilterFactory = factory;
699668
} catch (RuntimeException | ClassNotFoundException | NoSuchMethodException |
700669
IllegalAccessException | InstantiationException | InvocationTargetException ex) {
701-
configLog.log(System.Logger.Level.ERROR,
702-
"Error configuring filter factory", ex);
670+
Throwable th = (ex instanceof InvocationTargetException ite) ? ite.getCause() : ex;
671+
configLog.log(ERROR,
672+
"Error configuring filter factory: {0}", (Object)th);
703673
// Do not continue if configuration not initialized
704-
throw new ExceptionInInitializerError(
705-
"FilterFactory configuration: jdk.serialFilterFactory: " + ex.getMessage());
674+
throw new ExceptionInInitializerError(th);
706675
}
707676
}
708677
// Setup shared secrets for RegistryImpl to use.
@@ -719,9 +688,7 @@ private Config() {
719688
* Logger for filter actions.
720689
*/
721690
private static void traceFilter(String msg, Object... args) {
722-
if (traceFilters && configLog != null) {
723-
configLog.log(TRACE, msg, args);
724-
}
691+
configLog.log(TRACE, msg, args);
725692
}
726693

727694
/**
@@ -840,12 +807,14 @@ public static void setSerialFilterFactory(BinaryOperator<ObjectInputFilter> filt
840807
if (sm != null) {
841808
sm.checkPermission(ObjectStreamConstants.SERIAL_FILTER_PERMISSION);
842809
}
843-
if (serialFilterFactory == null)
844-
throw new IllegalStateException("Serial filter factory initialization incomplete");
845810
if (filterFactoryNoReplace.getAndSet(true)) {
846-
throw new IllegalStateException("Cannot replace filter factory: " +
847-
serialFilterFactory.getClass().getName());
811+
final String msg = serialFilterFactory != null
812+
? serialFilterFactory.getClass().getName()
813+
: "initialization incomplete";
814+
throw new IllegalStateException("Cannot replace filter factory: " + msg);
848815
}
816+
configLog.log(DEBUG,
817+
"Setting deserialization filter factory to {0}", filterFactory.getClass().getName());
849818
serialFilterFactory = filterFactory;
850819
}
851820

@@ -1163,7 +1132,7 @@ public Status checkInput(FilterInfo filterInfo) {
11631132
}
11641133
if (!checkComponentType) {
11651134
// As revised; do not check the component type for arrays
1166-
traceFilter("Pattern array class: {0}, filter: {1}", clazz, this);
1135+
traceFilter("Pattern filter array class: {0}, filter: {1}", clazz, this);
11671136
return Status.UNDECIDED;
11681137
}
11691138
do {
@@ -1174,7 +1143,7 @@ public Status checkInput(FilterInfo filterInfo) {
11741143

11751144
if (clazz.isPrimitive()) {
11761145
// Primitive types are undecided; let someone else decide
1177-
traceFilter("Pattern UNDECIDED, primitive class: {0}, filter: {1}", clazz, this);
1146+
traceFilter("Pattern filter UNDECIDED, primitive class: {0}, filter: {1}", clazz, this);
11781147
return UNDECIDED;
11791148
} else {
11801149
// Find any filter that allowed or rejected the class
@@ -1184,7 +1153,7 @@ public Status checkInput(FilterInfo filterInfo) {
11841153
.filter(p -> p != Status.UNDECIDED)
11851154
.findFirst();
11861155
Status s = status.orElse(Status.UNDECIDED);
1187-
traceFilter("Pattern {0}, class: {1}, filter: {2}", s, cl, this);
1156+
traceFilter("Pattern filter {0}, class: {1}, filter: {2}", s, cl, this);
11881157
return s;
11891158
}
11901159
}
@@ -1283,18 +1252,18 @@ private static class MergeFilter implements ObjectInputFilter {
12831252
public ObjectInputFilter.Status checkInput(FilterInfo info) {
12841253
Status firstStatus = Objects.requireNonNull(first.checkInput(info), "status");
12851254
if (REJECTED.equals(firstStatus)) {
1286-
traceFilter("MergeFilter REJECT first: {0}, filter: {1}",
1255+
traceFilter("MergeFilter REJECTED first: {0}, filter: {1}",
12871256
firstStatus, this);
12881257
return REJECTED;
12891258
}
12901259
Status secondStatus = Objects.requireNonNull(second.checkInput(info), "other status");
12911260
if (REJECTED.equals(secondStatus)) {
1292-
traceFilter("MergeFilter REJECT {0}, {1}, filter: {2}",
1261+
traceFilter("MergeFilter REJECTED {0}, {1}, filter: {2}",
12931262
firstStatus, secondStatus, this);
12941263
return REJECTED;
12951264
}
12961265
if (ALLOWED.equals(firstStatus) || ALLOWED.equals(secondStatus)) {
1297-
traceFilter("MergeFilter ALLOW either: {0}, {1}, filter: {2}",
1266+
traceFilter("MergeFilter ALLOWED either: {0}, {1}, filter: {2}",
12981267
firstStatus, secondStatus, this);
12991268
return ALLOWED;
13001269
}
@@ -1332,7 +1301,6 @@ public ObjectInputFilter.Status checkInput(FilterInfo info) {
13321301
Class<?> clazz = info.serialClass();
13331302
if (clazz == null || !UNDECIDED.equals(status))
13341303
return status;
1335-
status = REJECTED;
13361304
// Find the base component type
13371305
while (clazz.isArray()) {
13381306
clazz = clazz.getComponentType();

‎src/java.base/share/classes/jdk/internal/util/StaticProperty.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ public static String jdkSerialFilter() {
194194
* in this method. The caller of this method should take care to ensure
195195
* that the returned property is not made accessible to untrusted code.</strong>
196196
*
197-
* @return the {@code user.name} system property
197+
* @return the {@code jdk.serialFilterFactory} system property
198198
*/
199199
public static String jdkSerialFilterFactory() {
200200
return JDK_SERIAL_FILTER_FACTORY;

‎src/java.base/share/conf/security/java.security‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -980,14 +980,12 @@ jdk.xml.dsig.secureValidationPolicy=\
980980

981981

982982
#
983-
# Deserialization system-wide filter factory
983+
# Deserialization JVM-wide filter factory
984984
#
985-
# A filter factory class name is used to configure the system-wide filter factory.
986-
# The filter factory value "OVERRIDE" in combination with setting "jdk.serialFilter"
987-
# indicates that the builtin filter factory can be overridden by the application.
985+
# A filter factory class name is used to configure the JVM-wide filter factory.
988986
# The class must be public, must have a public zero-argument constructor, implement the
989-
# java.util.stream.BinaryOperator<ObjectInputFilter> interface, provide its implementation and
990-
# be accessible via the application class loader.
987+
# java.util.function.BinaryOperator<java.io.ObjectInputFilter> interface, provide its
988+
# implementation and be accessible via the application class loader.
991989
# A builtin filter factory is used if no filter factory is defined.
992990
# See java.io.ObjectInputFilter.Config for more information.
993991
#
@@ -997,7 +995,7 @@ jdk.xml.dsig.secureValidationPolicy=\
997995
#jdk.serialFilterFactory=<classname>
998996

999997
#
1000-
# Deserialization system-wide filter
998+
# Deserialization JVM-wide filter
1001999
#
10021000
# A filter, if configured, is used by the filter factory to provide the filter used by
10031001
# java.io.ObjectInputStream during deserialization to check the contents of the stream.

‎test/jdk/java/io/Serializable/serialFilter/FilterWithSecurityManagerTest.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -54,6 +54,7 @@ public class FilterWithSecurityManagerTest {
5454
ObjectInputFilter filter;
5555

5656
@BeforeClass
57+
@SuppressWarnings("removal")
5758
public void setup() throws Exception {
5859
setSecurityManager = System.getSecurityManager() != null;
5960
Object toDeserialized = Long.MAX_VALUE;
@@ -65,7 +66,8 @@ public void setup() throws Exception {
6566
* Test that setting process-wide filter is checked by security manager.
6667
*/
6768
@Test
68-
public void testGlobalFilter() throws Exception {
69+
@SuppressWarnings("removal")
70+
public void testGlobalFilter() {
6971
ObjectInputFilter global = ObjectInputFilter.Config.getSerialFilter();
7072

7173
try {
@@ -88,6 +90,7 @@ public void testGlobalFilter() throws Exception {
8890
* Test that setting specific filter is checked by security manager.
8991
*/
9092
@Test(dependsOnMethods = { "testGlobalFilter" })
93+
@SuppressWarnings("removal")
9194
public void testSpecificFilter() throws Exception {
9295
try (ByteArrayInputStream bais = new ByteArrayInputStream(bytes);
9396
ObjectInputStream ois = new ObjectInputStream(bais)) {

‎test/jdk/java/io/Serializable/serialFilter/GlobalFilterTest.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ static void globalFilter() {
142142
* If there is no security manager then setting it should work.
143143
*/
144144
@Test()
145+
@SuppressWarnings("removal")
145146
static void setGlobalFilter() {
146147
SecurityManager sm = System.getSecurityManager();
147148
ObjectInputFilter filter = new SerialFilterTest.Validator();

0 commit comments

Comments
 (0)