Skip to content

[ASM] Fix errors exceptions on HttpContext get_Items#6592

Merged
e-n-0 merged 5 commits into
masterfrom
flavien/asm/httpcontext-items-nullreference
Feb 5, 2025
Merged

[ASM] Fix errors exceptions on HttpContext get_Items#6592
e-n-0 merged 5 commits into
masterfrom
flavien/asm/httpcontext-items-nullreference

Conversation

@e-n-0

@e-n-0 e-n-0 commented Jan 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Summary of changes

Handle the errors when trying to access the Items features of the HttpContext.

All usage of Context.Items int the SecurityCoordinator has been changed so the errors are caught.

Should fix these example stacktraces:

System.NullReferenceException
   at Microsoft.AspNetCore.Http.DefaultHttpContext.get_Items()
   at Datadog.Trace.AppSec.Coordinator.SecurityCoordinator.HttpTransport.get_ReportedExternalWafsRequestHeaders()
   at Datadog.Trace.AppSec.Coordinator.SecurityCoordinator.TryReport(IResult result, Boolean blocked, Nullable`1 status)
   at Datadog.Trace.AppSec.Coordinator.SecurityCoordinator.ReportAndBlock(IResult result)
   at Datadog.Trace.AppSec.Rasp.RaspModule.RunWafRasp(Dictionary`2 arguments, Span rootSpan, String address)
   at Datadog.Trace.AppSec.VulnerabilitiesModule.OnSSRF(String uriText)
   at Datadog.Trace.Iast.Aspects.System.Net.HttpClientAspect.Review(String parameter)

