Skip to content

Support other scalar types on action parameters#441

Merged
Anilm3 merged 6 commits into
masterfrom
anilm3/support-other-action-types
Sep 1, 2025
Merged

Support other scalar types on action parameters#441
Anilm3 merged 6 commits into
masterfrom
anilm3/support-other-action-types

Conversation

@Anilm3

@Anilm3 Anilm3 commented Aug 18, 2025 •
edited by atlassian Bot
Loading

Copy link
Copy Markdown
Collaborator

This PR extends the set of allowed types on action parameters to include all other scalars. This is a small improvement which can be taken advantage of to prevent intermediate type conversions. In v2, other complex types will also be supported.

Related Jiras: APPSEC-58779

@codecov-commenter

codecov-commenter commented Aug 18, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.85507% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.15%. Comparing base (62dca00) to head (c4d828f).
⚠️ Report is 34 commits behind head on master.

Files with missing lines Patch % Lines
src/configuration/actions_parser.cpp 81.48% 1 Missing and 4 partials ⚠️
src/configuration/common/raw_configuration.cpp 96.66% 0 Missing and 1 partial ⚠️
src/serializer.cpp 91.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #441      +/-   ##
==========================================
+ Coverage   85.10%   85.15%   +0.04%     
==========================================
  Files         180      180              
  Lines        9226     9283      +57     
  Branches     4084     4116      +32     
==========================================
+ Hits         7852     7905      +53     
- Misses        548      550       +2     
- Partials      826      828       +2
Flag Coverage Δ
waf_test 85.15% <89.85%> (+0.04%) ⬆️

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.

@github-actions

github-actions Bot commented Aug 18, 2025 •
edited
Loading

Copy link
Copy Markdown

Artifact Size Comparison 📦

Artifact Previous Release This PR Difference
darwin-arm64::libddwaf.a 86105528 89505960 0.03%
darwin-arm64::libddwaf.a.stripped 4442896 4543304 0.02%
darwin-arm64::libddwaf.dylib 1920368 1960768 0.02%
darwin-universal::libddwaf.a 173548224 180426024 0.03%
darwin-universal::libddwaf.a.stripped 9465600 9685040 0.02%
darwin-universal::libddwaf.dylib 4050288 4139840 0.02%
darwin-x86_64::libddwaf.a 87442648 90920016 0.03%
darwin-x86_64::libddwaf.a.stripped 5022656 5141688 0.02%
darwin-x86_64::libddwaf.dylib 2107008 2147768 0.01%
linux-aarch64::libddwaf.a 70896324 72591856 0.02%
linux-aarch64::libddwaf.a.stripped 11575224 11765868 0.01%
linux-aarch64::libddwaf.so 2427912 2452088 0.00%
linux-armv7::libddwaf.a 62633514 64215966 0.02%
linux-armv7::libddwaf.a.stripped 10576914 10768430 0.01%
linux-armv7::libddwaf.so 2108812 2137868 0.01%
linux-i386::libddwaf.a 60811086 62369460 0.02%
linux-i386::libddwaf.a.stripped 9140778 9311580 0.01%
linux-i386::libddwaf.so 2346940 2381300 0.01%
linux-x86_64::libddwaf.a 71366196 73077672 0.02%
linux-x86_64::libddwaf.a.stripped 11397112 11587372 0.01%
linux-x86_64::libddwaf.so 2616768 2648016 0.01%
windows-arm64::ddwaf.dll 4769280 ∞
windows-arm64::ddwaf.lib 11698 ∞
windows-arm64::ddwaf_static.lib 57421528 ∞
windows-win32::ddwaf.dll 3298304 3355648 0.01%
windows-win32::ddwaf.lib 11922 11922 0.00%
windows-win32::ddwaf_static.lib 47761216 49101894 0.02%
windows-x64::ddwaf.dll 4030976 4089344 0.01%
windows-x64::ddwaf.lib 11698 11698 0.00%
windows-x64::ddwaf_static.lib 55348460 56982096 0.02%
windows-x86_64::libddwaf.a 6263042 6457332 0.03%
windows-x86_64::libddwaf.dll 18798493 18853216 0.00%
windows-x86_64::libddwaf.dll.a 31948 31948 0.00%

@pr-commenter

pr-commenter Bot commented Aug 18, 2025 •
edited
Loading

Copy link
Copy Markdown

Benchmarks gcc

Benchmark execution time: 2025-09-01 09:09:39

Comparing candidate commit c4d828f in PR branch anilm3/support-other-action-types with baseline commit 62dca00 in branch master.

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

@pr-commenter

pr-commenter Bot commented Aug 18, 2025 •
edited
Loading

Copy link
Copy Markdown

Benchmarks clang-pgo

Benchmark execution time: 2025-08-31 21:33:50

Comparing candidate commit cb2fe12 in PR branch anilm3/support-other-action-types with baseline commit 2ccf343 in branch master.

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

@pr-commenter

pr-commenter Bot commented Aug 21, 2025 •
edited
Loading

Copy link
Copy Markdown

Benchmarks clang

Benchmark execution time: 2025-09-01 09:08:29

Comparing candidate commit c4d828f in PR branch anilm3/support-other-action-types with baseline commit 62dca00 in branch master.

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

scenario:global-benchmark.random.clang

  • 🟩 execution_time [-40.907ms; -40.895ms] or [-14.285%; -14.281%]

cataphract
cataphract approved these changes Aug 28, 2025
View reviewed changes

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

Looks good enough, but I wonder what the point is of having an additional tagged union representation that you create from ddwaf_object (and then back), when ddwaf_object is already a tagged union capable of representing your values.

Comment thread src/configuration/actions_parser.cpp Outdated
Comment thread src/configuration/actions_parser.cpp Outdated
@Anilm3

Anilm3 commented Aug 28, 2025

Copy link
Copy Markdown
Collaborator Author

Looks good enough, but I wonder what the point is of having an additional tagged union representation that you create from ddwaf_object (and then back), when ddwaf_object is already a tagged union capable of representing your values.

Primarily to avoid having to manage cleanups, I will indeed use owned_object in v2.

@Anilm3
Anilm3 force-pushed the anilm3/support-other-action-types branch 2 times, most recently from f3e110f to cb2fe12 Compare August 31, 2025 20:57
@Anilm3
Anilm3 marked this pull request as ready for review September 1, 2025 08:16
@Anilm3
Anilm3 requested a review from a team as a code owner September 1, 2025 08:16
@Anilm3
Anilm3 force-pushed the anilm3/support-other-action-types branch from cb2fe12 to 6653205 Compare September 1, 2025 08:17
@Anilm3
Anilm3 enabled auto-merge (squash) September 1, 2025 08:34
@Anilm3
Anilm3 merged commit 1b2736a into master Sep 1, 2025
56 of 57 checks passed
@Anilm3
Anilm3 deleted the anilm3/support-other-action-types branch September 1, 2025 09:14
@Anilm3 Anilm3 mentioned this pull request Sep 3, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cataphract cataphract cataphract approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Anilm3 @codecov-commenter @cataphract