fix(json): int64 overflow in JSON numop

[jackyyyyyssss]

Json_val. NumOp only considers cases where the value of the double type is out of bounds, and does not consider cases where the int64 type is out of bounds. If the int64 type is out of bounds, it will cause the lambda function to exit abnormally and not catch. This exception will cause the kvrocks to crash

[PragmaTwice]

Thank you for your great finding.

[PragmaTwice]

I think it can be fixed in the JsonValue::NumOp. Here we can only fix for value, not the number inside the current json.

[jackyyyyyssss]

Thank you. Summarized all the situations to see if there were any omissions

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
61.2% Coverage on New Code
0.4% Duplication on New Code

See analysis details on SonarCloud

[PragmaTwice]

This function looks very weird. What's operation? What's 1 here? Why mul is executed if operation is not 1?

Please remove it.

[PragmaTwice]

Also, this function is totally wrong since the result is passed by value.

[PragmaTwice]

Please just add a catch block. Try not to add any meaningless weird code.

[jackyyyyyssss]

Please just add a catch block. Try not to add any meaningless weird code.

Simple catch doesn't seem to work, some overflow cases can return results that don't meet expectations without throwing exceptions
For example, 9223372036854775806<std:: numericlimits:: max() +20 is a negative number and will not throw an exception

[PragmaTwice]

Please just add a catch block. Try not to add any meaningless weird code.

Simple catch doesn't seem to work, some overflow cases can return results that don't meet expectations without throwing exceptions For example, 9223372036854775806<std:: numericlimits:: max() +20 is a negative number and will not throw an exception

It can be actually well defined. It follows the two's complement. So generally we can leave it as is until we make a big number implementation available.

[jackyyyyyssss]

So generally we can leave it as is until we make a big number implementation available.

What do you mean by directly adding a catch (const jsoncons:: json_runtime_error<std:: runtime_error>e){
Return {Status:: NotOK, e.what()};
}Is that enough for this ？

[PragmaTwice]

So generally we can leave it as is until we make a big number implementation available.

What do you mean by directly adding a catch (const jsoncons:: json_runtime_error<std:: runtime_error>e){ Return {Status:: NotOK, e.what()}; }Is that enough for this ？

Yeah, just preventing kvrocks from crash is enough.

[PragmaTwice]

Sorry I looked into this problem a little bit, and seems it's a design issue inside the NumOp method.

I opened a PR to address it (#2345).

But anyway thank you very much for your great finding and effort.

[jackyyyyyssss]

Sorry I looked into this problem a little bit, and seems it's a design issue inside the NumOp method.

I opened a PR to address it (#2345).

But anyway thank you very much for your great finding and effort.

You have made the same changes to the visual and redis stack processing, and have learned a lot