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

Use gson to replace fastjson #2462

Open
maixiaohai opened this issue Nov 30, 2020 · 13 comments
Open

Use gson to replace fastjson #2462

maixiaohai opened this issue Nov 30, 2020 · 13 comments
Labels
no stale This will never be considered stale progress/discuss type/enhancement

Comments

@maixiaohai
Copy link
Contributor

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

@duhenglucky
Copy link
Contributor

@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
Copy link
Member

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
Copy link
Contributor Author

@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
Copy link
Member

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

@maixiaohai
Copy link
Contributor Author

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
Copy link
Member

Yep~ how's this going so far?

@maixiaohai
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Contributor

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

@maixiaohai
Copy link
Contributor Author

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

It seems jackson have security problem too.

@zhaohai666
Copy link
Contributor

You can use protobuf to replace fastjson.

@NotFound403
Copy link

i consider that a json abstract layer may be better

@aaron-ai aaron-ai added the no stale This will never be considered stale label Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no stale This will never be considered stale progress/discuss type/enhancement
Projects
None yet
Development

No branches or pull requests

7 participants