Skip to content

Improvements for appsec libxml2 usage#3564

Merged
cataphract merged 2 commits intomasterfrom
glopes/libxml2-updates
Jan 13, 2026
Merged

Improvements for appsec libxml2 usage#3564
cataphract merged 2 commits intomasterfrom
glopes/libxml2-updates

Conversation

@cataphract
Copy link
Copy Markdown
Contributor

@cataphract cataphract commented Jan 12, 2026

Description

Follow up on #3558 (comment)

  • Generalize the vendoring script a bit
  • Make the vendoring script include fewer files
  • Add two new tests
  • Move to SAX2 and disable SAX1 support
  • Add CI job warning when we're not on the latest libxml2

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@cataphract cataphract requested review from a team as code owners January 12, 2026 00:28
@datadog-datadog-prod-us1
Copy link
Copy Markdown

datadog-datadog-prod-us1 Bot commented Jan 12, 2026 •
edited
Loading

⚠️ Tests

✨ Fix all issues with Cursor

⚠️ Warnings

❄️ 1 New flaky test detected

ext/sodium/tests/php_password_verify.phpt (Test interoperability of password_verify()) from php.ext.sodium.tests (Datadog) (Fix with Cursor) 
--
     bool(false)
     Using password: string(44) "%s"
     Hash: string(98) "$argon2id$v=19$m=262144,t=2,p=1$%s$%s"
008- bool(true)
009- bool(false)
010- Using password: string(44) "%s"
011- Hash: string(97) "$argon2id$v=19$m=65536,t=3,p=1$%s$%s"
012- bool(true)
013- bool(false)
...

🧪 2063 Tests failed

    testSearchPhpBinaries from integration.DDTrace\Tests\Integration\PHPInstallerTest (Fix with Cursor)

    testSimplePushAndProcess from laravel-58-test.DDTrace\Tests\Integrations\Laravel\V5_8\QueueTest (Fix with Cursor)

testSimplePushAndProcess from laravel-8x-test.DDTrace\Tests\Integrations\Laravel\V8_x\QueueTest (Datadog) (Fix with Cursor) 
DDTrace\Tests\Integrations\Laravel\V8_x\QueueTest::testSimplePushAndProcess
Test code or tested code printed unexpected output: spanLinksTraceId: 696622d400000000736fcff7da9d769d
tid: 696622d400000000
hexProcessTraceId: 736fcff7da9d769d
hexProcessSpanId: 6ee7294f0119441a
processTraceId: 8318095700208219805
processSpanId: 7991401483089822746
View all
This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 5666782 | Docs | Datadog PR Page | Was this helpful? Give us feedback!

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jan 12, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 96.49123% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.01%. Comparing base (4a26042) to head (5666782).

Files with missing lines Patch % Lines
appsec/src/extension/xml_truncated_parser.c 96.49% 0 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3564      +/-   ##
==========================================
+ Coverage   61.81%   62.01%   +0.20%     
==========================================
  Files         140      140              
  Lines       13281    13309      +28     
  Branches     1758     1762       +4     
==========================================
+ Hits         8209     8253      +44     
+ Misses       4282     4268      -14     
+ Partials      790      788       -2
Files with missing lines Coverage Δ
appsec/src/extension/xml_truncated_parser.c 73.31% <96.49%> (+3.20%) ⬆️

... and 2 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 4a26042...5666782. 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.

@cataphract
Improvements for appsec libxml2 usage
288e04a 
- Generalize the vendoring script a bit
- Make the vendoring script include fewer files
- Add two new tests
- Move to SAX2 and disable SAX1 support
- Add CI job warning when we're not on the latest libxml2
@cataphract cataphract force-pushed the glopes/libxml2-updates branch from 250a3eb to 288e04a Compare January 12, 2026 00:43
@realFlowControl
Copy link
Copy Markdown
Member

realFlowControl commented Jan 12, 2026

@cataphract some benchmarks started failing with "Aborted (core dumped)"

@cataphract
Copy link
Copy Markdown
Contributor Author

cataphract commented Jan 12, 2026

@cataphract some benchmarks started failing with "Aborted (core dumped)"

The error is unrelated (see e.g. https://gitlab.ddbuild.io/DataDog/apm-reliability/dd-trace-php/-/jobs/1332840054 also failing).

I'll check if b310ddd fixes it

@pr-commenter
Copy link
Copy Markdown

pr-commenter Bot commented Jan 13, 2026

Benchmarks [ appsec ]

Benchmark execution time: 2026-01-13 11:11:44

Comparing candidate commit 5666782 in PR branch glopes/libxml2-updates with baseline commit 4a26042 in branch master.

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

@cataphract cataphract merged commit 2c70c61 into master Jan 13, 2026
2000 of 2009 checks passed
@cataphract cataphract deleted the glopes/libxml2-updates branch January 13, 2026 13:42
@github-actions github-actions Bot added this to the 1.16.0 milestone Jan 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@realFlowControl realFlowControl realFlowControl approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.16.0

Development

Successfully merging this pull request may close these issues.

3 participants

@cataphract @codecov-commenter @realFlowControl