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

Harden handles against invalid connector id range #882

Conversation

Matthias-NIDEC
Copy link
Contributor

Describe your changes

If from backend site, the connectorId is not in valid range, the relevant reject is answered to avoid undefined behavior.

Issue ticket number and link

Checklist before requesting a review

@Pietfried
Copy link
Contributor

Hi @Matthias-NIDEC , thanks for your contribution! clang-format is failing in the CI.

You can run clang-format inside the libocpp root directory using

docker run -it \
  --mount type=bind,source=$(pwd),target=/source \
  ghcr.io/everest/everest-ci/build-env-base:v1.3.3 \
  run-clang-format \
    /source \
    --extensions hpp,cpp \
    -e cache \
    --color always \
    -ri

For future PRs it would be nice if you allow maintainers to push to the branches of your fork, so do something like this without your support ;) https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

@Pietfried Pietfried self-assigned this Dec 3, 2024
@Pietfried Pietfried force-pushed the harden_handles_against_invalid_connectorId_range branch from 15db6f1 to 97cd2d4 Compare December 4, 2024 18:06
* Allow SetChargingProfile.req with connector_id == 0
* Removed check from ClearChargingProfile.req since profile id was used instead of connector_id.
* Removed check from ReserveNow.req since implementation had business logic error and reserve_now_callback is responsible for checking the range

Signed-off-by: Piet Gömpel <[email protected]>
@Pietfried Pietfried merged commit c234ccc into EVerest:main Dec 4, 2024
7 of 8 checks passed
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.

2 participants