Skip to content

Fix 2609 binary userdata lists #2785

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Dingyi189-eng wants to merge 2 commits into
base: master
Choose a base branch
from
+47 −10
Open

Fix 2609 binary userdata lists #2785

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e808169
Fix BinaryExporter deserialization for multiple UserData lists (Fixes…
Dingyi189-eng May 18, 2026
747834c
Fix UserData list field prefixes for binary/XML export (Fixes #2609)
Dingyi189-eng May 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 42 additions & 9 deletions jme3-core/src/main/java/com/jme3/scene/UserData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicLong;

/**
* <code>UserData</code> is used to contain user data objects
Expand Down Expand Up @@ -74,9 +75,15 @@ public final class UserData implements Savable {
private static final int TYPE_SHORT = 10;
private static final int TYPE_BYTE = 11;

private static final AtomicLong uniqueIdCounter = new AtomicLong(0);

private static final String LEGACY_PREFIX = "0";

protected byte type;
protected Object value;

private transient volatile String uniqueId;

public UserData() {
}

Expand Down Expand Up @@ -104,6 +111,17 @@ public String toString() {
return value.toString();
}

/**
* Prefix for list/map/array fields in the export capsule.
* Legacy files use "0" / "1"; new files use "udN_0" / "udN_1" (no ':' for XML).
*/
private String listFieldPrefix(int index) {
if (LEGACY_PREFIX.equals(uniqueId)) {
return String.valueOf(index);
}
return uniqueId + "_" + index;
}

public static byte getObjectType(Object type) {
if (type instanceof Integer) {
return TYPE_INTEGER;
Expand Down Expand Up @@ -138,7 +156,16 @@ public static byte getObjectType(Object type) {
public void write(JmeExporter ex) throws IOException {
OutputCapsule oc = ex.getCapsule(this);
oc.write(type, "type", (byte) 0);


if (uniqueId == null) {
synchronized (this) {
if (uniqueId == null) {
Copy link
Copy Markdown
Contributor

@JNightRider JNightRider May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the double validation; isn't one validation enough?

uniqueId = "ud" + uniqueIdCounter.incrementAndGet();
}
}
}
oc.write(uniqueId, "uniqueId", null);

switch (type) {
case TYPE_INTEGER:
int i = (Integer) value;
Expand All @@ -165,15 +192,15 @@ public void write(JmeExporter ex) throws IOException {
oc.write(sav, "savableVal", null);
break;
case TYPE_LIST:
this.writeList(oc, (List<?>) value, "0");
this.writeList(oc, (List<?>) value, listFieldPrefix(0));
break;
case TYPE_MAP:
Map<?, ?> map = (Map<?, ?>) value;
this.writeList(oc, map.keySet(), "0");
this.writeList(oc, map.values(), "1");
this.writeList(oc, map.keySet(), listFieldPrefix(0));
this.writeList(oc, map.values(), listFieldPrefix(1));
break;
case TYPE_ARRAY:
this.writeList(oc, Arrays.asList((Object[]) value), "0");
this.writeList(oc, Arrays.asList((Object[]) value), listFieldPrefix(0));
break;
case TYPE_DOUBLE:
Double d = (Double) value;
Expand All @@ -196,6 +223,12 @@ public void write(JmeExporter ex) throws IOException {
public void read(JmeImporter im) throws IOException {
InputCapsule ic = im.getCapsule(this);
type = ic.readByte("type", (byte) 0);

uniqueId = ic.readString("uniqueId", null);
if (uniqueId == null) {
uniqueId = LEGACY_PREFIX;
}

switch (type) {
case TYPE_INTEGER:
value = ic.readInt("intVal", 0);
Expand All @@ -216,19 +249,19 @@ public void read(JmeImporter im) throws IOException {
value = ic.readSavable("savableVal", null);
break;
case TYPE_LIST:
value = this.readList(ic, "0");
value = this.readList(ic, listFieldPrefix(0));
break;
case TYPE_MAP:
Map<Object, Object> map = new HashMap<>();
List<?> keys = this.readList(ic, "0");
List<?> values = this.readList(ic, "1");
List<?> keys = this.readList(ic, listFieldPrefix(0));
List<?> values = this.readList(ic, listFieldPrefix(1));
for (int i = 0; i < keys.size(); ++i) {
map.put(keys.get(i), values.get(i));
}
value = map;
break;
case TYPE_ARRAY:
value = this.readList(ic, "0").toArray();
value = this.readList(ic, listFieldPrefix(0)).toArray();
break;
case TYPE_DOUBLE:
value = ic.readDouble("doubleVal", 0.);
Expand Down
4 changes: 4 additions & 0 deletions jme3-core/src/plugins/java/com/jme3/export/binary/BinaryInputCapsule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -959,6 +959,10 @@ protected long readLong(byte[] content) throws IOException {
}
bytes = ByteUtils.rightAlignBytes(bytes, 8);
long value = ByteUtils.convertLongFromBytes(bytes);
if (value == BinaryOutputCapsule.NULL_OBJECT
|| value == BinaryOutputCapsule.DEFAULT_OBJECT) {
index -= 4;
}
Comment on lines +962 to +965
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This change introduces a critical bug that will lead to stream corruption when reading long values of -1 or -2.

The index -= 4 logic is intended to compensate for the way BinaryOutputCapsule.deflate handles int values of -1 (NULL_OBJECT) or -2 (DEFAULT_OBJECT) by converting them into a single-byte marker. However, deflate only applies this special handling to 4-byte values (integers). For 8-byte long values, it writes the full (or partially deflated) bytes without using the magic markers.

Consequently, when readLong encounters a full 8-byte -1L, it will correctly increment the index by 9 bytes (1 length byte + 8 data bytes), but then this new code will incorrectly backtrack the index by 4 bytes. This causes the next read operation to start in the middle of the previous long value, corrupting the entire import process.

References
  1. Avoid using magic numbers (e.g., -1) as markers or default values in serialization to detect missing keys or special states, as this can lead to stream corruption or mask bugs.

Copy link
Copy Markdown
Contributor

@JNightRider JNightRider May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested these changes without this code snippet; the test runs without issues, so I suspect it's not related to the problem.

return value;
}

Expand Down
2 changes: 1 addition & 1 deletion jme3-plugins/src/test/java/com/jme3/export/JmeExporterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public void testSaveWithNullParent(JmeExporter exporter) throws IOException {
public void testExporterConsistency(JmeExporter currentExporter) {
//
final boolean testXML = true;
final boolean testLists = false;
final boolean testLists = true;
final boolean testMaps = true;
final boolean printXML = false;

Expand Down