Fix hessian2 serialized short, byte is converted to int bug by zonghaishang · Pull Request #1232 · apache/dubbo

zonghaishang · 2018-01-17T12:09:53Z

Fix hessian2 serialized short, byte is converted to int bug

for example:
if we test deserialize mode 'Hessian2StringShortType'
`
public class Hessian2StringShortType implements Serializable {

Map<String, Short> stringShortMap;

Map<String, Byte> stringByteMap;

Map<String, PersonType> stringPersonTypeMap;

public Hessian2StringShortType(){

}

}

public class PersonType implements Serializable {

String name;
int age;
double money;
short p1;
byte  p2;
List<Short> p3;

public PersonType(String name, int age, double money, short p1, byte p2, List<Short> p3) {
    this.name = name;
    this.age = age;
    this.money = money;
    this.p1 = p1;
    this.p2 = p2;
    this.p3 = p3;
}

@Override
public boolean equals(Object o) {
    if (this == o)
        return true;
    if (o == null || getClass() != o.getClass())
        return false;
    PersonType type = (PersonType) o;
    return age == type.age &&
        Double.compare(type.money, money) == 0 &&
        p1 == type.p1 &&
        p2 == type.p2 &&
        Objects.equals(name, type.name) &&
        Objects.equals(p3, type.p3);
}

@Override
public int hashCode() {
    return Objects.hash(name, age, money, p1, p2, p3);
}

}

@test
public void serialize_string_short_map_then_deserialize() throws Exception {

    Hessian2StringShortType stringShort = new Hessian2StringShortType();
    Map<String, Short> stringShortMap = new HashMap<String, Short>();
    stringShortMap.put("first", (short)0);
    stringShortMap.put("last", (short)60);
    stringShort.stringShortMap = stringShortMap;

    ByteArrayOutputStream bout = new ByteArrayOutputStream();
    Hessian2Output out = new Hessian2Output(bout);

    out.writeObject(stringShort);
    out.flush();

    ByteArrayInputStream bin = new ByteArrayInputStream(bout.toByteArray());
    Hessian2Input input = new Hessian2Input(bin);

    Hessian2StringShortType deserialize = (Hessian2StringShortType) input.readObject();
    assertTrue(deserialize.stringShortMap != null);
    assertTrue(deserialize.stringShortMap.size() == 2);
    // however, the deserialize result is Integer !!!
    assertTrue(deserialize.stringShortMap.get("last") instanceof Short);
    assertEquals(Short.valueOf((short)0), deserialize.stringShortMap.get("first"));
    assertEquals(Short.valueOf((short)60), deserialize.stringShortMap.get("last"));
}

`
When we test serialize and deserialize 'Hessian2StringShortType', because it contains short、byte,
the property stringShortMap will be deserialized Map<String, Integer>, not Map<String, Short>!!!

More detail unit test can see: com.alibaba.com.caucho.hessian.io.Hessian2StringShortTest

The problem has been fixed in the current branch, please verify and merge, Thank you.
Here is my email: shangzonghai@youzan.com or jason.shang@hotmail.com

codecov-io · 2018-01-18T00:43:53Z

Codecov Report

Merging #1232 into master will increase coverage by 0.5%.
The diff coverage is 43.62%.

@@            Coverage Diff            @@
##           master    #1232     +/-   ##
=========================================
+ Coverage   31.78%   32.29%   +0.5%     
=========================================
  Files         689      689             
  Lines       34454    34561    +107     
  Branches     6799     6848     +49     
=========================================
+ Hits        10950    11160    +210     
+ Misses      21591    21466    -125     
- Partials     1913     1935     +22

Impacted Files	Coverage Δ
...ba/com/caucho/hessian/io/AbstractHessianInput.java	`7.69% <0%> (-1.4%)`	⬇️
...ibaba/com/caucho/hessian/io/SerializerFactory.java	`59.43% <0%> (+4.43%)`	⬆️
...ba/com/caucho/hessian/io/AbstractDeserializer.java	`11.76% <20%> (+6.5%)`	⬆️
...m/alibaba/com/caucho/hessian/io/Hessian2Input.java	`17.94% <23.25%> (+2.18%)`	⬆️
.../com/caucho/hessian/io/CollectionDeserializer.java	`32.6% <25%> (+21.49%)`	⬆️
...libaba/com/caucho/hessian/io/JavaDeserializer.java	`53.1% <64.4%> (+16.89%)`	⬆️
...alibaba/com/caucho/hessian/io/MapDeserializer.java	`49.12% <91.66%> (+49.12%)`	⬆️
...om/alibaba/dubbo/rpc/filter/ActiveLimitFilter.java	`77.77% <0%> (-5.56%)`	⬇️
...a/dubbo/remoting/transport/netty/NettyChannel.java	`57.5% <0%> (-3.75%)`	⬇️
.../com/alibaba/dubbo/monitor/dubbo/DubboMonitor.java	`73.14% <0%> (-2.78%)`	⬇️
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 09722e3...efd5cae. Read the comment docs.

