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.

Sign up for GitHub

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

Jump to bottom

validate the request before processing the body #578

Merged
merged 4 commits into from
Jan 23, 2024
Merged

validate the request before processing the body #578

merged 4 commits into from
Jan 23, 2024

Conversation

jkralik
Copy link
Member

@jkralik jkralik commented Dec 5, 2023

No description provided.

@ocf-conformance-test-tool
Copy link

🎉 Thank you for your code contribution! To guarantee the change/addition is conformant to the OCF Specification, we would like to ask you to execute OCF Conformance Testing of your change ☝️ when your work is ready to be reviewed.

ℹ️ To verify your latest change (663ab61), label this PR with OCF Conformance Testing.

⚠️ Label is removed with every code change.

@jkralik jkralik force-pushed the jkralik/fix/validate-requests branch 10 times, most recently from 012dac3 to cf0defb Compare December 8, 2023 13:43
@jkralik jkralik requested a review from Danielius1922 December 8, 2023 15:10
@jkralik jkralik marked this pull request as ready for review December 8, 2023 15:10
@jkralik jkralik added the OCF Conformance Testing OCF Conformance Testing required label Dec 11, 2023
@jkralik jkralik force-pushed the jkralik/fix/validate-requests branch from cf0defb to 1037096 Compare December 11, 2023 12:30
@ocf-conformance-test-tool ocf-conformance-test-tool bot removed the OCF Conformance Testing OCF Conformance Testing required label Dec 11, 2023
@jkralik jkralik force-pushed the jkralik/fix/validate-requests branch 4 times, most recently from 1053ee3 to 80ee099 Compare December 11, 2023 14:35
@jkralik jkralik added the OCF Conformance Testing OCF Conformance Testing required label Dec 11, 2023
@jkralik jkralik force-pushed the jkralik/fix/validate-requests branch from 80ee099 to 793fa92 Compare December 11, 2023 20:09
@ocf-conformance-test-tool ocf-conformance-test-tool bot removed the OCF Conformance Testing OCF Conformance Testing required label Dec 11, 2023
@jkralik jkralik force-pushed the jkralik/fix/validate-requests branch 2 times, most recently from acc7b0f to cd0d2a3 Compare December 11, 2023 21:14
@jkralik jkralik added the OCF Conformance Testing OCF Conformance Testing required label Dec 11, 2023
@ocf-conformance-test-tool ocf-conformance-test-tool bot removed the OCF Conformance Testing OCF Conformance Testing required label Dec 12, 2023
@jkralik jkralik force-pushed the jkralik/fix/validate-requests branch 3 times, most recently from 69e81fc to 2ded549 Compare December 12, 2023 13:17
@jkralik jkralik force-pushed the jkralik/fix/validate-requests branch 6 times, most recently from a9f5f41 to c788bd4 Compare December 12, 2023 15:09
@jkralik jkralik added the OCF Conformance Testing OCF Conformance Testing required label Dec 12, 2023
@jkralik jkralik force-pushed the jkralik/fix/validate-requests branch from c788bd4 to 7d8eef7 Compare December 22, 2023 11:44
@ocf-conformance-test-tool ocf-conformance-test-tool bot removed the OCF Conformance Testing OCF Conformance Testing required label Dec 22, 2023
@jkralik jkralik force-pushed the jkralik/fix/validate-requests branch from 7d8eef7 to 2ed3476 Compare January 16, 2024 09:03
Danielius1922
Danielius1922 reviewed Jan 19, 2024
View reviewed changes
api/oc_ri_internal.h Outdated

/** @brief Validate a coap request. When fails it will set error message for the
* peer. This function doesn't validate body content. */
bool oc_ri_validate_coap_request(coap_make_response_ctx_t *ctx,
Copy link
Member

Choose a reason for hiding this comment

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

Cannot the ctx parameter be made const? Seems weird that a validation function would modify it.

Copy link
Member Author

Choose a reason for hiding this comment

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

No. I renamed it to oc_ri_parse_coap_request_header

messaging/coap/engine_internal.h Outdated
oc_endpoint_t *, void *);
const oc_endpoint_t *, void *);

typedef bool (*coap_validate_request_fn_t)(coap_make_response_ctx_t *,
Copy link
Member

Choose a reason for hiding this comment

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

const coap_make_response_ctx_t * ?

Copy link
Member Author

Choose a reason for hiding this comment

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

renamed coap_validate_request_fn_t -> coap_parse_request_header_fn_t

api/oc_ri.c Outdated Show resolved Hide resolved
api/oc_ri.c Outdated Show resolved Hide resolved
api/oc_ri.c Outdated Show resolved Hide resolved
api/oc_ri_preparsed_request_internal.h Outdated Show resolved Hide resolved
messaging/coap/engine.c Show resolved Hide resolved
api/oc_ri.c Outdated Show resolved Hide resolved
api/oc_ri.c Outdated Show resolved Hide resolved
Danielius1922
Danielius1922 reviewed Jan 19, 2024
View reviewed changes
api/oc_ri.c Outdated Show resolved Hide resolved
Danielius1922
Danielius1922 reviewed Jan 19, 2024
View reviewed changes
api/oc_ri.c Outdated Show resolved Hide resolved
api/oc_ri.c Outdated Show resolved Hide resolved
@jkralik jkralik force-pushed the jkralik/fix/validate-requests branch from 4b098ae to d85704a Compare January 22, 2024 10:49
@jkralik jkralik force-pushed the jkralik/fix/validate-requests branch from d85704a to a87ddd6 Compare January 22, 2024 11:19
@jkralik jkralik added the OCF Conformance Testing OCF Conformance Testing required label Jan 22, 2024
@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Jan 22, 2024 •
edited
Loading

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

1 New issue
0 Security Hotspots
88.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@jkralik jkralik requested a review from Danielius1922 January 22, 2024 16:04
@Danielius1922 Danielius1922 merged commit a882b59 into master Jan 23, 2024
123 checks passed
@Danielius1922 Danielius1922 deleted the jkralik/fix/validate-requests branch January 23, 2024 16:00
@github-actions github-actions bot locked and limited conversation to collaborators Jan 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Danielius1922 Danielius1922 Danielius1922 approved these changes

Assignees
No one assigned
Labels
OCF Conformance Testing OCF Conformance Testing required
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jkralik @Danielius1922