RuedigerMoeller / fast-serialization

FST: fast java serialization drop in-replacement
Apache License 2.0
1.59k stars 247 forks source link

Incorrect serialization of LinkedHashMap returned from writeReplace #204

Closed cesarb closed 7 years ago

cesarb commented 7 years ago

I encountered an issue serializing a data structure which happened to contain a LinkedTreeMap from Gson. LinkedTreeMap is a Map with the following writeReplace method:

  /**
   * If somebody is unlucky enough to have to serialize one of these, serialize
   * it as a LinkedHashMap so that they won't need Gson on the other side to
   * deserialize it. Using serialization defeats our DoS defence, so most apps
   * shouldn't use it.
   */
  private Object writeReplace() throws ObjectStreamException {
    return new LinkedHashMap<K, V>(this);
  }

The LinkedHashMap returned by that method is not serialized correctly; neither its writeObject method nor FSTMapSerializer is called, resulting in corrupted output.

The following testcase reproduces the issue. In the testcase, the deserialized Map is empty, while it should have one entry. I have also seen an ArrayIndexOutOfBoundsException, or even it trying to load a class with a corrupted name, probably because the structure of the serialized data does not match what the deserializer expects for a LinkedHashMap.

import org.junit.Test;
import org.nustaq.serialization.FSTConfiguration;
import org.nustaq.serialization.FSTObjectInput;
import org.nustaq.serialization.FSTObjectOutput;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.ObjectStreamException;
import java.io.Serializable;
import java.util.*;

import static org.junit.Assert.assertEquals;

public class MapWriteReplaceTest {
    @Test
    public void testMapWriteReplace() throws Exception {
        Map<String, Object> map = new MyMap<>();
        map.put("value", 1234.5);

        ByteArrayOutputStream buf = new ByteArrayOutputStream();
        FSTObjectOutput out = new FSTObjectOutput(buf, getTestConfiguration());
        out.writeObject(map);
        out.close();

        FSTObjectInput in = getTestConfiguration().getObjectInput(new ByteArrayInputStream(buf.toByteArray()));
        Map<?, ?> res = (Map<?, ?>) in.readObject();
        in.close();

        assertEquals(LinkedHashMap.class, res.getClass());
        assertEquals(1, res.size());
        assertEquals(1234.5, res.get("value"));
    }

    private FSTConfiguration getTestConfiguration() {
        return FSTConfiguration.createDefaultConfiguration();
    }

    public static class MyMap<K, V> implements Map<K, V>, Serializable {
        private final Map<K, V> data;

        public MyMap() {
            this(new HashMap<>());
        }

        public MyMap(Map<K, V> data) {
            this.data = data;
        }

        @Override
        public int size() {
            return data.size();
        }

        @Override
        public boolean isEmpty() {
            return data.isEmpty();
        }

        @Override
        public boolean containsKey(Object key) {
            return data.containsKey(key);
        }

        @Override
        public boolean containsValue(Object value) {
            return data.containsValue(value);
        }

        @Override
        public V get(Object key) {
            return data.get(key);
        }

        @Override
        public V put(K key, V value) {
            return data.put(key, value);
        }

        @Override
        public V remove(Object key) {
            return data.remove(key);
        }

        @Override
        public void putAll(Map<? extends K, ? extends V> m) {
            data.putAll(m);
        }

        @Override
        public void clear() {
            data.clear();
        }

        @Override
        public Set<K> keySet() {
            return data.keySet();
        }

        @Override
        public Collection<V> values() {
            return data.values();
        }

        @Override
        public Set<Entry<K, V>> entrySet() {
            return data.entrySet();
        }

        @Override
        public boolean equals(Object o) {
            if (this == o) return true;
            if (o == null || getClass() != o.getClass()) return false;
            MyMap<?, ?> myMap = (MyMap<?, ?>) o;
            return Objects.equals(data, myMap.data);
        }

        @Override
        public int hashCode() {
            return Objects.hash(data);
        }

        private Object writeReplace() throws ObjectStreamException {
            return new LinkedHashMap<K, V>(this);
        }
    }
}
RuedigerMoeller commented 7 years ago

Thanks for your report and testcase. writeReplace has many edge cases .. I'll check a soon I find time.

RuedigerMoeller commented 7 years ago

if wrtie replace returns a class which requires a registered serializer writeReplacement fails as there is missing a second lookup for registered serializers in after calling writeReplace.

fixed with 2.52

jovany-wang commented 5 years ago

+1