Skip to content

Conversation

estringana
Copy link
Contributor

@estringana estringana commented Oct 13, 2025

Description

Display the new block_id provided by the latest libddwaf version. This block id is provided on blocks and redirects. It is displayed to the user on html, json and redirections.

The placeholder for block id on the template is {block_id}. There can be multiple placeholders in the same template

Additionally, messaging around requests blocked/redirected has been reworded as some customer found them confusing.

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@estringana estringana changed the title Replace block id on redirections on the extension Print block_id Oct 13, 2025
@codecov-commenter
Copy link

codecov-commenter commented Oct 13, 2025

Codecov Report

❌ Patch coverage is 90.38462% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.04%. Comparing base (9298487) to head (d7e9379).

Files with missing lines Patch % Lines
appsec/src/extension/request_abort.c 90.22% 12 Missing and 1 partial ⚠️
appsec/src/extension/commands_helpers.c 90.47% 0 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3444      +/-   ##
==========================================
+ Coverage   61.89%   62.04%   +0.14%     
==========================================
  Files         141      141              
  Lines       12501    12624     +123     
  Branches     1633     1651      +18     
==========================================
+ Hits         7737     7832      +95     
- Misses       4042     4065      +23     
- Partials      722      727       +5     
Files with missing lines Coverage Δ
appsec/src/extension/ddappsec.c 74.18% <100.00%> (+0.08%) ⬆️
appsec/src/extension/request_abort.h 100.00% <ø> (ø)
appsec/src/extension/request_lifecycle.c 63.36% <100.00%> (+0.04%) ⬆️
appsec/src/extension/commands_helpers.c 68.65% <90.47%> (-1.24%) ⬇️
appsec/src/extension/request_abort.c 74.11% <90.22%> (+3.97%) ⬆️

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9298487...d7e9379. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pr-commenter
Copy link

pr-commenter bot commented Oct 13, 2025

Benchmarks [ appsec ]

Benchmark execution time: 2025-10-15 14:58:51

Comparing candidate commit d7e9379 in PR branch estringana/add-block_id with baseline commit 9298487 in branch master.

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

@estringana estringana force-pushed the estringana/add-block_id branch from 252d896 to b861a80 Compare October 13, 2025 11:32
@estringana estringana force-pushed the estringana/add-block_id branch from b861a80 to fc35f35 Compare October 13, 2025 11:57
@estringana estringana marked this pull request as ready for review October 14, 2025 14:04
@estringana estringana requested a review from a team as a code owner October 14, 2025 14:04
Copy link
Contributor

@cataphract cataphract left a comment

Choose a reason for hiding this comment

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

I also think adding 8 new tests is excessive, especially because the old tests already were changed to cover the new functionality. Maybe leave only the new tests with custom templates -- and add the placeholder more than once.

@estringana
Copy link
Contributor Author

@cataphract thanks for the review. I have addressed all comments.

@estringana estringana force-pushed the estringana/add-block_id branch from ab7f27b to 3787fd4 Compare October 14, 2025 15:54
"p{font-size:16px}</style></head><body><main><p>Sorry, you cannot access "
"this page. Please contact the customer service "
"team.</p></main><footer><p>Security provided by <a "
"team.</br> Block ID: {block_id}</p></main><footer><p>Security provided by "
Copy link
Contributor

Choose a reason for hiding this comment

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

This template hasn't been standardised, AFAIK?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not yet. But when it gets done, all I will have to do is to change this

@estringana estringana force-pushed the estringana/add-block_id branch from fc4552d to 868eadc Compare October 15, 2025 14:14
@estringana estringana force-pushed the estringana/add-block_id branch from 868eadc to d7e9379 Compare October 15, 2025 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants