[BEANUTILS-417] Refactor serialization methods to conform to Java Object Serialization Specification by rzanner · Pull Request #430 · apache/commons-beanutils

rzanner · 2026-07-27T14:27:09Z

The order of method calls in the serialization methods in {{DynaProperty}} must be changed to conform to the Java Object Serialization Specification. In our case EJB calls in Wildfly failed as the ObjectCloner of Wildfly was checking that the {{in.defaultReadObject}} and {{out.defaultWriteObject}} methods must be called first in the {{Serializable}}'s {{readObject}} and {{writeObject}} methods.

Here are the relevant parts of the Java Object Serialization Specification:

The stacktrace we got from our Wildfly server is as follows:

java.lang.IllegalStateException: defaultWriteObject may not be called in this context
	at org.jboss.marshalling.cloner.SerializingCloner$StepObjectOutputStream.defaultWriteObject(SerializingCloner.java:612)
	at org.apache.commons.beanutils.DynaProperty.writeObject(DynaProperty.java:336)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.jboss.marshalling.reflect.SerializableClass.callWriteObject(SerializableClass.java:271)
	at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:297)
	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:251)
	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:128)
	at org.jboss.marshalling.cloner.SerializingCloner$StepObjectInput.doReadObject(SerializingCloner.java:882)
    ...

Thanks for your contribution to Apache Commons! Your help is appreciated!

Before you push a pull request, review this list:

Read the contribution guidelines for this project.
Read the ASF Generative Tooling Guidance if you use Artificial Intelligence (AI).
I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute?
Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best practice.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process.

…ect Serialization Specification

garydgregory · 2026-07-27T15:31:43Z

@rzanner The build is broken.

Copilot

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Refactors DynaProperty custom Java serialization to call defaultReadObject() / defaultWriteObject() first (per the Java Object Serialization Spec) to avoid WildFly ObjectCloner failures.

Changes:

Reordered DynaProperty.readObject / writeObject to call defaultReadObject() / defaultWriteObject() before custom serialization.
Added tests covering basic Java serialization round-trip and WildFly ObjectCloner cloning.
Added a new test-scoped WildFly dependency and updated .gitignore for IDE metadata.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.

File	Description
src/main/java/org/apache/commons/beanutils/DynaProperty.java	Reorders serialization hooks to satisfy spec/WildFly cloner expectations.
src/test/java/org/apache/commons/beanutils/DynaPropertyTest.java	Adds serialization + WildFly cloner regression tests; adjusts test property type to be indexed (`List`).
pom.xml	Adds a test dependency to obtain the WildFly/JBoss marshalling cloner classes.
.gitignore	Ignores IntelliJ `.iml` files.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

rzanner · 2026-07-28T12:55:53Z

    private void readObject(final ObjectInputStream in) throws IOException, ClassNotFoundException {
+        // read default values
+        in.defaultReadObject();
+
+        // read custom values
        this.type = readAnyClass(in);
        if (isMapped() || isIndexed()) {
            this.contentType = readAnyClass(in);
        }
-        // read other values
-        in.defaultReadObject();
    }


I'll just bump serialVersionUID, as supporting both serialization strategies is non-trivial to impossible.

rzanner · 2026-07-28T12:56:34Z

    private void writeObject(final ObjectOutputStream out) throws IOException {
-        writeAnyClass(this.type,out);
+        // write out default
+        out.defaultWriteObject();
+
+        // write custom values
+        writeAnyClass(this.type, out);
        if (isMapped() || isIndexed()) {
-            writeAnyClass(this.contentType,out);
+            writeAnyClass(this.contentType, out);
        }
-        // write out other values
-        out.defaultWriteObject();
    }


writeObject() wouldn't need to be adapted, as we want to write the new format always.

However, I'll just bump serialVersionUID, as supporting both serialization strategies is non-trivial to impossible.

rzanner · 2026-07-28T12:56:45Z

+    public void testSerialization() throws Exception {
+        ByteArrayOutputStream buffer = new ByteArrayOutputStream();
+        ObjectOutputStream oos = new ObjectOutputStream(buffer);
+        oos.writeObject(testPropertyWithNameAndTypeAndContentType);
+        oos.flush();
+        oos.close();
+
+        ObjectInputStream ois = new ObjectInputStream(new ByteArrayInputStream(buffer.toByteArray()));
+        Object obj = ois.readObject();
+
+        assertEquals(testPropertyWithNameAndTypeAndContentType, obj);
+    }


rzanner · 2026-07-28T13:33:48Z

+    <dependency>
+      <groupId>org.wildfly</groupId>
+      <artifactId>wildfly-ejb3</artifactId>
+      <version>9.0.2.Final</version>
+      <scope>test</scope>
+    </dependency>


