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

RFC 0003 Protocols - Added note about always ignoring unrecognized items in a message #527

Merged
merged 1 commit into from
Aug 13, 2020
Merged

RFC 0003 Protocols - Added note about always ignoring unrecognized items in a message #527

merged 1 commit into from
Aug 13, 2020

Conversation

swcurran
Copy link
Member

@swcurran swcurran commented Aug 13, 2020

Also corrected the name of the problem-report message (vs. the report problem protocol name).

…n if the protocol minor versions match

Signed-off-by: Stephen Curran <[email protected]>
@swcurran swcurran changed the title Add explicit note about ignoring unrecognized items in a message, even if the protocol minor versions match RFC 0003 Protocols - Added note about always ignoring unrecognized items in a message Aug 13, 2020
@swcurran
Copy link
Member Author

@TimoGlastra -- this came up in the https://github.com/bcgov/aries-agent-test-harness when a test failed in ACA-Py based on a message from aries-framework-dotnet that had an unrecognized field that I think you mentioned was @id. I realize that although ACA-Py should have ignored that field, is the field there in place of an id field, or are both present? @id was used in the past (as I recall) but should no longer be used. Perhaps that is a transitional field?

Copy link
Contributor

@llorllale llorllale left a comment

Choose a reason for hiding this comment

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

LGTM

@dhh1128
Copy link
Contributor

dhh1128 commented Aug 13, 2020

Looks good to me, and this was discussed and approved on a community call. Merging.

@dhh1128 dhh1128 merged commit 2d01752 into hyperledger:master Aug 13, 2020
@TimoGlastra
Copy link
Member

@swcurran It was indeed the @id field. The value isn't set in the .NET framework, but due to how classes are parsed to JSON, unset values are included as null.


A recipient of a message from a supported major version of a protocol should ignore any unrecognized items in a received message

I'm curious if this also applies to the issue we had with the optional comment property in messages from .NET -> ACA-Py where ACA-Py only allowed the property to be of type string or for it to be left out, but .NET sends null.

@dhh1128
Copy link
Contributor

dhh1128 commented Aug 14, 2020

@TimoGlastra That's an interesting side note: what does null mean with respect to optional fields. I think the answer is that null is not the same as <missing>. If others agree, we maybe ought to say that somewhere...

@swcurran
Copy link
Member Author

Could someone else take a shot at that? I'm not the one to write that -- it gets a bit too far into JSON handling best practices for me.

@swcurran swcurran deleted the protocol-ignore-items branch October 8, 2020 20:46
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.

4 participants