Skip to content

Commit 6a29484

Browse files
mbastianclaude
mbastian
and
claude
authored
Harden serialization: fail on unknown tags, fix char encoding (#284)
* Harden serialization: fail on unknown tags, fix char encoding Three scoped fixes to the serialization layer. No change to the on-disk format, so no VERSION bump. - deserialize() silently returned null for an unrecognized type tag, surfacing later as a confusing ClassCastException or silent data loss. It now throws IOException naming the tag. The preceding `case -1` was unreachable (readUnsignedByte never returns -1) and is removed. - CHAR and CHAR_ARRAY relied on DataOutput.writeChar/readChar, but DataInputOutput implements those with 4 bytes instead of the 2 the interface specifies. Production writes via DataOutputStream, so no stored data is affected, but graphstore's own tests were round-tripping an encoding that never reaches disk. Both sides now use writeShort/readUnsignedShort, which is byte-identical to DataOutputStream.writeChar, and DataInputOutput.writeChar/readChar are fixed to honour the contract. - Dropped Locale support. Locale is not an AttributeUtils supported type, so it cannot enter a graph through the public API. Tag 124 is kept reserved so it is never reused. Adds a test asserting all serialization tag constants are distinct. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Cover non-ASCII chars in serialization tests The char tests only used ASCII values, so they could not detect a narrowing of the 2-byte encoding. Extend them across the boundaries of the 16-bit range: above 0x7F, above 0x7FF, either side of the signed-short flip, the 16-bit maximum, and an unpaired surrogate. Verified by temporarily narrowing CHAR to a symmetric 1-byte encoding, which the previous values did not catch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent f46c641 commit 6a29484

3 files changed

Lines changed: 40 additions & 30 deletions

File tree

‎src/main/java/org/gephi/graph/impl/Serialization.java‎

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@
5656
import java.time.ZoneId;
5757
import java.util.Date;
5858
import java.util.List;
59-
import java.util.Locale;
6059
import java.util.Map;
6160
import java.util.Set;
6261
import org.gephi.graph.api.Configuration;
@@ -178,6 +177,8 @@ public class Serialization {
178177
final static int STRING_EMPTY = 101;
179178
final static int NOTUSED_STRING_255 = 102;
180179
final static int STRING = 103;
180+
// Reserved, do not reuse: was java.util.Locale, removed because Locale isn't an
181+
// AttributeUtils supported type. Kept so old streams can still be identified.
181182
final static int LOCALE = 124;
182183
final static int PROPERTIES = 125;
183184
final static int CLASS = 126;
@@ -1470,7 +1471,9 @@ protected void serialize(final DataOutput out, final Object obj) throws IOExcept
14701471
}
14711472
} else if (clazz == Character.class) {
14721473
out.write(CHAR);
1473-
out.writeChar((Character) obj);
1474+
// Write as 2-byte short so the encoding doesn't depend on the DataOutput
1475+
// implementation. Byte-identical to DataOutputStream.writeChar().
1476+
out.writeShort((Character) obj);
14741477

14751478
} else if (clazz == String.class) {
14761479
String s = (String) obj;
@@ -1520,7 +1523,8 @@ protected void serialize(final DataOutput out, final Object obj) throws IOExcept
15201523
char[] a = (char[]) obj;
15211524
LongPacker.packInt(out, a.length);
15221525
for (char s : a) {
1523-
out.writeChar(s);
1526+
// See CHAR above: 2-byte encoding, independent of the DataOutput impl.
1527+
out.writeShort(s);
15241528
}
15251529
} else if (obj instanceof byte[]) {
15261530
byte[] b = (byte[]) obj;
@@ -1531,12 +1535,6 @@ protected void serialize(final DataOutput out, final Object obj) throws IOExcept
15311535
out.write(DATE);
15321536
out.writeLong(((Date) obj).getTime());
15331537

1534-
} else if (clazz == Locale.class) {
1535-
out.write(LOCALE);
1536-
Locale l = (Locale) obj;
1537-
out.writeUTF(l.getLanguage());
1538-
out.writeUTF(l.getCountry());
1539-
out.writeUTF(l.getVariant());
15401538
} else if (obj instanceof String[]) {
15411539
String[] b = (String[]) obj;
15421540
out.write(STRING_ARRAY);
@@ -2023,11 +2021,11 @@ protected Object deserialize(DataInput is) throws IOException, ClassNotFoundExce
20232021
size = LongPacker.unpackInt(is);
20242022
ret = new char[size];
20252023
for (int i = 0; i < size; i++) {
2026-
((char[]) ret)[i] = is.readChar();
2024+
((char[]) ret)[i] = (char) is.readUnsignedShort();
20272025
}
20282026
break;
20292027
case CHAR:
2030-
ret = is.readChar();
2028+
ret = Character.valueOf((char) is.readUnsignedShort());
20312029
break;
20322030
case FLOAT_MINUS_1:
20332031
ret = Float.valueOf(-1);
@@ -2116,9 +2114,6 @@ protected Object deserialize(DataInput is) throws IOException, ClassNotFoundExce
21162114
case ARRAY_BYTE_INT:
21172115
ret = deserializeArrayByteInt(is);
21182116
break;
2119-
case LOCALE:
2120-
ret = new Locale(is.readUTF(), is.readUTF(), is.readUTF());
2121-
break;
21222117
case STRING_ARRAY:
21232118
ret = deserializeStringArray(is);
21242119
break;
@@ -2224,9 +2219,8 @@ protected Object deserialize(DataInput is) throws IOException, ClassNotFoundExce
22242219
case INSTANT:
22252220
ret = deserializeInstant(is);
22262221
break;
2227-
case -1:
2228-
throw new EOFException();
2229-
2222+
default:
2223+
throw new IOException("Unknown serialization type tag: " + head);
22302224
}
22312225
return ret;
22322226
}