chickenlj · 2018-02-05T06:52:34Z

@lovepoem Do you have time to review this PR? I've seen you fixed bugs in hessian before, you must be familiar with hessian code.

lovepoem · 2018-02-05T07:30:17Z

ok I will review this PR

zonghaishang · 2018-02-06T06:42:13Z

@lovepoem Do you have any question about this pr ?

lovepoem

This PR will be merged to 2.5.x at first. please merge 2.5.x to you pr. Then please check the hessian-lite/src/test/java/com/alibaba/com/caucho/hessian/io/PersonType.java , these is a JDK version adaptation assue @zonghaishang

lovepoem · 2018-02-07T11:05:08Z

hessian-lite/src/test/java/com/alibaba/com/caucho/hessian/io/PersonType.java

+
+import java.io.Serializable;
+import java.util.List;
+import java.util.Objects;


Objects is a java8+ feature. please adapt the lower version

I‘m using Objects.hash is java7+ feature, must adapt jdk1.6 ?

zonghaishang · 2018-02-08T12:04:51Z

@chickenlj @lovepoem please help review code, thanks.

@reference

* @reference support annotate on annotation type * Fixes apache#1303 TimeUnit conversion error * Fixes apache#1289, use bind_port as mapping key * Fixes apache#1313, remove destroy check in Registry. * checkout .travis.yml from origin/master * Fix hessian2 serialized short, byte is converted to int bug (apache#1232) * Fix hessian2 serialized short, byte is converted to int bug * Fix hessian2 serialized short, byte is converted to int bug * adapt jdk1.5+ * fixed travis-ci failed because of test cases. (apache#1370) * Merge pull request apache#1377, remove redundant arguments for StatItem.isAllowable() * Merge pull request apache#1378, replace StringBuider with simple string concatenation in log. * Merge pull request apache#1375, remove unnecessary boxing. Fixes apache#1245 * Merge pull request apache#1331, add optional parameter to support hessian protocol method overload and request protocol version. * Merge pull request apache#1376, do not instantiate load balance if there is no invokers Fixes apache#1297 * Merge pull request apache#1384, fix build string bug. * Merge pull request apache#1040, refactor: replace some deprecated methods related with jedis. * Merge pull request apache#1242, remove redundant null check. fixes apache#1231

@reference

* @reference support annotate on annotation type * Fixes apache#1303 TimeUnit conversion error * Fixes apache#1289, use bind_port as mapping key * Fixes apache#1313, remove destroy check in Registry. * checkout .travis.yml from origin/master * Fix hessian2 serialized short, byte is converted to int bug (apache#1232) * Fix hessian2 serialized short, byte is converted to int bug * Fix hessian2 serialized short, byte is converted to int bug * adapt jdk1.5+ * fixed travis-ci failed because of test cases. (apache#1370) * Merge pull request apache#1377, remove redundant arguments for StatItem.isAllowable() * Merge pull request apache#1378, replace StringBuider with simple string concatenation in log. * Merge pull request apache#1375, remove unnecessary boxing. Fixes apache#1245 * Merge pull request apache#1331, add optional parameter to support hessian protocol method overload and request protocol version. * Merge pull request apache#1376, do not instantiate load balance if there is no invokers Fixes apache#1297 * Merge pull request apache#1384, fix build string bug. * Merge pull request apache#1040, refactor: replace some deprecated methods related with jedis. * Merge pull request apache#1242, remove redundant null check. fixes apache#1231 * Change Mailing list address * Fixed apache#1398, revert bugs introduced from apache#1375 * Fix time unit problem in UT * Fix time unit problem related with FutureAdapter in UT

@reference

