apache / rocketmq

Apache RocketMQ is a cloud native messaging and streaming platform, making it simple to build event-driven applications.
https://rocketmq.apache.org/
Apache License 2.0
21.19k stars 11.67k forks source link

Use gson to replace fastjson #2462

Open maixiaohai opened 3 years ago

maixiaohai commented 3 years ago

Do we have plan to replace fastjson with Gson considering the security problem

duhenglucky commented 3 years ago

@maixiaohai IMHO,this is a very good suggestion, but due to some historical reasons, rocketmq uses some JSON format that only fastjson supports, so it is a bit difficult, but I think everyone can discuss it firstly @zongtanghu @ShannonDing @RongtongJin @vongosling

vongosling commented 3 years ago

If we could make our data more standard JSON format, maybe smoothly transformed from one JSON lib to another lib. Who wants to have a try?

maixiaohai commented 3 years ago

@maixiaohai IMHO,this is a very good suggestion, but due to some historical reasons, rocketmq uses some JSON format that only fastjson supports, so it is a bit difficult, but I think everyone can discuss it firstly @zongtanghu @ShannonDing @RongtongJin @vongosling

I’ve changed some code to avoid fastjson importing from client SDK,but server still have dependency which import from acl module and dledger Wondering ”JSON format that only fastjson supports“ belong to which module?

vongosling commented 3 years ago

There are many similar articles that you can refer to[1][2][3]. Of course, this is just the tip of the iceberg. To enable smooth migration, I suggest you look for some standard tools to compare serialization and deserialization between different libs. Or, you could write a tool by yourself. That's definitely a gospel for those who are beset by the problem.

[1] https://github.com/alibaba/fastjson/issues/3024 [2] https://zhuanlan.zhihu.com/p/133419561 [3] https://blog.csdn.net/sbsujjbcy/article/details/50414188

maixiaohai commented 3 years ago

There are many similar articles that you can refer to[1][2][3]. Of course, this is just the tip of the iceberg. To enable smooth migration, I suggest you look for some standard tools to compare serialization and deserialization between different libs. Or, you could write a tool by yourself. That's definitely a gospel for those who are beset by the problem.

[1] alibaba/fastjson#3024 [2] https://zhuanlan.zhihu.com/p/133419561 [3] https://blog.csdn.net/sbsujjbcy/article/details/50414188

I‘ve used Gson to replace fastjson in remoting module and passed all unit tests. As your suggestion, this is not enough,the result of serialization and deserialization should both be checked.Do I understand right? Another question is : message store(including produce and consume) will not be affected by the replacement, is that right?

vongosling commented 3 years ago

Yep~ how's this going so far?

maixiaohai commented 3 years ago

Yep~ how's this going so far?

So sorry for delay reply. In our production cluster, gson version worked well, and all the clients upgraded to new version. While at community, smooth migration is necessary. So I will study some articles and make a tool for standard json format test, then figure out how to go on.

vongosling commented 3 years ago

Great, please feel free to ping the community if you have any problems. This issue will be open for a long time until reasonable RIP output is provided. More guys are welcome to participate in the community through this case.

maixiaohai commented 3 years ago

I recently sorted out the usage of fastjson in different model(acl remoting broker client tools and common). When replacing fastjson with gson, the remoting model will be the most complicated part because we should think about the compatible of lower version client and higher version server. Based on the situation, I made some tests about the compatiblility when client using fastjson and server using gson. In a result, there are some classes which produce non-standard json string and will cause compatibility problem.

Here is what I thought

  1. we could make a new bridge serializable class, and let the non-compatibility protocol class extends it, then through request version to distinguish which method to use to serialize/deserialize the class. The advantages are the structure of class and meta file will not change, and affect is limited, also the code will be remove easily when client all updated. The disadvantages are the dependency of fastjson will exists in a long term and the compatible code maybe not graceful.
  2. Other fastjson annotation problems could be fixed by using different gson parameters.

what's the community's opinions? @vongosling @duhenglucky @RongtongJin @ShannonDing

exceptionplayer commented 3 years ago

Maybe we should support gson or jackson, not only gson.

maixiaohai commented 3 years ago

Maybe we should support gson or jackson, not only gson.

It seems jackson have security problem too.

zhaohai666 commented 3 years ago

You can use protobuf to replace fastjson.

NotFound403 commented 2 years ago

i consider that a json abstract layer may be better