codelabsab / rust-ocpp

Libraries for ocpp 1.6 and 2.0.1
https://docs.rs/rust-ocpp/latest/rust_ocpp/
Apache License 2.0
64 stars 15 forks source link

fix: validate_string_identifier would allow some disallowed strings #24

Closed Erk- closed 10 months ago

Erk- commented 11 months ago

Any string containing at least one valid character would be allowed by the regex, this fix removes regex and does a normal scan of the string instead.

This patch also contains some cleaning in the Cargo.toml file where it removes dependencies that was not in use.

tommymalmqvist commented 10 months ago

Hello again @Erk- sorry about the late response and thanks for the PR.

This is actually a remnant from when I had plans to implement a server and client in this repo and the file is not really used. I started writing a server in another repo.

Do you need it for something special?

Erk- commented 10 months ago

I don't need it for anything, I was just looking through the dependencies and was seeing if I could remove some of them and spotted the issue above. So it may be better to just remove it.

tommymalmqvist commented 10 months ago

Thanks I’ll remove it then!

Erk- commented 10 months ago

@tommymalmqvist I can see that it is used for the #[validate(...)] attributes, should these also be removed or would it make more sense to keep validate_string_identifier in that case?

tommymalmqvist commented 10 months ago

@tommymalmqvist I can see that it is used for the #[validate(...)] attributes, should these also be removed or would it make more sense to keep validate_string_identifier in that case?

Yes, you are correct!

I could update the regex like so also:

let re = Regex::new(r"^[a-zA-Z0-9*+=:|@._-]*$").unwrap();
Erk- commented 10 months ago

Yeah that would also be a valid solution, though it should probably be ..]+$.

Would you rather keep a regex based solution because then I can switch to that.

tommymalmqvist commented 10 months ago

Lets go with regex 👍

Erk- commented 10 months ago

@tommymalmqvist I have updated it to use regex, so you can have another look now.

codecov-commenter commented 10 months ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +84.12% :tada:

Comparison is base (7dceebb) 0.34% compared to head (4b9019c) 84.47%. Report is 6 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #24 +/- ## ========================================== + Coverage 0.34% 84.47% +84.12% ========================================== Files 208 280 +72 Lines 287 4849 +4562 ========================================== + Hits 1 4096 +4095 - Misses 286 753 +467 ``` | [Files Changed](https://app.codecov.io/gh/codelabsab/rust-ocpp/pull/24?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=codelabsab) | Coverage Δ | | |---|---|---| | [src/v1\_6/messages/authorize.rs](https://app.codecov.io/gh/codelabsab/rust-ocpp/pull/24?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=codelabsab#diff-c3JjL3YxXzYvbWVzc2FnZXMvYXV0aG9yaXplLnJz) | `100.00% <ø> (ø)` | | | [src/v1\_6/messages/boot\_notification.rs](https://app.codecov.io/gh/codelabsab/rust-ocpp/pull/24?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=codelabsab#diff-c3JjL3YxXzYvbWVzc2FnZXMvYm9vdF9ub3RpZmljYXRpb24ucnM=) | `100.00% <ø> (ø)` | | | [src/v1\_6/messages/cancel\_reservation.rs](https://app.codecov.io/gh/codelabsab/rust-ocpp/pull/24?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=codelabsab#diff-c3JjL3YxXzYvbWVzc2FnZXMvY2FuY2VsX3Jlc2VydmF0aW9uLnJz) | `100.00% <ø> (ø)` | | | [src/v1\_6/messages/change\_availability.rs](https://app.codecov.io/gh/codelabsab/rust-ocpp/pull/24?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=codelabsab#diff-c3JjL3YxXzYvbWVzc2FnZXMvY2hhbmdlX2F2YWlsYWJpbGl0eS5ycw==) | `100.00% <ø> (ø)` | | | [src/v1\_6/messages/change\_configuration.rs](https://app.codecov.io/gh/codelabsab/rust-ocpp/pull/24?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=codelabsab#diff-c3JjL3YxXzYvbWVzc2FnZXMvY2hhbmdlX2NvbmZpZ3VyYXRpb24ucnM=) | `100.00% <ø> (ø)` | | | [src/v1\_6/messages/clear\_cache.rs](https://app.codecov.io/gh/codelabsab/rust-ocpp/pull/24?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=codelabsab#diff-c3JjL3YxXzYvbWVzc2FnZXMvY2xlYXJfY2FjaGUucnM=) | `100.00% <ø> (ø)` | | | [src/v1\_6/messages/clear\_charging\_profile.rs](https://app.codecov.io/gh/codelabsab/rust-ocpp/pull/24?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=codelabsab#diff-c3JjL3YxXzYvbWVzc2FnZXMvY2xlYXJfY2hhcmdpbmdfcHJvZmlsZS5ycw==) | `100.00% <ø> (ø)` | | | [src/v1\_6/messages/data\_transfer.rs](https://app.codecov.io/gh/codelabsab/rust-ocpp/pull/24?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=codelabsab#diff-c3JjL3YxXzYvbWVzc2FnZXMvZGF0YV90cmFuc2Zlci5ycw==) | `100.00% <ø> (ø)` | | | [...c/v1\_6/messages/diagnostics\_status\_notification.rs](https://app.codecov.io/gh/codelabsab/rust-ocpp/pull/24?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=codelabsab#diff-c3JjL3YxXzYvbWVzc2FnZXMvZGlhZ25vc3RpY3Nfc3RhdHVzX25vdGlmaWNhdGlvbi5ycw==) | `100.00% <ø> (ø)` | | | [src/v1\_6/messages/firmware\_status\_notification.rs](https://app.codecov.io/gh/codelabsab/rust-ocpp/pull/24?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=codelabsab#diff-c3JjL3YxXzYvbWVzc2FnZXMvZmlybXdhcmVfc3RhdHVzX25vdGlmaWNhdGlvbi5ycw==) | `100.00% <ø> (ø)` | | | ... and [87 more](https://app.codecov.io/gh/codelabsab/rust-ocpp/pull/24?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=codelabsab) | | ... and [367 files with indirect coverage changes](https://app.codecov.io/gh/codelabsab/rust-ocpp/pull/24/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=codelabsab)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.