Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fixed missing error description/code in security notifications #2057

Conversation

FrankElias77
Copy link
Contributor

@FrankElias77 FrankElias77 commented Oct 22, 2021

Fixes #2049

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

N/A

Core Tests

Tested setting a serverHandshakeData on runtime to force a run of sdl_serverSecurityFailedMessageWithClientMessageHeader in order to check if the control message created by the app is formed correctly as instructed in the proposal

Core version / branch / commit hash / module tested against: 8.0.0
HMI name / version / branch / commit hash / module tested against: generic_hmi v0.11.0

Summary

Added a jsonData and binaryData based on the proposal to our SDLSecurity in case our serverHandshakeData has failed

Changelog

Breaking Changes
  • Added a jsonData and binaryData based on the proposal to our SDLSecurity in case our serverHandshakeData has failed
Enhancements
  • N/A
Bug Fixes
  • N/A

Tasks Remaining:

N/A

CLA

@FrankElias77 FrankElias77 added bug A defect in the library protocol Relating to the protocol layer labels Oct 22, 2021
@codecov
Copy link

codecov bot commented Oct 22, 2021

Codecov Report

Merging #2057 (8f7bd9c) into develop (6c38240) will decrease coverage by 0.01%.
The diff coverage is 11.11%.

❗ Current head 8f7bd9c differs from pull request most recent head b2185ea. Consider uploading reports for the commit b2185ea to get more accurate results

@@             Coverage Diff             @@
##           develop    #2057      +/-   ##
===========================================
- Coverage    85.83%   85.81%   -0.02%     
===========================================
  Files          447      447              
  Lines        23296    23303       +7     
===========================================
+ Hits         19996    19998       +2     
- Misses        3300     3305       +5     

@FrankElias77 FrankElias77 marked this pull request as ready for review October 22, 2021 19:40
Copy link
Contributor

@joeljfischer joeljfischer left a comment

Choose a reason for hiding this comment

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

Your comments have nothing for core tests. This must be tested with Core. You can modify the lib slightly to force the error to be sent back, but you need to make sure Core doesn't crash or something when tested. Then list your test steps.

@joeljfischer joeljfischer merged commit 9c19083 into develop Nov 19, 2021
@joeljfischer joeljfischer deleted the bugfix/issue-2049-security-error-notifications-are-missing-error-description-and-code branch November 19, 2021 16:47
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug A defect in the library protocol Relating to the protocol layer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants