Validate message topics/tag before sending #365
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently these fields are verified only by the receiver, which can lead to sending a bad message that is rejected on the other end. Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Codecov Report
@@ Coverage Diff @@
## main #365 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 239 239
Lines 12888 12884 -4
=========================================
- Hits 12888 12884 -4
Continue to review full report at Codecov.
|
Contributor
Author
|
Looks like on Fabric, we generate an invalid topic name for org broadcasts. Not sure why it was working before - perhaps we're not doing the receiver-side validation of messages when dealing with org broadcasts? Seems like that should be in effect for all messages, including system definitions. |
Contributor
Author
|
After further investigation, there are actually a few layers of problems here.
|
Currently "names" and UUIDs are mutually exclusive in all cases - but this was overly broad. New behavior is outlined below. Some "named" objects already have an ID - therefore it's confusing to allow their name to also be a UUID. These objects still cannot be named with a UUID: - organizations - nodes - groups - subscriptions - datatypes - token pools For "name" fields that are simple strings, there is no confusion in setting them to a UUID, and in fact a UUID may be a good choice in some scenarios. These fields can be a UUID: - namespaces - topics - tags Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Contributor
Author
|
Pushed additional fixes for the issues noted above. See each commit message for more details. |
peterbroadhurst
approved these changes
Dec 29, 2021
Contributor
peterbroadhurst
left a comment
peterbroadhurst
left a comment
There was a problem hiding this comment.
Noting this contains a material change, that organizations are now all published on a common ff_organizations topic.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently these fields are verified only by the receiver, which can lead to
sending a bad message that is rejected on the other end.