apache / dubbo

The java implementation of Apache Dubbo. An RPC and microservice framework.
https://dubbo.apache.org/
Apache License 2.0
40.48k stars 26.43k forks source link

incomplete: Fix hessian2 serialized short, byte is converted to int bug (#1232) #1609

Closed takeseem closed 6 years ago

takeseem commented 6 years ago

1232

It is just supported Hessian2, imcomplete in hessian1.

Example: https://github.com/apache/incubator-dubbo/blob/3d4f2fcdbbc327c2dac872962f65b2f011c7d247/hessian-lite/src/test/java/com/alibaba/com/caucho/hessian/io/Hessian2StringShortTest.java#L47

if add hessian1 test, it will be fail:

    @Test
    public void serialize_string_byte_map_then_deserialize() throws Exception {

        Hessian2StringShortType stringShort = new Hessian2StringShortType();
        Map<String, Byte> stringByteMap = new HashMap<String, Byte>();
        stringByteMap.put("first", (byte)0);
        stringByteMap.put("last", (byte)60);
        stringShort.stringByteMap = stringByteMap;

        Hessian2StringShortType deserialize = baseHession2Serialize(stringShort);
        assertTrue(deserialize.stringByteMap != null);
        assertTrue(deserialize.stringByteMap.size() == 2);
        assertTrue(deserialize.stringByteMap.get("last") instanceof Byte);
        assertEquals(Byte.valueOf((byte)0), deserialize.stringByteMap.get("first"));
        assertEquals(Byte.valueOf((byte) 60), deserialize.stringByteMap.get("last"));

//add hessian1 test, it will be fail
        deserialize = baseHessionSerialize(stringShort);
        assertTrue(deserialize.stringByteMap != null);
        assertTrue(deserialize.stringByteMap.size() == 2);
        assertTrue(deserialize.stringByteMap.get("last") instanceof Byte);
        assertEquals(Byte.valueOf((byte)0), deserialize.stringByteMap.get("first"));
        assertEquals(Byte.valueOf((byte) 60), deserialize.stringByteMap.get("last"));
    }

so my fix push test fail: #1608 JavaDeserializer._constructor.newInstance([null, null]) NPE old issue #210 dubbo调用报错HessianFieldException

zonghaishang commented 6 years ago

The hessian2 protocol has been used by default. Which scenario needs to use hessian1? Please tell me which version you are using.

takeseem commented 6 years ago

dubbo作为一个严谨的框架,既然hessian1未废弃,就应当保持一致性,因为其他人可能在使用。

这是我第3次看到dubbo的master,合并到主干时未考虑兼容性。

建议先完成:功能性审核,再讨论代码样式和规范。

zonghaishang commented 6 years ago

Dubbo does not use the hessain1 protocol. I have fix hessian1 protocol , see: https://github.com/apache/incubator-dubbo/pull/1616

takeseem commented 6 years ago

今天仔细看下代码,hessian1确实没有用了,建议从hessian-lite中移除。 多余的代码需要维护!

这确实是我对dubbo了解的还不够,想当然看到有hessian1就以为还需要兼容。

chickenlj commented 6 years ago

Hi takeseem,

Thanks for your suggestion, also very sorry for the incompatible problems brings to you. We should pay more attention on change compatibilities, especially for legacy systems using old framework/protocol versions. At least, there can be a compatible notification.

chickenlj commented 6 years ago

For hessian1, i also recommend remove from hessian-lite

diecui1202 commented 6 years ago

1616 has been merged. I submit a new issue for hessian-lite #1778

So please close this issue.