System.ObjectDisposedException
   at Microsoft.AspNetCore.Http.Features.FeatureReferences`1.ThrowContextDisposed()
   at Microsoft.AspNetCore.Http.DefaultHttpContext.get_Items()
   at Datadog.Trace.AppSec.Coordinator.SecurityCoordinator.HttpTransport.get_ReportedExternalWafsRequestHeaders()
   at Datadog.Trace.AppSec.Coordinator.SecurityReporter.TryReport(IResult result, Boolean blocked, Nullable`1 status)
   at Datadog.Trace.AppSec.Coordinator.SecurityCoordinator.ReportAndBlock(IResult result)
   at Datadog.Trace.AppSec.Rasp.RaspModule.RunWafRasp(Dictionary`2 arguments, Span rootSpan, String address)
   at Datadog.Trace.AppSec.VulnerabilitiesModule.OnSSRF(String uriText)
   at Datadog.Trace.Iast.Aspects.System.Net.HttpClientAspect.Review(String parameter)

Reason for change

A ObjectDisposedException and a NullReferenceException can happen in concurrent access to the HttpContext.
This situation of race condition can happen while the request is finished while async jobs still run in the background and the waf for RASP is run.

The HttpContext can be disposed from different manner by the user. You can find more information in this PR.

Implementation details

A new boolean have been added that reflect the state if the Items threw an exception, thus making the code not access the HttpContext anymore in the TryReport method.

Test coverage

Added a unit test verifying that our code doesn't throw exceptions when the Items of the HttpContext isn't available.

Other details

Related PRs:

@datadog-ddstaging

datadog-ddstaging Bot commented Jan 24, 2025 •
edited
Loading

Copy link
Copy Markdown

Datadog Report

Branch report: flavien/asm/httpcontext-items-nullreference
Commit report: 1f4749b
Test service: dd-trace-dotnet

✅ 0 Failed, 242639 Passed, 2093 Skipped, 19h 38m 41.38s Total Time

@andrewlock

andrewlock commented Jan 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

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 (6592) - mean (69ms)  : 66, 72
     .   : milestone, 69,
    master - mean (69ms)  : 67, 72
     .   : milestone, 69,

    section CallTarget+Inlining+NGEN
    This PR (6592) - mean (981ms)  : 958, 1005
     .   : milestone, 981,
    master - mean (983ms)  : 953, 1012
     .   : milestone, 983,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6592) - mean (107ms)  : 105, 109
     .   : milestone, 107,
    master - mean (108ms)  : 106, 110
     .   : milestone, 108,

    section CallTarget+Inlining+NGEN
    This PR (6592) - mean (672ms)  : 657, 686
     .   : milestone, 672,
    master - mean (678ms)  : 662, 695
     .   : milestone, 678,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6592) - mean (91ms)  : 89, 93
     .   : milestone, 91,
    master - mean (91ms)  : 89, 94
     .   : milestone, 91,

    section CallTarget+Inlining+NGEN
    This PR (6592) - mean (629ms)  : 613, 645
     .   : milestone, 629,
    master - mean (632ms)  : 615, 648
     .   : milestone, 632,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6592) - mean (190ms)  : 185, 195
     .   : milestone, 190,
    master - mean (190ms)  : 185, 195
     .   : milestone, 190,

    section CallTarget+Inlining+NGEN
    This PR (6592) - mean (1,087ms)  : 1058, 1115
     .   : milestone, 1087,
    master - mean (1,083ms)  : 1060, 1106
     .   : milestone, 1083,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6592) - mean (275ms)  : 270, 280
     .   : milestone, 275,
    master - mean (276ms)  : 273, 280
     .   : milestone, 276,

    section CallTarget+Inlining+NGEN
    This PR (6592) - mean (861ms)  : 829, 894
     .   : milestone, 861,
    master - mean (868ms)  : 834, 901
     .   : milestone, 868,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6592) - mean (263ms)  : 260, 267
     .   : milestone, 263,
    master - mean (264ms)  : 259, 268
     .   : milestone, 264,

    section CallTarget+Inlining+NGEN
    This PR (6592) - mean (843ms)  : 809, 877
     .   : milestone, 843,
    master - mean (842ms)  : 809, 875
     .   : milestone, 842,
Loading

@andrewlock

andrewlock commented Jan 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Benchmarks Report for appsec 🐌

Benchmarks for #6592 compared to master:

  • 1 benchmarks are faster, with geometric mean 1.173
  • All benchmarks have the same 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 - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6592

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorMoreComplexBody‑net472 1.173 4,300.32 3,666.44

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master AllCycleSimpleBody net6.0 205μs 115ns 429ns 2.63 0 0 188.69 KB
master AllCycleSimpleBody netcoreapp3.1 307μs 288ns 1.08μs 2.6 0 0 196.02 KB
master AllCycleSimpleBody net472 265μs 158ns 591ns 35.8 1.98 0 225.75 KB
master AllCycleMoreComplexBody net6.0 206μs 79ns 296ns 2.67 0 0 192.19 KB
master AllCycleMoreComplexBody netcoreapp3.1 325μs 136ns 508ns 2.71 0 0 199.43 KB
master AllCycleMoreComplexBody net472 273μs 69.2ns 250ns 36.4 2.07 0 229.27 KB
master ObjectExtractorSimpleBody net6.0 141ns 0.0765ns 0.286ns 0.00391 0 0 280 B
master ObjectExtractorSimpleBody netcoreapp3.1 198ns 0.129ns 0.445ns 0.00371 0 0 272 B
master ObjectExtractorSimpleBody net472 164ns 0.293ns 1.14ns 0.0446 0 0 281 B
master ObjectExtractorMoreComplexBody net6.0 2.88μs 2.15ns 7.76ns 0.0536 0 0 3.78 KB
master ObjectExtractorMoreComplexBody netcoreapp3.1 3.87μs 3.76ns 14.1ns 0.0501 0 0 3.69 KB
master ObjectExtractorMoreComplexBody net472 4.3μs 1.92ns 7.43ns 0.603 0.00648 0 3.8 KB
#6592 AllCycleSimpleBody net6.0 204μs 49.4ns 178ns 2.66 0 0 188.69 KB
#6592 AllCycleSimpleBody netcoreapp3.1 304μs 158ns 568ns 2.58 0 0 196.02 KB
#6592 AllCycleSimpleBody net472 267μs 154ns 577ns 35.8 2 0 225.75 KB
#6592 AllCycleMoreComplexBody net6.0 209μs 92.6ns 346ns 2.7 0 0 192.19 KB
#6592 AllCycleMoreComplexBody netcoreapp3.1 308μs 118ns 427ns 2.59 0 0 199.43 KB
#6592 AllCycleMoreComplexBody net472 276μs 192ns 691ns 36.4 2.07 0 229.27 KB
#6592 ObjectExtractorSimpleBody net6.0 142ns 0.177ns 0.684ns 0.00392 0 0 280 B
#6592 ObjectExtractorSimpleBody netcoreapp3.1 195ns 0.155ns 0.6ns 0.00373 0 0 272 B
#6592 ObjectExtractorSimpleBody net472 164ns 0.173ns 0.67ns 0.0446 0 0 281 B
#6592 ObjectExtractorMoreComplexBody net6.0 3.03μs 1.46ns 5.45ns 0.0535 0 0 3.78 KB
#6592 ObjectExtractorMoreComplexBody netcoreapp3.1 3.8μs 2.23ns 8.63ns 0.0511 0 0 3.69 KB
#6592 ObjectExtractorMoreComplexBody net472 3.67μs 1.74ns 6.29ns 0.603 0.00553 0 3.8 KB
Benchmarks.Trace.Asm.AppSecEncoderBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EncodeArgs net6.0 38.3μs 12.8ns 44.2ns 0.441 0 0 32.4 KB
master EncodeArgs netcoreapp3.1 54.3μs 21.2ns 79.3ns 0.439 0 0 32.4 KB
master EncodeArgs net472 67.1μs 31.9ns 123ns 5.15 0.0668 0 32.5 KB
master EncodeLegacyArgs net6.0 71.2μs 19.3ns 72.1ns 0 0 0 2.14 KB
master EncodeLegacyArgs netcoreapp3.1 106μs 134ns 517ns 0 0 0 2.14 KB
master EncodeLegacyArgs net472 153μs 97.6ns 365ns 0.309 0 0 2.15 KB
#6592 EncodeArgs net6.0 39.2μs 27.8ns 108ns 0.451 0 0 32.4 KB
#6592 EncodeArgs netcoreapp3.1 54.2μs 27.7ns 107ns 0.431 0 0 32.4 KB
#6592 EncodeArgs net472 66.7μs 70.5ns 273ns 5.15 0.0665 0 32.5 KB
#6592 EncodeLegacyArgs net6.0 77.6μs 433ns 2.67μs 0 0 0 2.14 KB
#6592 EncodeLegacyArgs netcoreapp3.1 105μs 371ns 1.44μs 0 0 0 2.14 KB
#6592 EncodeLegacyArgs net472 152μs 85ns 329ns 0.304 0 0 2.15 KB
Benchmarks.Trace.Asm.AppSecWafBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunWafRealisticBenchmark net6.0 173μs 84.1ns 315ns 0 0 0 2.54 KB
master RunWafRealisticBenchmark netcoreapp3.1 188μs 214ns 828ns 0 0 0 2.49 KB
master RunWafRealisticBenchmark net472 200μs 87ns 337ns 0.401 0 0 2.55 KB
master RunWafRealisticBenchmarkWithAttack net6.0 116μs 71.8ns 278ns 0 0 0 1.57 KB
master RunWafRealisticBenchmarkWithAttack netcoreapp3.1 123μs 68ns 245ns 0 0 0 1.55 KB
master RunWafRealisticBenchmarkWithAttack net472 132μs 82.2ns 318ns 0.198 0 0 1.58 KB
#6592 RunWafRealisticBenchmark net6.0 173μs 221ns 855ns 0 0 0 2.54 KB
#6592 RunWafRealisticBenchmark netcoreapp3.1 189μs 308ns 1.15μs 0 0 0 2.49 KB
#6592 RunWafRealisticBenchmark net472 201μs 108ns 419ns 0.402 0 0 2.55 KB
#6592 RunWafRealisticBenchmarkWithAttack net6.0 117μs 93.6ns 363ns 0 0 0 1.57 KB
#6592 RunWafRealisticBenchmarkWithAttack netcoreapp3.1 124μs 114ns 442ns 0 0 0 1.55 KB
#6592 RunWafRealisticBenchmarkWithAttack net472 135μs 202ns 781ns 0.201 0 0 1.58 KB
Benchmarks.Trace.Iast.StringAspectsBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StringConcatBenchmark net6.0 52.2μs 237ns 1.09μs 0 0 0 43.44 KB
master StringConcatBenchmark netcoreapp3.1 54.8μs 292ns 1.49μs 0 0 0 42.64 KB
master StringConcatBenchmark net472 37.3μs 94.8ns 355ns 0 0 0 59.12 KB
master StringConcatAspectBenchmark net6.0 303μs 1.45μs 5.99μs 0 0 0 254.32 KB
master StringConcatAspectBenchmark netcoreapp3.1 339μs 1.84μs 10.8μs 0 0 0 254.18 KB
master StringConcatAspectBenchmark net472 288μs 6.53μs 63.3μs 0 0 0 278.53 KB
#6592 StringConcatBenchmark net6.0 59.9μs 649ns 6.32μs 0 0 0 43.44 KB
#6592 StringConcatBenchmark netcoreapp3.1 55.1μs 278ns 1.64μs 0 0 0 42.64 KB
#6592 StringConcatBenchmark net472 37.5μs 165ns 594ns 0 0 0 58.94 KB
#6592 StringConcatAspectBenchmark net6.0 322μs 1.47μs 5.29μs 0 0 0 255.3 KB
#6592 StringConcatAspectBenchmark netcoreapp3.1 339μs 1.86μs 11.2μs 0 0 0 253.85 KB
#6592 StringConcatAspectBenchmark net472 306μs 7.01μs 69μs 0 0 0 278.53 KB

@andrewlock

andrewlock commented Jan 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Benchmarks Report for tracer 🐌

Benchmarks for #6592 compared to master:

  • 1 benchmarks are faster, with geometric mean 1.129
  • 2 benchmarks are slower, with geometric mean 1.131
  • All benchmarks have the same 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.ActivityBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartStopWithChild net6.0 8.23μs 45.9ns 315ns 0.0205 0.0123 0.0041 5.6 KB
master StartStopWithChild netcoreapp3.1 9.82μs 50.9ns 233ns 0.0146 0.00487 0 5.8 KB
master StartStopWithChild net472 16.1μs 58.7ns 219ns 1.04 0.306 0.0967 6.21 KB
#6592 StartStopWithChild net6.0 7.89μs 44.1ns 286ns 0.0149 0.00743 0 5.61 KB
#6592 StartStopWithChild netcoreapp3.1 10.1μs 55.8ns 340ns 0.0192 0.0096 0 5.8 KB
#6592 StartStopWithChild net472 16.1μs 58.5ns 227ns 1.05 0.318 0.103 6.22 KB
Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 521μs 668ns 2.5μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 650μs 343ns 1.33μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 856μs 490ns 1.9μs 0.428 0 0 3.3 KB
#6592 WriteAndFlushEnrichedTraces net6.0 497μs 292ns 1.13μs 0 0 0 2.7 KB
#6592 WriteAndFlushEnrichedTraces netcoreapp3.1 658μs 420ns 1.63μs 0 0 0 2.7 KB
#6592 WriteAndFlushEnrichedTraces net472 857μs 713ns 2.67μs 0.425 0 0 3.3 KB
Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendRequest net6.0 129μs 404ns 1.56μs 0.195 0 0 14.47 KB
master SendRequest netcoreapp3.1 140μs 564ns 2.18μs 0.207 0 0 17.27 KB
master SendRequest net472 0.0224ns 0.00436ns 0.0169ns 0 0 0 0 b
#6592 SendRequest net6.0 127μs 419ns 1.62μs 0.188 0 0 14.47 KB
#6592 SendRequest netcoreapp3.1 147μs 235ns 912ns 0.219 0 0 17.27 KB
#6592 SendRequest net472 0.00472ns 0.0023ns 0.00892ns 0 0 0 0 b
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 563μs 2.68μs 11.1μs 0.566 0 0 41.41 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 660μs 2.53μs 9.45μs 0.327 0 0 41.62 KB
master WriteAndFlushEnrichedTraces net472 838μs 3.2μs 12μs 8.22 2.47 0.411 53.29 KB
#6592 WriteAndFlushEnrichedTraces net6.0 569μs 2.85μs 12.1μs 0.561 0 0 41.55 KB
#6592 WriteAndFlushEnrichedTraces netcoreapp3.1 659μs 2.41μs 8.7μs 0.365 0 0 41.79 KB
#6592 WriteAndFlushEnrichedTraces net472 812μs 2.47μs 8.91μs 8.28 2.48 0.414 53.33 KB
Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteNonQuery net6.0 1.25μs 0.848ns 3.28ns 0.0144 0 0 1.02 KB
master ExecuteNonQuery netcoreapp3.1 1.71μs 1.23ns 4.59ns 0.0137 0 0 1.02 KB
master ExecuteNonQuery net472 1.92μs 2.24ns 8.68ns 0.157 0.000961 0 987 B
#6592 ExecuteNonQuery net6.0 1.29μs 1.14ns 4.43ns 0.0142 0 0 1.02 KB
#6592 ExecuteNonQuery netcoreapp3.1 1.82μs 1.1ns 3.8ns 0.0137 0 0 1.02 KB
#6592 ExecuteNonQuery net472 2.04μs 1.48ns 5.74ns 0.156 0.00101 0 987 B
Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master CallElasticsearch net6.0 1.29μs 0.681ns 2.55ns 0.0136 0 0 976 B
master CallElasticsearch netcoreapp3.1 1.5μs 0.374ns 1.35ns 0.0128 0 0 976 B
master CallElasticsearch net472 2.51μs 1.28ns 4.8ns 0.158 0 0 995 B
master CallElasticsearchAsync net6.0 1.25μs 0.539ns 2.02ns 0.0132 0 0 952 B
master CallElasticsearchAsync netcoreapp3.1 1.62μs 2.58ns 9.66ns 0.0137 0 0 1.02 KB
master CallElasticsearchAsync net472 2.62μs 1.68ns 6.28ns 0.167 0 0 1.05 KB
#6592 CallElasticsearch net6.0 1.2μs 0.511ns 1.91ns 0.0134 0 0 976 B
#6592 CallElasticsearch netcoreapp3.1 1.62μs 5.31ns 20.6ns 0.013 0 0 976 B
#6592 CallElasticsearch net472 2.6μs 1.49ns 5.78ns 0.158 0 0 995 B
#6592 CallElasticsearchAsync net6.0 1.38μs 0.866ns 3.36ns 0.0131 0 0 952 B
#6592 CallElasticsearchAsync netcoreapp3.1 1.62μs 0.696ns 2.61ns 0.0139 0 0 1.02 KB
#6592 CallElasticsearchAsync net472 2.57μs 1.29ns 4.99ns 0.166 0 0 1.05 KB
Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteAsync net6.0 1.31μs 0.619ns 2.4ns 0.0132 0 0 952 B
master ExecuteAsync netcoreapp3.1 1.63μs 0.778ns 3.01ns 0.0124 0 0 952 B
master ExecuteAsync net472 1.84μs 0.383ns 1.43ns 0.145 0 0 915 B
#6592 ExecuteAsync net6.0 1.3μs 0.48ns 1.86ns 0.0129 0 0 952 B
#6592 ExecuteAsync netcoreapp3.1 1.69μs 0.944ns 3.66ns 0.0127 0 0 952 B
#6592 ExecuteAsync net472 1.81μs 0.22ns 0.792ns 0.145 0 0 915 B
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendAsync net6.0 4.42μs 1.51ns 5.66ns 0.0309 0 0 2.31 KB
master SendAsync netcoreapp3.1 5.38μs 1.63ns 6.1ns 0.0376 0 0 2.85 KB
master SendAsync net472 7.38μs 1.79ns 6.95ns 0.496 0 0 3.12 KB
#6592 SendAsync net6.0 4.4μs 1.7ns 6.59ns 0.0312 0 0 2.31 KB
#6592 SendAsync netcoreapp3.1 5.35μs 22.9ns 115ns 0.0372 0 0 2.85 KB
#6592 SendAsync net472 7.41μs 1.42ns 5.48ns 0.496 0 0 3.12 KB
Benchmarks.Trace.ILoggerBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #6592

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.ILoggerBenchmark.EnrichedLog‑net6.0 1.128 1,515.21 1,709.50

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 1.52μs 0.699ns 2.61ns 0.0228 0 0 1.64 KB
master EnrichedLog netcoreapp3.1 2.26μs 1.61ns 6.24ns 0.0214 0 0 1.64 KB
master EnrichedLog net472 2.53μs 0.661ns 2.47ns 0.249 0 0 1.57 KB
#6592 EnrichedLog net6.0 1.71μs 0.853ns 3.19ns 0.023 0 0 1.64 KB
#6592 EnrichedLog netcoreapp3.1 2.18μs 1.33ns 4.99ns 0.0218 0 0 1.64 KB
#6592 EnrichedLog net472 2.6μs 1.12ns 4.18ns 0.25 0 0 1.57 KB
Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 116μs 146ns 545ns 0.0582 0 0 4.28 KB
master EnrichedLog netcoreapp3.1 120μs 166ns 644ns 0.0597 0 0 4.28 KB
master EnrichedLog net472 150μs 82.9ns 321ns 0.677 0.226 0 4.46 KB
#6592 EnrichedLog net6.0 116μs 169ns 654ns 0 0 0 4.28 KB
#6592 EnrichedLog netcoreapp3.1 119μs 70.7ns 255ns 0 0 0 4.28 KB
#6592 EnrichedLog net472 150μs 109ns 424ns 0.673 0.224 0 4.46 KB
Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 3.16μs 1.52ns 5.48ns 0.03 0 0 2.2 KB
master EnrichedLog netcoreapp3.1 4.34μs 1.67ns 6.47ns 0.0283 0 0 2.2 KB
master EnrichedLog net472 5.02μs 1.31ns 5.06ns 0.319 0 0 2.02 KB
#6592 EnrichedLog net6.0 2.95μs 0.786ns 2.83ns 0.031 0 0 2.2 KB
#6592 EnrichedLog netcoreapp3.1 4.26μs 1.1ns 4.11ns 0.0298 0 0 2.2 KB
#6592 EnrichedLog net472 4.91μs 1.27ns 4.91ns 0.319 0 0 2.02 KB
Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendReceive net6.0 1.46μs 0.5ns 1.94ns 0.0161 0 0 1.14 KB
master SendReceive netcoreapp3.1 1.89μs 0.609ns 2.28ns 0.0151 0 0 1.14 KB
master SendReceive net472 2.15μs 0.785ns 3.04ns 0.183 0 0 1.16 KB
#6592 SendReceive net6.0 1.41μs 1.15ns 4.44ns 0.0156 0 0 1.14 KB
#6592 SendReceive netcoreapp3.1 1.87μs 1.19ns 4.62ns 0.0157 0 0 1.14 KB
#6592 SendReceive net472 2.13μs 2.35ns 9.11ns 0.183 0 0 1.16 KB
Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 2.83μs 0.957ns 3.7ns 0.0213 0 0 1.6 KB
master EnrichedLog netcoreapp3.1 3.94μs 3.02ns 11.7ns 0.0217 0 0 1.65 KB
master EnrichedLog net472 4.46μs 2.86ns 11.1ns 0.322 0 0 2.04 KB
#6592 EnrichedLog net6.0 2.81μs 0.761ns 2.85ns 0.0224 0 0 1.6 KB
#6592 EnrichedLog netcoreapp3.1 3.95μs 1.35ns 5.05ns 0.0215 0 0 1.65 KB
#6592 EnrichedLog net472 4.45μs 2.82ns 10.9ns 0.322 0 0 2.04 KB
Benchmarks.Trace.SpanBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #6592

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑netcoreapp3.1 1.133 687.47 778.82

Faster 🎉 in #6592

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net472 1.129 647.27 573.39

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartFinishSpan net6.0 396ns 0.4ns 1.55ns 0.0081 0 0 576 B
master StartFinishSpan netcoreapp3.1 577ns 0.721ns 2.79ns 0.00786 0 0 576 B
master StartFinishSpan net472 645ns 1.54ns 5.97ns 0.0916 0 0 578 B
master StartFinishScope net6.0 539ns 0.536ns 2.07ns 0.00986 0 0 696 B
master StartFinishScope netcoreapp3.1 688ns 1.1ns 4.25ns 0.00943 0 0 696 B
master StartFinishScope net472 799ns 2.8ns 10.8ns 0.104 0 0 658 B
#6592 StartFinishSpan net6.0 399ns 0.722ns 2.8ns 0.00799 0 0 576 B
#6592 StartFinishSpan netcoreapp3.1 559ns 0.413ns 1.6ns 0.00777 0 0 576 B
#6592 StartFinishSpan net472 573ns 1.07ns 4.13ns 0.0918 0 0 578 B
#6592 StartFinishScope net6.0 598ns 0.67ns 2.59ns 0.00984 0 0 696 B
#6592 StartFinishScope netcoreapp3.1 779ns 1.24ns 4.79ns 0.0093 0 0 696 B
#6592 StartFinishScope net472 848ns 2.35ns 9.11ns 0.105 0 0 658 B
Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunOnMethodBegin net6.0 655ns 0.62ns 2.4ns 0.00988 0 0 696 B
master RunOnMethodBegin netcoreapp3.1 939ns 1.56ns 6.05ns 0.00936 0 0 696 B
master RunOnMethodBegin net472 1.05μs 3.37ns 12.6ns 0.104 0 0 658 B
#6592 RunOnMethodBegin net6.0 654ns 0.79ns 3.06ns 0.00961 0 0 696 B
#6592 RunOnMethodBegin netcoreapp3.1 848ns 1.42ns 5.48ns 0.00939 0 0 696 B
#6592 RunOnMethodBegin net472 1.02μs 1.81ns 6.99ns 0.104 0 0 658 B

@e-n-0 e-n-0 changed the title Fix NullReferenceException on Uninitialized HttpContext Fix errors exceptions on HttpContext get_Items Jan 27, 2025
@e-n-0
e-n-0 force-pushed the flavien/asm/httpcontext-items-nullreference branch from 1095c8b to 5602d72 Compare January 27, 2025 15:53
@e-n-0 e-n-0 changed the title Fix errors exceptions on HttpContext get_Items [ASM] Fix errors exceptions on HttpContext get_Items Jan 27, 2025
@e-n-0 e-n-0 self-assigned this Jan 27, 2025
@e-n-0
e-n-0 marked this pull request as ready for review January 27, 2025 16:01
@e-n-0
e-n-0 requested a review from a team as a code owner January 27, 2025 16:01
@e-n-0
e-n-0 force-pushed the flavien/asm/httpcontext-items-nullreference branch 2 times, most recently from 7bc95b1 to e0cfc66 Compare January 28, 2025 14:05
NachoEchevarria
NachoEchevarria approved these changes Jan 28, 2025
View reviewed changes

@NachoEchevarria NachoEchevarria left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

@e-n-0
e-n-0 force-pushed the flavien/asm/httpcontext-items-nullreference branch from e0cfc66 to 1f4749b Compare January 29, 2025 14:38
@NachoEchevarria NachoEchevarria mentioned this pull request Jan 29, 2025
Merged
@e-n-0
e-n-0 merged commit ca322c4 into master Feb 5, 2025
@e-n-0
e-n-0 deleted the flavien/asm/httpcontext-items-nullreference branch February 5, 2025 14:00
@github-actions github-actions Bot added this to the vNext-v3 milestone Feb 5, 2025
chojomok pushed a commit that referenced this pull request Jul 15, 2025
@e-n-0 @chojomok
[ASM] Fix errors exceptions on HttpContext get_Items (#6592)
38d12e6 
## Summary of changes

Handle the errors when trying to access the Items features of the
`HttpContext`.

All usage of Context.Items int the SecurityCoordinator has been changed
so the errors are caught.

Should fix these example stacktraces:

```
System.NullReferenceException
   at Microsoft.AspNetCore.Http.DefaultHttpContext.get_Items()
   at Datadog.Trace.AppSec.Coordinator.SecurityCoordinator.HttpTransport.get_ReportedExternalWafsRequestHeaders()
   at Datadog.Trace.AppSec.Coordinator.SecurityCoordinator.TryReport(IResult result, Boolean blocked, Nullable`1 status)
   at Datadog.Trace.AppSec.Coordinator.SecurityCoordinator.ReportAndBlock(IResult result)
   at Datadog.Trace.AppSec.Rasp.RaspModule.RunWafRasp(Dictionary`2 arguments, Span rootSpan, String address)
   at Datadog.Trace.AppSec.VulnerabilitiesModule.OnSSRF(String uriText)
   at Datadog.Trace.Iast.Aspects.System.Net.HttpClientAspect.Review(String parameter)
```

```
System.ObjectDisposedException
   at Microsoft.AspNetCore.Http.Features.FeatureReferences`1.ThrowContextDisposed()
   at Microsoft.AspNetCore.Http.DefaultHttpContext.get_Items()
   at Datadog.Trace.AppSec.Coordinator.SecurityCoordinator.HttpTransport.get_ReportedExternalWafsRequestHeaders()
   at Datadog.Trace.AppSec.Coordinator.SecurityReporter.TryReport(IResult result, Boolean blocked, Nullable`1 status)
   at Datadog.Trace.AppSec.Coordinator.SecurityCoordinator.ReportAndBlock(IResult result)
   at Datadog.Trace.AppSec.Rasp.RaspModule.RunWafRasp(Dictionary`2 arguments, Span rootSpan, String address)
   at Datadog.Trace.AppSec.VulnerabilitiesModule.OnSSRF(String uriText)
   at Datadog.Trace.Iast.Aspects.System.Net.HttpClientAspect.Review(String parameter)
```


## Reason for change

A `ObjectDisposedException` and a `NullReferenceException` can happen in
concurrent access to the `HttpContext`.
This situation of race condition can happen while the request is
finished while async jobs still run in the background and the waf for
RASP is run.

The HttpContext can be disposed from different manner by the user. You
can find more information in this
[PR](#6529).

## Implementation details

A new boolean have been added that reflect the state if the Items threw
an exception, thus making the code not access the HttpContext anymore in
the `TryReport` method.

## Test coverage

Added a unit test verifying that our code doesn't throw exceptions when
the `Items` of the `HttpContext` isn't available.

## Other details

Related PRs:
- #6529
- #6030
- #6017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@robertpi robertpi robertpi approved these changes
@NachoEchevarria NachoEchevarria NachoEchevarria approved these changes

Assignees

@e-n-0 e-n-0

Labels

area:asm identified-by:telemetry

Projects

None yet

Milestone

3.10.0

Development

Successfully merging this pull request may close these issues.

4 participants

@e-n-0 @andrewlock @robertpi @NachoEchevarria