[ASM] Fix Blocking Redirect Invalid Status Code by e-n-0 · Pull Request #6619 · DataDog/dd-trace-dotnet

e-n-0 · 2025-02-03T12:49:32Z

Summary of changes

This PR fixes the returned status code of the response when a request has been blocked. The configured status code from the blocking action was not applied. It was always 302 or 301 that was applied on the response.

Reason for change

Incorrect status code value applied.
(Detected by System Tests: the test wasn't passing).

Implementation details

The Redirect() method from either the .NET Framework or .NET Core sets the status code when called. But in our implementation we were setting the blocking action status code and after we called that method, thus overriding the status code.

Test coverage

Integrations tests have been implemented for both .NET Framework and .NET Core.

e-n-0 · 2025-02-03T12:58:12Z

-                httpResponse.ContentType = action.ContentType;
-                endedResponse = true;
-                return httpResponse.WriteAsync(action.ResponseContent);
+                httpResponse.Headers[HeaderNames.Location] = action.RedirectLocation;


I removed the Redirect() call here. The call was only setting the status code and the Location header.
The status code is already set on above and we set here the header.

datadog-ddstaging · 2025-02-03T13:15:06Z

Datadog Report

Branch report: flavien/asm/incorrect-blocking-status-code
Commit report: 0197050
Test service: dd-trace-dotnet

✅ 0 Failed, 550761 Passed, 4457 Skipped, 32h 53m 10.37s Total Time
❄️ 1 New Flaky

New Flaky Tests (1)

Limits_LargeCollection - Datadog.Trace.Tests.Debugger.DebuggerSnapshotCreatorTests - Last Failure
Expand for error
```
 No token to close. Path ''.
```

andrewlock · 2025-02-03T13:49:26Z

Benchmarks Report for appsec 🐌

Benchmarks for #6619 compared to master:

1 benchmarks are faster, with geometric mean 1.207
1 benchmarks are slower, with geometric mean 1.223
1 benchmarks have fewer allocations
2 benchmarks have more allocations

The following thresholds were used for comparing the benchmark speeds:

Mann–Whitney U test with statistical test for significance of 5%
Only results indicating a difference greater than 10% and 0.3 ns are considered.

Allocation changes below 0.5% are ignored.

Benchmark details

Benchmarks.Trace.Asm.AppSecBodyBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #6619

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody‑netcoreapp3.1	1.223	208.99	255.69

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`AllCycleSimpleBody`	net6.0	202μs	167ns	623ns	2.65	0	188.88 KB
master	`AllCycleSimpleBody`	netcoreapp3.1	295μs	138ns	534ns	2.65	0	196.21 KB
master	`AllCycleSimpleBody`	net472	270μs	314ns	1.22μs	35.9	2.04	226.09 KB
master	`AllCycleMoreComplexBody`	net6.0	212μs	146ns	566ns	2.72	0	192.38 KB
master	`AllCycleMoreComplexBody`	netcoreapp3.1	315μs	77.3ns	289ns	2.67	0	199.62 KB
master	`AllCycleMoreComplexBody`	net472	275μs	154ns	575ns	36.4	1.97	229.6 KB
master	`ObjectExtractorSimpleBody`	net6.0	143ns	0.172ns	0.666ns	0.00395	0	280 B
master	`ObjectExtractorSimpleBody`	netcoreapp3.1	209ns	0.186ns	0.697ns	0.00375	0	272 B
master	`ObjectExtractorSimpleBody`	net472	166ns	0.27ns	0.973ns	0.0446	0	281 B
master	`ObjectExtractorMoreComplexBody`	net6.0	2.92μs	1.84ns	6.89ns	0.0537	0	3.78 KB
master	`ObjectExtractorMoreComplexBody`	netcoreapp3.1	4.04μs	2.21ns	8.57ns	0.0504	0	3.69 KB
master	`ObjectExtractorMoreComplexBody`	net472	3.68μs	8.72ns	33.8ns	0.603	0.00548	3.8 KB
#6619	`AllCycleSimpleBody`	net6.0	212μs	93.6ns	350ns	2.65	0	188.88 KB
#6619	`AllCycleSimpleBody`	netcoreapp3.1	305μs	134ns	501ns	2.59	0	196.21 KB
#6619	`AllCycleSimpleBody`	net472	270μs	244ns	846ns	35.9	2.02	226.09 KB
#6619	`AllCycleMoreComplexBody`	net6.0	213μs	181ns	702ns	2.64	0	192.38 KB
#6619	`AllCycleMoreComplexBody`	netcoreapp3.1	311μs	118ns	442ns	2.64	0	199.63 KB
#6619	`AllCycleMoreComplexBody`	net472	275μs	199ns	718ns	36.4	2.06	229.6 KB
#6619	`ObjectExtractorSimpleBody`	net6.0	143ns	0.151ns	0.587ns	0.00392	0	280 B
#6619	`ObjectExtractorSimpleBody`	netcoreapp3.1	256ns	0.256ns	0.991ns	0.00371	0	272 B
#6619	`ObjectExtractorSimpleBody`	net472	169ns	0.252ns	0.977ns	0.0446	0	281 B
#6619	`ObjectExtractorMoreComplexBody`	net6.0	2.94μs	0.959ns	3.46ns	0.0541	0	3.78 KB
#6619	`ObjectExtractorMoreComplexBody`	netcoreapp3.1	3.87μs	2.59ns	9.34ns	0.0503	0	3.69 KB
#6619	`ObjectExtractorMoreComplexBody`	net472	3.68μs	9.1ns	35.2ns	0.601	0.00548	3.8 KB

Benchmarks.Trace.Asm.AppSecEncoderBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EncodeArgs`	net6.0	37.8μs	22.5ns	87.3ns	0.456	0	32.4 KB
master	`EncodeArgs`	netcoreapp3.1	53.9μs	15.7ns	58.8ns	0.43	0	32.4 KB
master	`EncodeArgs`	net472	66.7μs	28.9ns	108ns	5.13	0.0671	32.5 KB
master	`EncodeLegacyArgs`	net6.0	82.1μs	28.4ns	110ns	0	0	2.14 KB
master	`EncodeLegacyArgs`	netcoreapp3.1	107μs	43ns	161ns	0	0	2.14 KB
master	`EncodeLegacyArgs`	net472	155μs	100ns	388ns	0.311	0	2.15 KB
#6619	`EncodeArgs`	net6.0	39μs	13.6ns	50.7ns	0.454	0	32.4 KB
#6619	`EncodeArgs`	netcoreapp3.1	54.2μs	28.9ns	112ns	0.432	0	32.4 KB
#6619	`EncodeArgs`	net472	67.1μs	98.3ns	381ns	5.15	0.0668	32.5 KB
#6619	`EncodeLegacyArgs`	net6.0	76.5μs	441ns	3.44μs	0	0	2.14 KB
#6619	`EncodeLegacyArgs`	netcoreapp3.1	106μs	321ns	1.24μs	0	0	2.14 KB
#6619	`EncodeLegacyArgs`	net472	157μs	149ns	577ns	0.313	0	2.15 KB

Benchmarks.Trace.Asm.AppSecWafBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunWafRealisticBenchmark`	net6.0	173μs	120ns	464ns	0	2.54 KB
master	`RunWafRealisticBenchmark`	netcoreapp3.1	187μs	304ns	1.18μs	0	2.49 KB
master	`RunWafRealisticBenchmark`	net472	203μs	108ns	418ns	0.401	2.55 KB
master	`RunWafRealisticBenchmarkWithAttack`	net6.0	116μs	51.9ns	180ns	0	1.57 KB
master	`RunWafRealisticBenchmarkWithAttack`	netcoreapp3.1	123μs	149ns	517ns	0	1.55 KB
master	`RunWafRealisticBenchmarkWithAttack`	net472	134μs	108ns	419ns	0.199	1.58 KB
#6619	`RunWafRealisticBenchmark`	net6.0	175μs	268ns	1.04μs	0	2.54 KB
#6619	`RunWafRealisticBenchmark`	netcoreapp3.1	189μs	273ns	1.06μs	0	2.49 KB
#6619	`RunWafRealisticBenchmark`	net472	202μs	86.1ns	334ns	0.399	2.55 KB
#6619	`RunWafRealisticBenchmarkWithAttack`	net6.0	115μs	106ns	412ns	0	1.57 KB
#6619	`RunWafRealisticBenchmarkWithAttack`	netcoreapp3.1	124μs	105ns	406ns	0	1.55 KB
#6619	`RunWafRealisticBenchmarkWithAttack`	net472	133μs	44.4ns	172ns	0.2	1.58 KB

Benchmarks.Trace.Iast.StringAspectsBenchmark - Faster 🎉 More allocations ⚠️

Faster 🎉 in #6619

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑netcoreapp3.1	1.207	64,150.00	53,150.00	bimodal

More allocations ⚠️ in #6619

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net472	60.76 KB	62.09 KB	1.33 KB	2.19%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1	262.15 KB	265.02 KB	2.86 KB	1.09%

Fewer allocations 🎉 in #6619

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net472	278.53 KB	273.52 KB	-5.01 KB	-1.80%

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Allocated
master	`StringConcatBenchmark`	net6.0	52.6μs	261ns	1.11μs	43.44 KB
master	`StringConcatBenchmark`	netcoreapp3.1	64.8μs	878ns	8.78μs	42.64 KB
master	`StringConcatBenchmark`	net472	36.8μs	79.6ns	287ns	60.76 KB
master	`StringConcatAspectBenchmark`	net6.0	300μs	4.4μs	42.7μs	255.38 KB
master	`StringConcatAspectBenchmark`	netcoreapp3.1	327μs	2.63μs	25.8μs	262.15 KB
master	`StringConcatAspectBenchmark`	net472	296μs	7.05μs	69.1μs	278.53 KB
#6619	`StringConcatBenchmark`	net6.0	60.4μs	873ns	8.73μs	43.44 KB
#6619	`StringConcatBenchmark`	netcoreapp3.1	53.2μs	212ns	792ns	42.64 KB
#6619	`StringConcatBenchmark`	net472	36.8μs	79.3ns	275ns	62.09 KB
#6619	`StringConcatAspectBenchmark`	net6.0	322μs	5.9μs	57.8μs	256.19 KB
#6619	`StringConcatAspectBenchmark`	netcoreapp3.1	349μs	1.61μs	12.4μs	265.02 KB
#6619	`StringConcatAspectBenchmark`	net472	287μs	6.05μs	58.7μs	273.52 KB

andrewlock · 2025-02-03T13:53:30Z

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing the following branches/commits:

Execution-time benchmarks measure the whole time it takes to execute a program. And are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are shown in red. The following thresholds were used for comparing the execution times:

Welch test with statistical test for significance of 5%
Only results indicating a difference greater than 5% and 5 ms are considered.

Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard.

Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph).

gantt
    title Execution time (ms) FakeDbCommand (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6619) - mean (69ms)  : 67, 71
     .   : milestone, 69,
    master - mean (69ms)  : 67, 72
     .   : milestone, 69,

    section CallTarget+Inlining+NGEN
    This PR (6619) - mean (992ms)  : 964, 1020
     .   : milestone, 992,
    master - mean (994ms)  : 975, 1013
     .   : milestone, 994,

gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6619) - mean (102ms)  : 100, 104
     .   : milestone, 102,
    master - mean (103ms)  : 100, 105
     .   : milestone, 103,

    section CallTarget+Inlining+NGEN
    This PR (6619) - mean (670ms)  : 654, 687
     .   : milestone, 670,
    master - mean (677ms)  : 662, 692
     .   : milestone, 677,

gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6619) - mean (89ms)  : 87, 91
     .   : milestone, 89,
    master - mean (90ms)  : 88, 92
     .   : milestone, 90,

    section CallTarget+Inlining+NGEN
    This PR (6619) - mean (629ms)  : 609, 648
     .   : milestone, 629,
    master - mean (635ms)  : 618, 651
     .   : milestone, 635,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6619) - mean (189ms)  : 185, 193
     .   : milestone, 189,
    master - mean (191ms)  : 187, 195
     .   : milestone, 191,

    section CallTarget+Inlining+NGEN
    This PR (6619) - mean (1,095ms)  : 1062, 1128
     .   : milestone, 1095,
    master - mean (1,110ms)  : 1062, 1158
     .   : milestone, 1110,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6619) - mean (270ms)  : 264, 275
     .   : milestone, 270,
    master - mean (271ms)  : 266, 276
     .   : milestone, 271,

    section CallTarget+Inlining+NGEN
    This PR (6619) - mean (856ms)  : 821, 891
     .   : milestone, 856,
    master - mean (875ms)  : 806, 944
     .   : milestone, 875,

gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6619) - mean (260ms)  : 255, 265
     .   : milestone, 260,
    master - mean (262ms)  : 258, 265
     .   : milestone, 262,

    section CallTarget+Inlining+NGEN
    This PR (6619) - mean (837ms)  : 801, 874
     .   : milestone, 837,
    master - mean (846ms)  : 808, 884
     .   : milestone, 846,

NachoEchevarria

Thanks!

## Summary of changes This PR fixes the returned status code of the response when a request has been blocked. The configured status code from the blocking action was not applied. It was always `302` or `301` that was applied on the response. ## Reason for change Incorrect status code value applied. (Detected by System Tests: the test wasn't passing). ## Implementation details The `Redirect()` method from either the .NET Framework or .NET Core sets the status code when called. But in our implementation we were setting the blocking action status code and after we called that method, thus overriding the status code. ## Test coverage Integrations tests have been implemented for both .NET Framework and .NET Core.

e-n-0 added 3 commits January 31, 2025 17:49

Fix

de69592

update tests for net framework

fa27320

fix snapshots net core

0197050

github-actions Bot added the area:asm label Feb 3, 2025

e-n-0 commented Feb 3, 2025

View reviewed changes

e-n-0 marked this pull request as ready for review February 3, 2025 13:00

e-n-0 requested review from a team as code owners February 3, 2025 13:00

andrewlock approved these changes Feb 3, 2025

View reviewed changes

NachoEchevarria approved these changes Feb 3, 2025

View reviewed changes

dromanol approved these changes Feb 3, 2025

View reviewed changes

e-n-0 merged commit de72354 into master Feb 3, 2025

e-n-0 deleted the flavien/asm/incorrect-blocking-status-code branch February 3, 2025 16:11

github-actions Bot added this to the vNext-v3 milestone Feb 3, 2025

e-n-0 mentioned this pull request Feb 4, 2025

[dotnet] Enable Custom Blocking Response tests DataDog/system-tests#3971

Merged

7 tasks

andrewlock added the type:bug label Feb 11, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[ASM] Fix Blocking Redirect Invalid Status Code#6619

[ASM] Fix Blocking Redirect Invalid Status Code#6619
e-n-0 merged 3 commits into
masterfrom
flavien/asm/incorrect-blocking-status-code

e-n-0 commented Feb 3, 2025 •

edited

Loading

Uh oh!

e-n-0 Feb 3, 2025

Uh oh!

datadog-ddstaging Bot commented Feb 3, 2025 •

edited

Loading

Uh oh!

andrewlock commented Feb 3, 2025

Slower ⚠️ in #6619

Raw results

Raw results

Raw results

Faster 🎉 in #6619

More allocations ⚠️ in #6619

Fewer allocations 🎉 in #6619

Raw results

Uh oh!

andrewlock commented Feb 3, 2025

Uh oh!

NachoEchevarria left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

e-n-0 commented Feb 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Reason for change

Implementation details

Test coverage

Uh oh!

e-n-0 Feb 3, 2025

Choose a reason for hiding this comment

Uh oh!

datadog-ddstaging Bot commented Feb 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Datadog Report

New Flaky Tests (1)

Uh oh!

andrewlock commented Feb 3, 2025

Benchmarks Report for appsec 🐌

Benchmark details

Slower ⚠️ in #6619

Raw results

Raw results

Raw results

Faster 🎉 in #6619

More allocations ⚠️ in #6619

Fewer allocations 🎉 in #6619

Raw results

Uh oh!

andrewlock commented Feb 3, 2025

Execution-Time Benchmarks Report ⏱️

Uh oh!

NachoEchevarria left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

e-n-0 commented Feb 3, 2025 •

edited

Loading

datadog-ddstaging Bot commented Feb 3, 2025 •

edited

Loading