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

refactor: request validation check #394

Merged
merged 14 commits into from
Aug 4, 2023
Merged

Conversation

bsbds
Copy link
Collaborator

@bsbds bsbds commented Jul 21, 2023

Described in: #398
Based on: #399

Please briefly answer these questions:

  • what problem are you trying to solve? (or if there's no problem, what's the motivation for this change?)

    Currently some of the validation checks are done in the RPC server. However, these checks are not applied in the storage request handlers (eg. handle_range_request), so that the request from outside client will not be checked.

  • what changes does this pull request make?

    This pull request applied the validation check in both RPC server and the storage handlers.

  • are there any non-obvious implications of these changes? (does it break compatibility with previous versions, etc)

@codecov
Copy link

codecov bot commented Jul 21, 2023

Codecov Report

Patch coverage: 82.44% and project coverage change: +1.01% 🎉

Comparison is base (6f1cff4) 52.44% compared to head (38eab5e) 53.46%.
Report is 11 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #394      +/-   ##
==========================================
+ Coverage   52.44%   53.46%   +1.01%     
==========================================
  Files          98      100       +2     
  Lines       15100    15531     +431     
  Branches    15100    15531     +431     
==========================================
+ Hits         7919     8303     +384     
- Misses       6686     6727      +41     
- Partials      495      501       +6     
Files Changed Coverage Δ
xline/src/lib.rs 100.00% <ø> (ø)
xline/src/server/auth_server.rs 0.00% <0.00%> (ø)
xline/src/storage/auth_store/store.rs 56.75% <0.00%> (-0.16%) ⬇️
xline/src/storage/kv_store.rs 71.69% <65.38%> (+0.68%) ⬆️
xline/src/server/kv_server.rs 41.77% <81.08%> (-10.52%) ⬇️
xline/src/request_validation.rs 84.41% <84.41%> (ø)
xline/src/revision_check.rs 91.66% <91.66%> (ø)
xline/src/storage/execute_error.rs 35.71% <100.00%> (+35.71%) ⬆️

... and 8 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bsbds bsbds linked an issue Jul 26, 2023 that may be closed by this pull request
@bsbds bsbds force-pushed the add-check branch 4 times, most recently from 950e0e1 to cc0639a Compare July 26, 2023 11:37
@bsbds bsbds marked this pull request as ready for review August 3, 2023 12:53
xline/src/storage/kv_store.rs Outdated Show resolved Hide resolved
xline/src/storage/kv_store.rs Show resolved Hide resolved
xline/src/storage/kv_store.rs Show resolved Hide resolved
xline/src/storage/kv_store.rs Outdated Show resolved Hide resolved
xline/src/storage/kv_store.rs Outdated Show resolved Hide resolved
xline/src/server/auth_server.rs Show resolved Hide resolved
xline/src/storage/kv_store.rs Outdated Show resolved Hide resolved
xline-client/tests/kv.rs Outdated Show resolved Hide resolved
xline/src/request_validation.rs Show resolved Hide resolved
xline/src/request_validation.rs Show resolved Hide resolved
xline/src/lib.rs Outdated Show resolved Hide resolved
@mergify mergify bot merged commit b4ef3c8 into xline-kv:master Aug 4, 2023
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.

[Refactor] Request validation
5 participants