Skip to content

Object view: read only abstraction to ddwaf_object#341

Merged
Anilm3 merged 81 commits into
anilm3/v2from
anilm3/object_view
Mar 12, 2025
Merged

Object view: read only abstraction to ddwaf_object#341
Anilm3 merged 81 commits into
anilm3/v2from
anilm3/object_view

Conversation

@Anilm3

@Anilm3 Anilm3 commented Oct 1, 2024

Copy link
Copy Markdown
Collaborator

This PR introduces the object_view class which is a zero-cost abstraction on top of ddwaf_object with the objective of:

  • Providing a more convenient interface to access the provided data.
  • Adding generalised debug assertions which apply to every access and use of objects.
  • Abstracting away the internals of how a ddwaf_object is structured, paving the way for the changes introduced in Object layout and abstraction #293.

In addition, further guarantees and checks can be added to each access, for now the interface mimics the use of an std::optional, where a check must be performed before an access. This applies to accessing fields (nullability check), accessing elements (size check) or accessing the contained values (type check).

@codecov-commenter

codecov-commenter commented Oct 1, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 83.19672% with 82 lines in your changes missing coverage. Please review.

Project coverage is 84.90%. Comparing base (b6d3ae7) to head (44127f9).
Report is 63 commits behind head on anilm3/v2.

Files with missing lines Patch % Lines
src/object_view.hpp 70.62% 4 Missing and 38 partials ⚠️
src/iterator.cpp 89.52% 0 Missing and 11 partials ⚠️
src/iterator.hpp 68.75% 1 Missing and 4 partials ⚠️
src/condition/cmdi_detector.cpp 84.61% 0 Missing and 4 partials ⚠️
src/exclusion/object_filter.cpp 66.66% 0 Missing and 4 partials ⚠️
src/condition/match_iterator.hpp 57.14% 0 Missing and 3 partials ⚠️
src/processor/extract_schema.cpp 83.33% 0 Missing and 3 partials ⚠️
src/processor/fingerprint.cpp 88.46% 0 Missing and 3 partials ⚠️
src/argument_retriever.hpp 50.00% 0 Missing and 2 partials ⚠️
src/condition/shi_common.cpp 75.00% 0 Missing and 2 partials ⚠️
... and 3 more
Additional details and impacted files
@@              Coverage Diff              @@
##           anilm3/v2     #341      +/-   ##
=============================================
- Coverage      85.13%   84.90%   -0.23%     
=============================================
  Files            164      167       +3     
  Lines           8232     8385     +153     
  Branches        3609     3655      +46     
=============================================
+ Hits            7008     7119     +111     
- Misses           462      465       +3     
- Partials         762      801      +39     
Flag Coverage Δ
waf_test 84.90% <83.19%> (-0.23%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@pr-commenter

pr-commenter Bot commented Oct 1, 2024

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2025-03-12 15:10:54

Comparing candidate commit 44127f9 in PR branch anilm3/object_view with baseline commit b6d3ae7 in branch anilm3/v2.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics.

@Anilm3
Anilm3 force-pushed the anilm3/object_view branch from 63d6557 to 98c50a8 Compare October 3, 2024 18:23
@Anilm3
Anilm3 changed the base branch from master to anilm3/v2 October 9, 2024 09:36
@Anilm3
Anilm3 marked this pull request as ready for review March 3, 2025 21:57
@Anilm3
Anilm3 requested a review from a team as a code owner March 3, 2025 21:57
@Anilm3
Anilm3 changed the base branch from master to anilm3/v2 March 4, 2025 13:13

@cataphract cataphract left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nt much to say besides the reservations/suggestions I already transmitted on slack (null safety/type safety/better abstraction for keyed values). I also find it a bit template-happy in certain places, which hurts readability (esp. when the template keyword is needed on call sites) and makes for worse discover/error messages.

const ddwaf_object &param = *(*it);
if (param.type != DDWAF_OBJ_STRING) {
const object_view param = *it;
if (!param.is<std::string_view>()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not just is_string, is_unsigned, is_signed, is_bool? I mean, this doesn't read right. param is is not a string_view

Comment thread src/mkmap.hpp
namespace ddwaf {
template <typename Key, typename T, class Compare = std::less<Key>,
typename = std::enable_if_t<std::is_copy_constructible_v<std::remove_cv_t<std::decay_t<T>>>>>
typename = std::enable_if_t<std::is_copy_constructible_v<std::decay_t<T>>>>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

might as well modernize for c++20 here too, like below

@Anilm3
Anilm3 merged commit 3dbe612 into anilm3/v2 Mar 12, 2025
@Anilm3
Anilm3 deleted the anilm3/object_view branch March 12, 2025 19:55
Anilm3 added a commit that referenced this pull request Oct 1, 2025
* Object view: read only abstraction to ddwaf_object (#341)
* [v2] Remove mingw builds (#381)
* Writable objects: owned and borrowed object and object limits removal (#378, #382)
* [v2] Refactor and improve object types (#387)
* [v2] Update unit tests to use new abstractions (#389)
* [v2] Update `raw_configuration` type to use `object_view` (#390)
* [v2] Remove remaining uses of ddwaf_object in `src` and `tests/unit` (#391)
* [v2] JWT Decoding Processor (#401)
* [v2] First iteration of object layout changes (#394)
* Split context data insertion from evaluation (#407)
* [v2] Second iteration of object layout changes (#408)
* [v2] Add new fingerprint and object view tests (#414)
* Reenable attribute collector unit test (#415)
* [v2] Container view types (#413)
* Exclude assertions from coverage (#416)
* [v2] Use allocators internally instead of malloc/free and stop generating zero-terminated strings (#418)
* Add memory resource to owned and borrowed objects (#428)
* [v2] Propagate allocators from context (#420)
* [v2] Update interface and expose allocators (#427)
* Refactor evaluation stages out of the context (#442)
* Subcontext: replace ephemerals with a new scope with user-defined lifetime derived from the context (#443)
* Validator: Add support for testing subcontexts and attributes (#451)
* [v2] Pass allocator to context and subcontext eval and add new allocators (#452)
* Update logger to avoid dependencies on ddwaf.h (#453)
* [v2] Return DDWAF_MATCH when there are events, attributes or actions (#455)
* [v2] Cleanup: remove exclusion namespace and some redundant references (#456)
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.

3 participants