On Session Ended, hashID should be sent for RPC Service. #346
Conversation
| @@ -28,6 +28,7 @@ @interface SDLProtocol () { | |||
| dispatch_queue_t _sendQueue; | |||
| SDLPrioritizedObjectCollection *_prioritizedCollection; | |||
| NSMutableDictionary *_sessionIDs; | |||
| NSMutableDictionary *_hashIDs; | |||
There was a problem hiding this comment.
Do you think that instead of storing multiple dictionaries with sessionIDs / hashIDs / whatever else in the future, it should just be one dict with various metadata about the service? That's what I'm starting to lean towards.
There was a problem hiding this comment.
This would probably be a good idea for a future enhancement. We would need to make sure it's dynamic enough to accept any future metadata. Would this be suitable for now, do you think?
There was a problem hiding this comment.
Either dynamic enough or private enough to change it without consequence. Probably the latter.
I'm considering the change in this security branch to track which services are encrypted, so that if you try to send an encrypted RPC on a non-encrypted service type it fails, I think this would fit in the same place. So I would say yeah, it probably fine for now, but should change sooner rather than later. Maybe as a "hotfix" separate from this.
There was a problem hiding this comment.
Also, technically it should be hashIds, since "Id" is short for one word, not two. Not sure how we want to handle that since it's already done incorrectly with sessionIDs
There was a problem hiding this comment.
Just tried to best following existing naming so when we do changes it'll be consistent ;)
| @@ -310,7 +345,7 @@ - (void)sendRawData:(NSData *)data withServiceType:(SDLServiceType)serviceType { | |||
|
|
|||
|
|
|||
| #pragma mark - SDLProtocolListener Implementation | |||
| - (void)handleProtocolStartSessionACK:(SDLServiceType)serviceType sessionID:(Byte)sessionID version:(Byte)version { | |||
| - (void)handleProtocolStartSessionACK:(SDLServiceType)serviceType sessionID:(Byte)sessionID hashID:(UInt32)hashID version:(Byte)version { | |||
There was a problem hiding this comment.
Unfortunately, we're both touching the same code here. The security framework deprecates and updates most of these SDLProtocolListener calls. This one that you're changing would have necessitated a major version change (so that you are aware in the future). The security framework adds a method that passes the entire SessionACK header, that might require some changes to this on your part.
…ture/end_session_hash
…ines of SmartDeviceLink.
|
@joeljfischer how's this latest commit? |
| @@ -9,7 +9,8 @@ | |||
| @protocol SDLProtocolListener <NSObject> | |||
|
|
|||
| @optional | |||
| - (void)handleProtocolStartSessionACK:(SDLServiceType)serviceType sessionID:(Byte)sessionID version:(Byte)version; | |||
| - (void)handleProtocolStartSessionACK:(SDLServiceType)serviceType sessionID:(Byte)sessionID version:(Byte)version __deprecated_msg("use -handleProtocolStartSessionACK:sessionID:hashID:version: instead"); | |||
| - (void)handleProtocolStartSessionACK:(SDLServiceType)serviceType sessionID:(Byte)sessionID hashID:(UInt32)hashID version:(Byte)version; | |||
There was a problem hiding this comment.
I'm still not happy with this because security PR is creating a different method, so in the security PR I'd have to deprecate this method. It would be better to have it follow the security framework's changes to this protocol instead.
In any case, thanks for the merge nightmare 😰
|
This was fixed in 4.2.0 |
Fixes #345