Fix SDL crash on validation of numeric schemas with type overflow #3750
Conversation
|
@theresalech This PR is ready for Livio review |
|
@AKalinich-Luxoft , thank you! We will include this in the upcoming release if time allows. |
| NumberType value(0); | ||
| if (typeid(int32_t) == typeid(value)) { | ||
| value = utils::SafeStaticCast<int64_t, int32_t>(Object.asInt()); | ||
| if (Object.asInt() > std::numeric_limits<int32_t>::max() || | ||
| Object.asInt() < std::numeric_limits<int32_t>::min()) { |
There was a problem hiding this comment.
One issue with this approach, some integer parameters don't have a maxvalue param, so those should be able to handle whatever value is sent (barring it being larger that 64 bits, since that is a hard limit).
Something like cancelID is currently stored in a 32 bit integer, but has no limitations as far as range. This change would make it so any value larger than 32 bits would be rejected.
To solve this, I think that we should use 64 bits for parameters which are missing either maxValue or minValue. This would need to be done in the RPC spec parser script. That, combined with this fix, should cover all cases.
There was a problem hiding this comment.
@jacobkeeler generator script has been updated in 2852ac4 and 9891d93
Now generator has rules for int/uint 32/64 by analyzing minValue and maxValue and providing the best fit for each integer in the schema.
Note that schema item itself has been updated to properly reflect all changes from generator.
|
Hi @AKalinich-Luxoft , can you please review and respond to Jacob's comment? We are looking to complete development for the Core 8.0 release soon, so will need to make progress on this PR quickly in order to ensure its inclusion in the release. Thank you! |
@theresalech Luxoft is working on the fix and going to provide extra changes according to Livio comments above. |
|
Hi @AKalinich-Luxoft, Livio is going to begin Release candidate prep, starting on 2021-08-30. As mentioned in Theresa's comment above, we will need to make progress on this PR asap for it to be included in the upcoming release. Can you please provide timing on when Luxoft will have this PR ready for Livio review? Thanks! |
|
@AKalinich-Luxoft It looks like you have made new commits to this PR, is this ready for Livio review? |
@jordynmackool this PR will be ready for review today. We have all the required changes and just need some time to finalize the regression testing. I will let you know once it is ready for Livio review. |
|
@jordynmackool regression testing is finished. This PR is ready for Livio review |
tools/InterfaceGenerator/generator/generators/SmartFactoryBase.py
Outdated
Show resolved
Hide resolved
tools/InterfaceGenerator/generator/generators/SmartFactoryBase.py
Outdated
Show resolved
Hide resolved
tools/InterfaceGenerator/generator/generators/SmartFactoryBase.py
Outdated
Show resolved
Hide resolved
tools/InterfaceGenerator/generator/generators/SmartFactoryBase.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Jacob Keeler <jacob.keeler@livioradio.com>
Fixes #3226
This PR is ready for review.
Risk
This PR makes no API changes.
Testing Plan
Reproduction Steps
Expected Behavior
No Crash should be observed while testing
Observed Behavior
SDL crashed while testing
Summary
SDL crashes when the HMI sends UI.OnTouchEvent with the 'ts' param has a value greater than maxvalue.
Was added processing for the case where the SDL gets numbers that go beyond the type limit.
CLA