* @reference support annotate on annotation type * Fixes apache#1303 TimeUnit conversion error * Fixes apache#1289, use bind_port as mapping key * Fixes apache#1313, remove destroy check in Registry. * checkout .travis.yml from origin/master * Fix hessian2 serialized short, byte is converted to int bug (apache#1232) * Fix hessian2 serialized short, byte is converted to int bug * Fix hessian2 serialized short, byte is converted to int bug * adapt jdk1.5+ * fixed travis-ci failed because of test cases. (apache#1370) * Merge pull request apache#1377, remove redundant arguments for StatItem.isAllowable() * Merge pull request apache#1378, replace StringBuider with simple string concatenation in log. * Merge pull request apache#1375, remove unnecessary boxing. Fixes apache#1245 * Merge pull request apache#1331, add optional parameter to support hessian protocol method overload and request protocol version. * Merge pull request apache#1376, do not instantiate load balance if there is no invokers Fixes apache#1297 * Merge pull request apache#1384, fix build string bug. * Merge pull request apache#1040, refactor: replace some deprecated methods related with jedis. * Merge pull request apache#1242, remove redundant null check. fixes apache#1231 * Change Mailing list address * Fixed apache#1398, revert bugs introduced from apache#1375 * Fix time unit problem in UT * Fix time unit problem related with FutureAdapter in UT * Fix time unit problem related with FutureAdapter in UT * Merge pull request apache#1391, fix typo of method name in qos module. * fix hessian lite test case fail bug (apache#1394) * fix hessian lite test case fail bug * update test * remove ignore * Fix time unit problem related with FutureAdapter in UT * revert file * fix number type is lost in yaml config file (apache#1401) * apache#1399 fi * update test * update readme to add some details (apache#1403) * update readme to add some details update readme to add some details * delete duplicated words delete duplicated words * update README format * apache#1411: Locale deserialize 'zh-hant_CN'

takeseem · 2018-04-15T01:55:35Z

hessian-lite/src/main/java/com/alibaba/com/caucho/hessian/io/AbstractHessianInput.java

+     * @param expectedTypes the runtime type hints, eg: expectedClass equals Map, expectedTypes can
+     * equals String.class, Short.class
+     */
+    public Object readObject(Class expectedClass, Class<?>... expectedTypes)


#1232
@lovepoem @zonghaishang
the add method will work in Hessian2Input, but in HessianInput not unsupported.
I think the HessianInput need to change to support the new methods.

takeseem · 2018-04-15T02:39:35Z

HessianInput.readObject(Class, Class[]) nosupported, effect #1608

) * Fix hessian2 serialized short, byte is converted to int bug * Fix hessian2 serialized short, byte is converted to int bug * adapt jdk1.5+

zonghaishang added 2 commits January 17, 2018 17:52

Fix hessian2 serialized short, byte is converted to int bug

c970eda

Fix hessian2 serialized short, byte is converted to int bug

efd5cae

lovepoem self-requested a review February 5, 2018 07:30

lovepoem changed the base branch from master to 2.5.x February 7, 2018 10:59

lovepoem reviewed Feb 7, 2018

View reviewed changes

adapt jdk1.5+

e518ba3

lovepoem merged commit f247751 into apache:2.5.x Feb 8, 2018

zonghaishang deleted the support_new_serialization branch February 8, 2018 15:01

takeseem reviewed Apr 15, 2018

View reviewed changes

This was referenced Apr 15, 2018

fix #210 JavaDeserializer._constructor.newInstance([null, null]) NPE #1608

Closed

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

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix hessian2 serialized short, byte is converted to int bug#1232

Fix hessian2 serialized short, byte is converted to int bug#1232
lovepoem merged 3 commits intoapache:2.5.xfrom
zonghaishang:support_new_serialization

zonghaishang commented Jan 17, 2018

Uh oh!

codecov-io commented Jan 18, 2018

Uh oh!

chickenlj commented Feb 5, 2018

Uh oh!

lovepoem commented Feb 5, 2018

Uh oh!

zonghaishang commented Feb 6, 2018

Uh oh!

lovepoem left a comment •

edited

Loading

Uh oh!

lovepoem Feb 7, 2018

Uh oh!

zonghaishang Feb 7, 2018

Uh oh!

zonghaishang commented Feb 8, 2018

Uh oh!

takeseem Apr 15, 2018 •

edited

Loading

Uh oh!

takeseem commented Apr 15, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

zonghaishang commented Jan 17, 2018

Uh oh!

codecov-io commented Jan 18, 2018

Codecov Report

Uh oh!

chickenlj commented Feb 5, 2018

Uh oh!

lovepoem commented Feb 5, 2018

Uh oh!

zonghaishang commented Feb 6, 2018

Uh oh!

lovepoem left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lovepoem Feb 7, 2018

Choose a reason for hiding this comment

Uh oh!

zonghaishang Feb 7, 2018

Choose a reason for hiding this comment

Uh oh!

zonghaishang commented Feb 8, 2018

Uh oh!

takeseem Apr 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

takeseem commented Apr 15, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

lovepoem left a comment •

edited

Loading

takeseem Apr 15, 2018 •

edited

Loading