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

Fix a crash when RPC encryption starts and callback when RPCs fail due to RPC encryption #1972

Merged
merged 20 commits into from
Apr 20, 2021

Conversation

joeljfischer
Copy link
Contributor

@joeljfischer joeljfischer commented Apr 15, 2021

Fixes #1966, #1971

Risk

This PR makes no API changes.

Testing Plan

  • I have verified that I have not introduced new warnings in this PR (or explain why below)
  • I have run the unit tests with this PR
  • I have tested this PR against Core and verified behavior (if applicable, if not applicable, explain why below).

Unit Tests

Unit tests were updated to ensure that the callback is handled correctly.

Core Tests

Tested an RPC encrypted app with PutFile encrypted. Ensured that no crashes occurred and the app set up correctly.

Core version / branch / commit hash / module tested against: v7.1.0-RC.1 (0df66080512cbe76ee5d4872910a54adf3ca1f36)
HMI name / version / branch / commit hash / module tested against: sdl_hmi v5.5.0

Summary

  1. SDLLifecycleManager now ignores RPC StartServiceACK if we're already connected instead of crashing.
  2. Updates SDLProtocol sendRPC: to pass back an error and return a success BOOL so that the handler can be called in the SDLLifecycleManager if the RPC fails to send in the protocol layer.

Changelog

Bug Fixes
  • Fixed RPC encryption causing a crash when the second RPC StartServiceACK is received.
  • Fixed not calling an RPC request handler if the message failed to send in the protocol layer of the library.

Tasks Remaining:

  • Add a unit test for the RPC failing to send in the protocol layer should call the handler.
  • Smoke test on Core v7.1.0
  • Smoke test on Sync 3.4

CLA

@joeljfischer joeljfischer added bug A defect in the library manager-lifecycle Relating to the manager layer - lifecycle manager protocol Relating to the protocol layer labels Apr 15, 2021
@joeljfischer joeljfischer self-assigned this Apr 15, 2021
@codecov
Copy link

codecov bot commented Apr 15, 2021

Codecov Report

Merging #1972 (1fa7c01) into hotfix/7.1.1 (62d1859) will increase coverage by 0.02%.
The diff coverage is 50.98%.

@@               Coverage Diff                @@
##           hotfix/7.1.1    #1972      +/-   ##
================================================
+ Coverage         83.66%   83.69%   +0.02%     
================================================
  Files               441      441              
  Lines             22543    22564      +21     
================================================
+ Hits              18861    18885      +24     
+ Misses             3682     3679       -3     

@joeljfischer joeljfischer marked this pull request as ready for review April 16, 2021 15:36
@joeljfischer joeljfischer changed the base branch from develop to hotfix/7.1.1 April 16, 2021 15:39
@NicoleYarroch NicoleYarroch linked an issue Apr 16, 2021 that may be closed by this pull request
Copy link
Contributor

@NicoleYarroch NicoleYarroch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I small documentation fix requested.

SmartDeviceLink/private/SDLProtocol.h Show resolved Hide resolved
@NicoleYarroch NicoleYarroch self-requested a review April 20, 2021 15:41
@joeljfischer joeljfischer merged commit 5bd0d7c into hotfix/7.1.1 Apr 20, 2021
@joeljfischer joeljfischer deleted the bugfix/issue-1966-rpc-encryption-crash branch April 20, 2021 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A defect in the library manager-lifecycle Relating to the manager layer - lifecycle manager protocol Relating to the protocol layer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Post-Connection RPC StartService w/ Encryption Causes a Crash
2 participants