Skip to content

Drop redundant eacl checks#3481

Merged
cthulhu-rider merged 3 commits intomasterfrom
redundant-eacl-checks
Jul 29, 2025
Merged

Drop redundant eacl checks#3481
cthulhu-rider merged 3 commits intomasterfrom
redundant-eacl-checks

Conversation

@roman-khimov
Copy link
Member

Fixes #3095.

@codecov
Copy link

codecov bot commented Jul 24, 2025

Codecov Report

❌ Patch coverage is 0% with 42 lines in your changes missing coverage. Please review.
✅ Project coverage is 23.34%. Comparing base (ba3431d) to head (53c5106).
⚠️ Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
pkg/services/object/server.go 0.00% 38 Missing ⚠️
pkg/services/object/acl/acl.go 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3481      +/-   ##
==========================================
+ Coverage   23.15%   23.34%   +0.19%     
==========================================
  Files         669      669              
  Lines       50251    50252       +1     
==========================================
+ Hits        11634    11730      +96     
+ Misses      37708    37606     -102     
- Partials      909      916       +7     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@roman-khimov roman-khimov force-pushed the redundant-eacl-checks branch from 3709c89 to d2d7a33 Compare July 28, 2025 11:57
GetRange operates with byte chunks, the result doesn't have any new additional
data compared to request processing time, if we had an object header we still
have it, if we had no object header we still don't have it, the request itself
hasn't change, so this check is no-op, it can't affect the result.

Refs #3095.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
Search result is just IDs, the result doesn't have any new additional data
compared to request processing time, if we had an object header we still
have it, if we had no object header we still don't have it, the request itself
hasn't change, so this check is no-op, it can't affect the result.

Notice that SearchV2 doesn't do this at all. Refs #3095.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
Get and Head handlers check EACL before doing anything (just like other
handlers do), but if there are rules based on object headers and this object
is not stored locally the action would be allowed. Once we get a header from
the other node EACL can and will always be rechecked which is suboptimal since
in most cases it's the same simple (non-filter-dependent) set of rules. The
patch tries to optimize it by avoiding rechecks if there was a rule match.
If that's the case another EACL check would yield the same (positive) result
anyway, so it's just a waste of time.

Notice that from the security perspective this is still questionable since if
the node storing object (erroneously) returns data this means the data can
technically be retrieved by asking it directly.

Closes #3095.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
@roman-khimov roman-khimov force-pushed the redundant-eacl-checks branch from d2d7a33 to 53c5106 Compare July 29, 2025 09:32
@cthulhu-rider cthulhu-rider merged commit a4145c5 into master Jul 29, 2025
22 of 24 checks passed
@cthulhu-rider cthulhu-rider deleted the redundant-eacl-checks branch July 29, 2025 09:47
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.

Avoid checking the response in object service

3 participants