Object view: read only abstraction to ddwaf_object#341
Conversation
Codecov ReportAttention: Patch coverage is
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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
63d6557 to
98c50a8
Compare
… anilm3/object_view
cataphract
left a comment
There was a problem hiding this comment.
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 ¶m = *(*it); | ||
| if (param.type != DDWAF_OBJ_STRING) { | ||
| const object_view param = *it; | ||
| if (!param.is<std::string_view>()) { |
There was a problem hiding this comment.
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
| 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>>>> |
There was a problem hiding this comment.
might as well modernize for c++20 here too, like below
* 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)
This PR introduces the
object_viewclass which is a zero-cost abstraction on top ofddwaf_objectwith the objective of:ddwaf_objectis 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).