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

Adding #[serde(deny_unknown_fields, rename_all = "camelCase")] #29

Open
fulup-bzh opened this issue Oct 31, 2023 · 5 comments
Open

Adding #[serde(deny_unknown_fields, rename_all = "camelCase")] #29

fulup-bzh opened this issue Oct 31, 2023 · 5 comments

Comments

@fulup-bzh
Copy link

Without deny_unknown_fields, Serde may matched any message to '{}'. Getting a stricter control is especially important for transaction that send '{}' as response (i.e. StatusNotification). As a result with enforcing 'deny_unknown_fields' Serde may match to {} any backend error message.

Proposal in all message definition:

  • replace #[serde(rename_all = "camelCase")]
  • with #[serde(deny_unknown_fields, rename_all = "camelCase")]
@tommymalmqvist
Copy link
Member

Hi @fulup-bzh, thanks. Will check this out when I have the time. As far as I remember there where multiple responses that can have empty objects.

@fulup-bzh
Copy link
Author

To test, I have a fork at https://github.com/tux-evse/codelab-ocpp with a global change. As least in my case it solves my issues for empty messages. I will publish my OCPP-client code as soon as the patches I made on my dependencies are public. So far outside of this 'deny_unknown_fields' I did not had any trouble with your code.

On dependencies side, I'm wondering with 'regex' is mandatory and 'chrono' attached to 'serde'. When using your code for an OCPP/client I would expect nothing outside of 'serde/(serialization/deserialization)'. Did I miss something ?

@tommymalmqvist
Copy link
Member

I use Chrono to validate and serialize the dateTime strings being sent to be valid dateTime strings specified in RFC3339, which the OCPP standard is using. If found it was easy enough, but maybe there is some easier way?

regex is only used in the validate_identifier_string function.

The specification states:

identifierString: This is a case-insensitive dataType and can only contain characters from the following character set: a-z, A-Z, 0-9, '*', '-', '_', '=', ':', '+', '|', '@', '.'

@fulup-bzh
Copy link
Author

In this case when not enforcing the validation and relying only on serde for serialisation/deserialisation those dependencies should drop, isn't it ?

@tommymalmqvist
Copy link
Member

In this case when not enforcing the validation and relying only on serde for serialisation/deserialisation those dependencies should drop, isn't it ?

Yes, that is true. You would need to trust that the client/server is sending/creating valid dateTime strings.

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

No branches or pull requests

2 participants