Skip to content
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.

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

serialize/deserialize field with default value mistake #851

Open
KolorYan opened this issue Aug 18, 2021 · 6 comments · May be fixed by #854
Open

serialize/deserialize field with default value mistake #851

KolorYan opened this issue Aug 18, 2021 · 6 comments · May be fixed by #854
Labels

Comments

@KolorYan
Copy link

KolorYan commented Aug 18, 2021

Describe the bug

    public static void main(String[] args) {

        A param = new  A();
        param.setI(null);
        System.out.println(param);
        // console print "null"

        Kryo kryo = new Kryo();
        kryo.setRegistrationRequired(false);

        kryo.setDefaultSerializer(CompatibleFieldSerializer.class);

        Output output = getOutput();
        kryo.writeClassAndObject(output, param);

        param = (A) kryo.readClassAndObject(new Input(output.toBytes()));

        System.out.println(param);
        // console print 10
    }

    public static class A {
        private Integer i = 10;

        public Integer getI() {
            return i;
        }

        public void setI(Integer i) {
            this.i = i;
        }
        
        @Override
        public String toString() {
            return String.valueOf(i);
        }

    }

Environment:

  • Kryo Version: 5.2.0
@KolorYan
Copy link
Author

before serialize, I change field value from default value 10 to null
but after deserialize, the field value is still 10

so null field is not serialized?

@KolorYan
Copy link
Author

KolorYan commented Aug 18, 2021

I fix it by myself now, waiting for new version release

package com.esotericsoftware.kryo.serializers;

import static com.esotericsoftware.kryo.util.Util.className;
import static com.esotericsoftware.kryo.util.Util.pos;
import static com.esotericsoftware.minlog.Log.DEBUG;
import static com.esotericsoftware.minlog.Log.TRACE;
import static com.esotericsoftware.minlog.Log.debug;
import static com.esotericsoftware.minlog.Log.trace;

import com.esotericsoftware.kryo.Kryo;
import com.esotericsoftware.kryo.KryoException;
import com.esotericsoftware.kryo.Registration;
import com.esotericsoftware.kryo.io.Input;
import com.esotericsoftware.kryo.io.InputChunked;
import com.esotericsoftware.kryo.util.Util;

/**
 * 解决KRYO <= 5.2.0原生实现针对存在默认值字段,如果设值为null时,反序列化后依旧为默认值问题</br>
 * 
 * @see https://github.com/EsotericSoftware/kryo/issues/851
 * 
 * @author Kolor
 * @date 2021-8-18 12:10:11
 * @param <T>
 */
@SuppressWarnings("rawtypes")
public class DefaultValueSetNullSupportCompatibleFieldSerializer<T> extends CompatibleFieldSerializer<T> {
    private static final int                      binarySearchThreshold = 32;
    private final CompatibleFieldSerializerConfig config;

    public DefaultValueSetNullSupportCompatibleFieldSerializer(Kryo kryo, Class type) {
        this(kryo, type, new CompatibleFieldSerializerConfig());
    }

    public DefaultValueSetNullSupportCompatibleFieldSerializer(Kryo kryo, Class type, CompatibleFieldSerializerConfig config) {
        super(kryo, type, config);
        this.config = config;
    }

    private void setNullValue(CachedField cachedField, Object obj) {
        if (!cachedField.getField().getType().isPrimitive()) {
            try {
                cachedField.getField().set(obj, null);
            } catch (IllegalArgumentException | IllegalAccessException e) {
                throw new RuntimeException(e);
            }
        }
    }

    public T read(Kryo kryo, Input input, Class<? extends T> type) {
        int pop = pushTypeVariables();

        T object = create(kryo, input, type);
        kryo.reference(object);

        CachedField[] fields = (CachedField[]) kryo.getGraphContext().get(this);
        if (fields == null) fields = readFields(kryo, input);

        boolean chunked = config.chunked, readUnknownTagData = config.readUnknownFieldData;
        Input fieldInput;
        InputChunked inputChunked = null;
        if (chunked)
            fieldInput = inputChunked = new InputChunked(input, config.chunkSize);
        else
            fieldInput = input;
        for (int i = 0, n = fields.length; i < n; i++) {
            CachedField cachedField = fields[i];

            if (readUnknownTagData) {
                Registration registration;
                try {
                    registration = kryo.readClass(fieldInput);
                } catch (KryoException ex) {
                    String message = "Unable to read unknown data (unknown type). (" + getType().getName() + "#" + cachedField + ")";
                    if (!chunked) throw new KryoException(message, ex);
                    if (DEBUG) debug("kryo", message, ex);
                    inputChunked.nextChunk();
                    continue;
                }
                if (registration == null) {
                    // 修复存在默认值时,而当前值为null时,未设置的问题
                    setNullValue(cachedField, object);

                    if (chunked) inputChunked.nextChunk();
                    continue;
                }
                Class valueClass = registration.getType();
                if (cachedField == null) {
                    // Read unknown data in case it is a reference.
                    if (TRACE) trace("kryo", "Read unknown data, type: " + className(valueClass) + pos(input.position()));
                    try {
                        kryo.readObject(fieldInput, valueClass);
                    } catch (KryoException ex) {
                        String message = "Unable to read unknown data, type: " + className(valueClass) + " (" + getType().getName()
                                + "#" + cachedField + ")";
                        if (!chunked) throw new KryoException(message, ex);
                        if (DEBUG) debug("kryo", message, ex);
                    }
                    if (chunked) inputChunked.nextChunk();
                    continue;
                }

                // Ensure the type in the data is compatible with the field type.
                if (cachedField.valueClass != null && !Util.isAssignableTo(valueClass, cachedField.field.getType())) {
                    String message = "Read type is incompatible with the field type: " + className(valueClass) + " -> "
                            + className(cachedField.valueClass) + " (" + getType().getName() + "#" + cachedField + ")";
                    if (!chunked) throw new KryoException(message);
                    if (DEBUG) debug("kryo", message);
                    inputChunked.nextChunk();
                    continue;
                }

                cachedField.setCanBeNull(false);
                cachedField.setValueClass(valueClass);
                cachedField.setReuseSerializer(false);
            } else if (cachedField == null) {
                if (!chunked) throw new KryoException("Unknown field. (" + getType().getName() + ")");
                if (TRACE) trace("kryo", "Skip unknown field.");
                inputChunked.nextChunk();
                continue;
            }

            if (TRACE) log("Read", cachedField, input.position());
            cachedField.read(fieldInput, object);
            if (chunked) inputChunked.nextChunk();
        }

        popTypeVariables(pop);
        return object;
    }

    private CachedField[] readFields(Kryo kryo, Input input) {
        if (TRACE) trace("kryo", "Read fields for class: " + type.getName());

        int length = input.readVarInt(true);
        String[] names = new String[length];
        for (int i = 0; i < length; i++) {
            names[i] = input.readString();
            if (TRACE) trace("kryo", "Read field name: " + names[i]);
        }

        CachedField[] fields = new CachedField[length];
        CachedField[] allFields = cachedFields.fields;
        if (length < binarySearchThreshold) {
            outer: for (int i = 0; i < length; i++) {
                String schemaName = names[i];
                for (int ii = 0, nn = allFields.length; ii < nn; ii++) {
                    if (allFields[ii].name.equals(schemaName)) {
                        fields[i] = allFields[ii];
                        continue outer;
                    }
                }
                if (TRACE) trace("kryo", "Unknown field will be skipped: " + schemaName);
            }
        } else {
            int low, mid, high, compare;
            int lastFieldIndex = allFields.length - 1;
            outer: for (int i = 0; i < length; i++) {
                String schemaName = names[i];
                low = 0;
                high = lastFieldIndex;
                while (low <= high) {
                    mid = (low + high) >>> 1;
                    compare = schemaName.compareTo(allFields[mid].name);
                    if (compare < 0)
                        high = mid - 1;
                    else if (compare > 0)
                        low = mid + 1;
                    else {
                        fields[i] = allFields[mid];
                        continue outer;
                    }
                }
                if (TRACE) trace("kryo", "Unknown field will be skipped: " + schemaName);
            }
        }

        kryo.getGraphContext().put(this, fields);
        return fields;
    }

}

@theigl
Copy link
Collaborator

theigl commented Aug 18, 2021

@KolorYan: Thanks for the report. Please improve the formatting of your ticket. It is difficult to see what's going on at the moment.

@KolorYan
Copy link
Author

KolorYan commented Aug 31, 2021

the code need check cachedField is null

if (registration == null) {
	// 修复存在默认值时,而当前值为null时,未设置的问题
	if (cachedField != null) {
		setNullValue(cachedField, object);
	}

	if (chunked) inputChunked.nextChunk();
	continue;
}

@theigl theigl removed the needs repro label Sep 3, 2021
@theigl
Copy link
Collaborator

theigl commented Sep 3, 2021

@KolorYan: I can reproduce the issue. This does indeed look like a bug. Your fix works without breaking any tests, but I'm not sure if it causes any unintended side-effects. I'll have to think about this some more.

@elw00d
Copy link

elw00d commented Jul 13, 2023

Workaround for this bug: use StdInstantiatorStrategy. It doesn't call constructors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 participants