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
Add additional check for non-manadatory parameters #2538
Add additional check for non-manadatory parameters #2538
Conversation
message[strings::msg_params][strings::file_size] = | ||
(*message_)[strings::msg_params][strings::length]; | ||
if ((*message_)[strings::msg_params].keyExists(strings::offset)) { | ||
message[strings::msg_params][strings::offset] = offset_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assignment is redundant, the same assignment is two lines above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jacobkeeler Yes, good point, please take a look b4691c7
message[strings::msg_params][strings::offset] = offset_; | ||
if (0 == offset_ && (*message_)[strings::msg_params].keyExists(strings::length)) { | ||
message[strings::msg_params][strings::file_size] = | ||
(*message_)[strings::msg_params][strings::length]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong case, this should be set to length_
if 0 == offset_ && !keyExists(strings::length)
, file_size
should not be set if length
is sent by the app. This is because if length
is provided or offset
is nonzero, it means that this is a partial data chunk, which means that you cannot determine the length of the file.
message[strings::msg_params][strings::offset] = offset_;
if (0 == offset_ && !(*message_)[strings::msg_params].keyExists(strings::length)) {
message[strings::msg_params][strings::file_size] = length_;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jacobkeeler please take a look b4691c7
The OnPutFile notification has non mandatory parameters `fileSize` and `length`. If the mobile application has no provided appropriate parameters SDL sends `fileSize`: null as json value to HMI. The commit fixes the issue and simply remove non mandatory parameter in case mobile app sends nothing for one.
08ccbaf
to
b4691c7
Compare
Fixes #2443
This PR is ready for review.
Risk
This PR makes no API changes.
Testing Plan
ATF script - smartdevicelink/sdl_atf_test_scripts#2020
Summary
The OnPutFile notification has non mandatory parameters
fileSize
andlength
. If the mobile application has no providedappropriate parameters SDL sends
fileSize
: null as json value to HMI.The commit fixes the issue and simply remove non mandatory parameter in
case mobile app sends nothing for one.
Tasks Remaining:
CLA