garydgregory · 2026-07-27T17:48:21Z

@rzanner
Please see the broken build and the obvious break to the wire format.

rzanner · 2026-07-27T19:17:57Z

Hi @garydgregory,

thanks for the review - will fix the comments that make all sense to me tomorrow.

For the broken build: is there a specific reason why the JBoss repos are blocked?
I hope that after replacing the Wildfly test dependency with the JBoss marshalling one this will be fine, but otherwise I would have to remove the respective dependency along with the test, which I'd rather avoid...

garydgregory · 2026-07-27T19:28:01Z

Hello @rzanner

The build is broken on GitHub and for local builds for me as well. Are you on an intranet that provides its own Maven repository (Nexus or Artifacory for exampel) before hitting the internet? That could explain why it works for you. It could also be that the JARs are blocked due to CVEs.

There is no sense in creating another PR when this one is broken.

…ed versions of DynaProperty

…s-marshalling dependency

rzanner · 2026-07-28T13:35:07Z

Posted some fixes for the dev review comments - I needed to adapt the serialization test cases as the new version is not supporting the old serialization format anymore.

garydgregory · 2026-07-28T14:18:02Z

I needed to adapt the serialization test cases as the new version is not supporting the old serialization format anymore.

I don't think we want to break 1.x call sites. For 2.0, that's OK, not in 1.x. This will just lead to bug reports and unhappy folks. It's OK for a new version to write a new wire format but it should be able to read the old format as well as the new one.

Or, am I missing something?

rzanner · 2026-07-28T15:01:27Z

I don't think we want to break 1.x call sites. For 2.0, that's OK, not in 1.x. This will just lead to bug reports and unhappy folks. It's OK for a new version to write a new wire format but it should be able to read the old format as well as the new one.

Or, am I missing something?

After a whole day of trying around with the serialization methods I see no solution how to support both formats to be read by the same implementation. It's Java serialization, after all, which is based on Object streams which cannot be tried to read and then read again in the correct way. 😐

The 2.x stream doesn't have the serialization methods and therefore also not this problem anymore, which is fine for me. Unfortunately it's not released, yet.
So I'm stuck with my old patched version or I patch the newest 1.x version just for my "private" use. Maybe I have to find a workaround on my application, for instance inheriting from DynaProperty or so...

garydgregory · 2026-07-28T19:01:50Z

How about https://docs.oracle.com/javase/8/docs/platform/serialization/spec/class.html#4100 and something like:

ObjectInputStream in = new ObjectInputStream(fileStream) {
    @Override
    protected ObjectStreamClass readClassDescriptor() throws IOException, ClassNotFoundException {
        ObjectStreamClass desc = super.readClassDescriptor();
        if (desc.getName().equals(MyClass.class.getName())) {
            return ObjectStreamClass.lookup(MyClass.class);
        }
        return desc;
    }
};

?

rzanner and others added 4 commits July 27, 2026 16:19

[BEANUTILS-417] Refactor serialization methods to conform to Java Obj…

de7ae87

…ect Serialization Specification

Merge branch '1.X' into BEANUTILS-417-Serialization

7370240

[BEANUTILS-417] adding *.iml to .gitignore

2ce79e2

[BEANUTILS-417] formatting fixes after merge from 1.x

ceb91fd

garydgregory requested a review from Copilot July 27, 2026 17:29

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Copilot started reviewing on behalf of garydgregory July 27, 2026 18:11 View session

René Zanner added 3 commits July 28, 2026 09:07

Merge branch '1.X' into BEANUTILS-417-Serialization

eb2d854

[BEANUTILS-417] dev review - adding proper support for older serializ…

c8e1ede

…ed versions of DynaProperty

[BEANUTILS-417] dev review - replacing wildfly-ejb3 with minimal jbos…

2c2a6b6

…s-marshalling dependency

garydgregory marked this pull request as draft July 28, 2026 14:18

Uh oh!

Conversation

rzanner commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

garydgregory commented Jul 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

rzanner Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

rzanner Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

rzanner Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

rzanner Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

garydgregory commented Jul 27, 2026

Uh oh!

rzanner commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

garydgregory commented Jul 27, 2026

Uh oh!

rzanner commented Jul 28, 2026

Uh oh!

garydgregory commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rzanner commented Jul 28, 2026

Uh oh!

garydgregory commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rzanner commented Jul 27, 2026 •

edited

Loading

rzanner commented Jul 27, 2026 •

edited

Loading

garydgregory commented Jul 28, 2026 •

edited

Loading

garydgregory commented Jul 28, 2026 •

edited

Loading