‎src/main/java/org/gephi/graph/impl/utils/DataInputOutput.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ public int readUnsignedShort() throws IOException {
124124

125125
@Override
126126
public char readChar() throws IOException {
127-
return (char) readInt();
127+
return (char) readUnsignedShort();
128128
}
129129

130130
@Override
@@ -209,7 +209,7 @@ public void writeShort(int v) throws IOException {
209209

210210
@Override
211211
public void writeChar(int v) throws IOException {
212-
writeInt(v);
212+
writeShort(v);
213213
}
214214

215215
@Override

‎src/test/java/org/gephi/graph/impl/SerializationTest.java‎

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@
4242
import it.unimi.dsi.fastutil.shorts.ShortOpenHashSet;
4343
import java.io.DataOutput;
4444
import java.io.IOException;
45+
import java.lang.reflect.Field;
46+
import java.lang.reflect.Modifier;
4547
import java.math.BigDecimal;
4648
import java.math.BigInteger;
4749
import java.time.Instant;
@@ -53,7 +55,6 @@
5355
import java.util.HashMap;
5456
import java.util.HashSet;
5557
import java.util.List;
56-
import java.util.Locale;
5758
import java.util.Map;
5859
import java.util.Random;
5960
import java.util.Set;
@@ -999,7 +1000,10 @@ public void testFloat() throws IOException, ClassNotFoundException {
9991000
@Test
10001001
public void testChar() throws IOException, ClassNotFoundException {
10011002
Serialization ser = new Serialization(null);
1002-
char[] vals = { 'a', ' ' };
1003+
// Chars are written as a 2-byte short, so cover the boundaries of that
1004+
// range: above 0x7F, above 0x7FF, either side of the signed-short flip,
1005+
// the 16-bit maximum, and an unpaired surrogate
1006+
char[] vals = { 'a', ' ', '\u00e9', '\u4e2d', '\u7fff', '\u8000', '\uffff', '\ud83d' };
10031007
for (char i : vals) {
10041008
byte[] buf = ser.serialize(i);
10051009
Object l2 = ser.deserialize(buf);
@@ -1144,7 +1148,7 @@ public void testByteArray() throws ClassNotFoundException, IOException {
11441148
@Test
11451149
public void testCharArray() throws ClassNotFoundException, IOException {
11461150
Serialization ser = new Serialization(null);
1147-
char[] l = new char[] { '1', 'a', '&' };
1151+
char[] l = new char[] { '1', 'a', '&', '\u00e9', '\u4e2d', '\u8000', '\uffff' };
11481152
Object deserialize = ser.deserialize(ser.serialize(l));
11491153
Assert.assertTrue(Arrays.equals(l, (char[]) deserialize));
11501154
}
@@ -1184,14 +1188,6 @@ public void testBigInteger() throws IOException, ClassNotFoundException {
11841188
Assert.assertEquals(d, ser.deserialize(ser.serialize(d)));
11851189
}
11861190

1187-
@Test
1188-
public void testLocale() throws Exception {
1189-
Serialization ser = new Serialization(null);
1190-
Assert.assertEquals(Locale.FRANCE, ser.deserialize(ser.serialize(Locale.FRANCE)));
1191-
Assert.assertEquals(Locale.CANADA_FRENCH, ser.deserialize(ser.serialize(Locale.CANADA_FRENCH)));
1192-
Assert.assertEquals(Locale.SIMPLIFIED_CHINESE, ser.deserialize(ser.serialize(Locale.SIMPLIFIED_CHINESE)));
1193-
}
1194-
11951191
@Test
11961192
public void testSmallGraphModel() throws Exception {
11971193
GraphModelImpl gm = GraphGenerator.generateSmallGraphStore().graphModel;
@@ -1314,4 +1310,24 @@ public void testBitVectorEqual() throws Exception {
13141310
Assert.assertEquals(bs, deserializedBs);
13151311
}
13161312
}
1313+
1314+
@Test
1315+
public void testSerializationTagsAreUnique() throws Exception {
1316+
// NULL_ID is excluded: it's an idMap sentinel (-1), not a wire tag
1317+
Map<Integer, String> tagsByValue = new HashMap<>();
1318+
for (Field field : Serialization.class.getDeclaredFields()) {
1319+
int modifiers = field.getModifiers();
1320+
if (!Modifier.isStatic(modifiers) || !Modifier.isFinal(modifiers) || field.getType() != int.class) {
1321+
continue;
1322+
}
1323+
String name = field.getName();
1324+
if (name.equals("NULL_ID")) {
1325+
continue;
1326+
}
1327+
field.setAccessible(true);
1328+
int value = field.getInt(null);
1329+
String previous = tagsByValue.put(value, name);
1330+
Assert.assertNull(previous, "Duplicate serialization tag " + value + " shared by " + previous + " and " + name);
1331+
}
1332+
}
13171333
}

0 commit comments

Comments
 (0)