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

Remove Iot Core specific topic and client id validation checks in MQTT5 #315

Merged
merged 2 commits into from
Aug 10, 2023

Conversation

bretambrose
Copy link
Contributor

@bretambrose bretambrose commented Aug 9, 2023

  • Removes IoT Core specific topic length and segment count validation for subscribe, unsubscribe, and publish
  • Removes Iot Core specific client id length validation
  • Removes all validation tests checking these properties
  • Adds skipping $share/<share-name> to the iot core validation utility functions. Adds testing for the additional functionality.

Leaves the validation utility functions intact. We have determined these were not necessary in MQTT5 (and unnecessary client-side validation should always be avoided for future-compatibility reasons) as IoT Core now returns sensible reason codes that help us diagnose these issues (when using MQTT5; MQTT311 continues to be impacted due to the limitations of the spec). We may revisit and use this functionality as an opt-in setting on the 311 client to help prevent/diagnose particular problems.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

… mqtt5 client; preserve (and extend for shared subs) validation utility functions in case we decide to add support in the mqtt311 client
@codecov-commenter
Copy link

Codecov Report

Patch coverage: 75.00% and project coverage change: +0.11% 🎉

Comparison is base (b7a323d) 82.12% compared to head (51368c0) 82.24%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #315      +/-   ##
==========================================
+ Coverage   82.12%   82.24%   +0.11%     
==========================================
  Files          20       20              
  Lines        8679     8660      -19     
==========================================
- Hits         7128     7122       -6     
+ Misses       1551     1538      -13     
Files Changed Coverage Δ
source/v5/mqtt5_utils.c 80.09% <72.41%> (-2.69%) ⬇️
source/v5/mqtt5_options_storage.c 90.59% <100.00%> (+0.09%) ⬆️

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bretambrose bretambrose merged commit a2ee9a3 into main Aug 10, 2023
31 checks passed
@bretambrose bretambrose deleted the OverzealousIotCoreValidation branch August 10, 2023 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants