You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by "JinwooHwang (via GitHub)" <gi...@apache.org> on 2026/10/14 17:18:41 UTC
[PR] [GEODE-10676] Refactor locator request read path [geode]
JinwooHwang opened a new pull request, #8111:
URL: https://github.com/apache/geode/pull/8111
<!-- Thank you for submitting a contribution to Apache Geode. -->
Refactor locator request read path
<!-- In order to streamline review of your contribution we ask that you
ensure you've taken the following steps. -->
### For all changes, please confirm:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
- [x] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?
- [x] Is your initial contribution a single, squashed commit?
- [x] Does `gradlew build` run cleanly?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GEODE-10676] Refactor locator request read path [geode]
Posted by "JinwooHwang (via GitHub)" <gi...@apache.org>.
JinwooHwang merged PR #8111:
URL: https://github.com/apache/geode/pull/8111
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GEODE-10676] Refactor locator request read path [geode]
Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #8111:
URL: https://github.com/apache/geode/pull/8111#discussion_r4274165023
##########
geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java:
##########
@@ -2371,6 +2373,124 @@
}
}
+ /**
+ * Reads an object from a stream that an unauthenticated peer may control, reconstructing only the
+ * types the caller expects. A {@link DataSerializableFixedID} (whose wire form names a registered
+ * fixed id rather than a class) is read normally. An object written with an explicit class name
+ * is
+ * reconstructed only when {@code classIsExpected} accepts that name; for data-serialized objects
+ * the name is checked before the class is loaded. Any other wire form, including a
+ * Java-serialized
+ * object of an unexpected type, is rejected with an {@link IOException}.
+ */
+ public static Object readRequestObject(final DataInput in,
+ final Predicate<String> classIsExpected) throws IOException, ClassNotFoundException {
+ checkIn(in);
+ byte header = in.readByte();
+ DSCODE headerDSCode = DscodeHelper.toDSCODE(header);
+ if (headerDSCode == null) {
+ throw new IOException("Unknown header byte: " + header);
+ }
+ switch (headerDSCode) {
+ case DS_FIXED_ID_BYTE:
+ return dsfidFactory.create(in.readByte(), in);
+ case DS_FIXED_ID_SHORT:
+ return dsfidFactory.create(in.readShort(), in);
+ case DS_FIXED_ID_INT:
+ return dsfidFactory.create(in.readInt(), in);
+ case DS_NO_FIXED_ID:
+ case DATA_SERIALIZABLE:
+ return readDataSerializable(in, classIsExpected);
+ case SERIALIZABLE:
+ return readSerializable(in, classIsExpected);
+ case NULL:
+ case NULL_STRING:
+ return null;
+ default:
+ throw new IOException("Unexpected request format: " + headerDSCode);
+ }
+ }
+
+ private static Object readDataSerializable(final DataInput in,
+ final Predicate<String> classIsExpected) throws IOException, ClassNotFoundException {
+ final Class<?> c = readExpectedClass(in, classIsExpected);
+ try {
+ Constructor<?> init = c.getConstructor();
+ init.setAccessible(true);
+ Object o = init.newInstance();
+
+ invokeFromData(o, in);
+
+ return o;
+ } catch (EOFException | SocketException ex) {
+ // client went away - ignore
+ throw ex;
+ } catch (Exception ex) {
+ throw new SerializationException(
+ String.format("Could not create an instance of %s .",
+ c.getName()),
+ ex);
+ }
+ }
+
+ /**
+ * Reads a class written by {@link DataSerializer#writeClass}, resolving it only when
+ * {@code classIsExpected} accepts its name. The name is checked before the class is loaded, so an
+ * unexpected name is rejected without running that class's static initializer. Primitive type
+ * codes carry no class name and are resolved directly.
+ */
+ private static Class<?> readExpectedClass(final DataInput in,
+ final Predicate<String> classIsExpected) throws IOException, ClassNotFoundException {
+ byte typeCode = in.readByte();
+ if (typeCode == DSCODE.CLASS.toByte()) {
+ String className = processIncomingClassName(readString(in));
+ if (!classIsExpected.test(className)) {
+ throw new IOException("Unexpected class name: " + className);
+ }
+ return getCachedClass(className);
+ }
+ return StaticSerialization.decodePrimitiveClass(typeCode);
+ }
+
+ private static Serializable readSerializable(final DataInput in,
+ final Predicate<String> classIsExpected) throws IOException, ClassNotFoundException {
+ InputStream stream;
+ if (in instanceof InputStream) {
+ stream = (InputStream) in;
+ } else {
+ stream = new InputStream() {
+ @Override
+ public int read() throws IOException {
+ try {
+ return in.readUnsignedByte();
+ } catch (EOFException ignored) {
+ return -1;
+ }
+ }
+ };
+ }
+
+ ObjectInput ois = new DSObjectInputStream(stream);
+ ((ObjectInputStream) ois).setObjectInputFilter(filterInfo -> {
+ Class<?> serialClass = filterInfo.serialClass();
+ if (serialClass == null) {
+ return ObjectInputFilter.Status.UNDECIDED;
+ }
+ return classIsExpected.test(serialClass.getName())
+ ? ObjectInputFilter.Status.ALLOWED
+ : ObjectInputFilter.Status.REJECTED;
+ });
+
+ if (stream instanceof VersionedDataStream) {
+ KnownVersion v = ((VersionedDataStream) stream).getVersion();
+ if (KnownVersion.CURRENT != v && v != null) {
+ ois = new VersionedObjectInput(ois, v);
+ }
+ }
+
+ return (Serializable) ois.readObject();
Review Comment:
## CodeQL / Deserialization of user-controlled data
Unsafe deserialization depends on a [user-provided value](1).
Unsafe deserialization depends on a [user-provided value](2).
Unsafe deserialization depends on a [user-provided value](3).
Unsafe deserialization depends on a [user-provided value](4).
Unsafe deserialization depends on a [user-provided value](5).
Unsafe deserialization depends on a [user-provided value](6).
Unsafe deserialization depends on a [user-provided value](7).
Unsafe deserialization depends on a [user-provided value](8).
[Show more details](https://github.com/apache/geode/security/code-scanning/147)
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GEODE-10676] Refactor locator request read path [geode]
Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #8111:
URL: https://github.com/apache/geode/pull/8111#discussion_r4275398805
##########
geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java:
##########
@@ -2371,6 +2373,141 @@
}
}
+ /**
+ * Reads an object from a stream that an unauthenticated peer may control, reconstructing only the
+ * types the caller expects. A {@link DataSerializableFixedID} (whose wire form names a registered
+ * fixed id rather than a class) is read normally. An object written with an explicit class name
+ * is
+ * reconstructed only when {@code classIsExpected} accepts that name; for data-serialized objects
+ * the name is checked before the class is loaded. Any other wire form, including a
+ * Java-serialized
+ * object of an unexpected type, is rejected with an {@link IOException}.
+ */
+ public static Object readRequestObject(final DataInput in,
+ final Predicate<String> classIsExpected) throws IOException, ClassNotFoundException {
+ checkIn(in);
+ byte header = in.readByte();
+ DSCODE headerDSCode = DscodeHelper.toDSCODE(header);
+ if (headerDSCode == null) {
+ throw new IOException("Unknown header byte: " + header);
+ }
+ switch (headerDSCode) {
+ case DS_FIXED_ID_BYTE:
+ return dsfidFactory.create(in.readByte(), in);
+ case DS_FIXED_ID_SHORT:
+ return dsfidFactory.create(in.readShort(), in);
+ case DS_FIXED_ID_INT:
+ return dsfidFactory.create(in.readInt(), in);
+ case DS_NO_FIXED_ID:
+ case DATA_SERIALIZABLE:
+ return readDataSerializable(in, classIsExpected);
+ case SERIALIZABLE:
+ return readSerializable(in, classIsExpected);
+ case NULL:
+ case NULL_STRING:
+ return null;
+ default:
+ throw new IOException("Unexpected request format: " + headerDSCode);
+ }
+ }
+
+ private static Object readDataSerializable(final DataInput in,
+ final Predicate<String> classIsExpected) throws IOException, ClassNotFoundException {
+ final Class<?> c = readExpectedClass(in, classIsExpected);
+ try {
+ Constructor<?> init = c.getConstructor();
+ init.setAccessible(true);
+ Object o = init.newInstance();
+
+ invokeFromData(o, in);
+
+ return o;
+ } catch (EOFException | SocketException ex) {
+ // client went away - ignore
+ throw ex;
+ } catch (Exception ex) {
+ throw new SerializationException(
+ String.format("Could not create an instance of %s .",
+ c.getName()),
+ ex);
+ }
+ }
+
+ /**
+ * Reads a class written by {@link DataSerializer#writeClass}, resolving it only when
+ * {@code classIsExpected} accepts its name. The name is checked before the class is loaded, so an
+ * unexpected name is rejected without running that class's static initializer. Primitive type
+ * codes carry no class name and are resolved directly.
+ */
+ private static Class<?> readExpectedClass(final DataInput in,
+ final Predicate<String> classIsExpected) throws IOException, ClassNotFoundException {
+ byte typeCode = in.readByte();
+ if (typeCode == DSCODE.CLASS.toByte()) {
+ String className = processIncomingClassName(readString(in));
+ if (!classIsExpected.test(className)) {
+ throw new IOException("Unexpected class name: " + className);
+ }
+ return getCachedClass(className);
+ }
+ return StaticSerialization.decodePrimitiveClass(typeCode);
+ }
+
+ private static Serializable readSerializable(final DataInput in,
+ final Predicate<String> classIsExpected) throws IOException, ClassNotFoundException {
+ InputStream stream;
+ if (in instanceof InputStream) {
+ stream = (InputStream) in;
+ } else {
+ stream = new InputStream() {
+ @Override
+ public int read() throws IOException {
+ try {
+ return in.readUnsignedByte();
+ } catch (EOFException ignored) {
+ return -1;
+ }
+ }
+ };
+ }
+
+ ObjectInput ois = new ExpectedClassObjectInputStream(stream, classIsExpected);
+
+ if (stream instanceof VersionedDataStream) {
+ KnownVersion v = ((VersionedDataStream) stream).getVersion();
+ if (KnownVersion.CURRENT != v && v != null) {
+ ois = new VersionedObjectInput(ois, v);
+ }
+ }
+
+ return (Serializable) ois.readObject();
Review Comment:
## CodeQL / Deserialization of user-controlled data
Unsafe deserialization depends on a [user-provided value](1).
Unsafe deserialization depends on a [user-provided value](2).
Unsafe deserialization depends on a [user-provided value](3).
Unsafe deserialization depends on a [user-provided value](4).
Unsafe deserialization depends on a [user-provided value](5).
Unsafe deserialization depends on a [user-provided value](6).
Unsafe deserialization depends on a [user-provided value](7).
Unsafe deserialization depends on a [user-provided value](8).
[Show more details](https://github.com/apache/geode/security/code-scanning/148)
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org