Skip to content

Commit 716a980

Browse files
committed
Address PR review feedback: cache filter, add null check, add logging
- Implement filter caching using double-checked locking with volatile fields to eliminate race conditions and improve performance - Add null check before setObjectInputFilter() for defensive programming - Add INFO logging when filter is configured and WARN logging when not configured to improve security visibility Addresses review comments by @sboorlagadda on PR #7966
1 parent 1605e10 commit 716a980

2 files changed

Lines changed: 40 additions & 7 deletions

File tree

extensions/geode-modules-session-internal/src/main/java/org/apache/geode/modules/session/internal/filter/GemfireHttpSession.java

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,13 @@ public class GemfireHttpSession implements HttpSession, DataSerializable, Delta
7979

8080
private ServletContext context;
8181

82+
/**
83+
* Cached ObjectInputFilter to avoid recreating on every deserialization.
84+
* Initialized lazily on first use with double-checked locking.
85+
*/
86+
private volatile ObjectInputFilter cachedFilter;
87+
private volatile boolean filterLogged = false;
88+
8289
/**
8390
* A session becomes invalid if it is explicitly invalidated or if it expires.
8491
*/
@@ -108,6 +115,34 @@ public DataSerializable newInstance() {
108115
});
109116
}
110117

118+
/**
119+
* Gets or creates the cached ObjectInputFilter. Uses double-checked locking to avoid
120+
* unnecessary synchronization after initialization.
121+
*
122+
* @return the cached ObjectInputFilter, or null if no filter is configured
123+
*/
124+
private ObjectInputFilter getOrCreateFilter() {
125+
if (cachedFilter == null && !filterLogged) {
126+
synchronized (this) {
127+
if (cachedFilter == null && !filterLogged) {
128+
String filterPattern = getServletContext()
129+
.getInitParameter("serializable-object-filter");
130+
131+
if (filterPattern != null) {
132+
cachedFilter = ObjectInputFilter.Config.createFilter(filterPattern);
133+
LOG.info("ObjectInputFilter configured with pattern: {}", filterPattern);
134+
} else {
135+
LOG.warn("No ObjectInputFilter configured. Session deserialization is not protected " +
136+
"against malicious payloads. Configure 'serializable-object-filter' in web.xml " +
137+
"to enable deserialization security.");
138+
}
139+
filterLogged = true;
140+
}
141+
}
142+
}
143+
return cachedFilter;
144+
}
145+
111146
/**
112147
* Constructor used for de-serialization
113148
*/
@@ -145,12 +180,8 @@ public Object getAttribute(String name) {
145180
oos.writeObject(obj);
146181
oos.close();
147182

148-
// Create filter from user configuration for secure deserialization
149-
String filterPattern = getServletContext()
150-
.getInitParameter("serializable-object-filter");
151-
ObjectInputFilter filter = filterPattern != null
152-
? ObjectInputFilter.Config.createFilter(filterPattern)
153-
: null;
183+
// Get or create cached filter for secure deserialization
184+
ObjectInputFilter filter = getOrCreateFilter();
154185

155186
ObjectInputStream ois = new ClassLoaderObjectInputStream(
156187
new ByteArrayInputStream(baos.toByteArray()), loader, filter);

extensions/geode-modules/src/main/java/org/apache/geode/modules/util/ClassLoaderObjectInputStream.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@ public ClassLoaderObjectInputStream(InputStream in, ClassLoader loader, ObjectIn
4040
throws IOException {
4141
super(in);
4242
this.loader = loader;
43-
setObjectInputFilter(filter);
43+
if (filter != null) {
44+
setObjectInputFilter(filter);
45+
}
4446
}
4547

4648
/**

0 commit comments

Comments
 (0)