-
Notifications
You must be signed in to change notification settings - Fork 306
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ASM] multer instrumentation #4781
Conversation
Overall package sizeSelf size: 7.61 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | @datadog/native-appsec | 8.2.1 | 19.18 MB | 19.19 MB | | @datadog/native-iast-taint-tracking | 3.2.0 | 13.9 MB | 13.91 MB | | @datadog/pprof | 5.4.1 | 9.76 MB | 10.13 MB | | protobufjs | 7.2.5 | 2.77 MB | 5.16 MB | | @datadog/native-iast-rewriter | 2.5.0 | 2.51 MB | 2.65 MB | | @opentelemetry/core | 1.14.0 | 872.87 kB | 1.47 MB | | @datadog/native-metrics | 2.0.0 | 898.77 kB | 1.3 MB | | @opentelemetry/api | 1.8.0 | 1.21 MB | 1.21 MB | | import-in-the-middle | 1.11.2 | 112.74 kB | 826.22 kB | | msgpack-lite | 0.1.26 | 201.16 kB | 281.59 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | lru-cache | 7.18.3 | 133.92 kB | 133.92 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | @datadog/sketches-js | 2.1.0 | 109.9 kB | 109.9 kB | | semver | 7.6.3 | 95.82 kB | 95.82 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 5.3.1 | 51.46 kB | 51.46 kB | | int64-buffer | 0.1.10 | 49.18 kB | 49.18 kB | | shell-quote | 1.8.1 | 44.96 kB | 44.96 kB | | istanbul-lib-coverage | 3.2.0 | 29.34 kB | 29.34 kB | | rfdc | 1.3.1 | 25.21 kB | 25.21 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | dc-polyfill | 0.1.4 | 23.1 kB | 23.1 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | koalas | 1.0.2 | 6.47 kB | 6.47 kB | | path-to-regexp | 0.1.10 | 6.38 kB | 6.38 kB | | module-details-from-path | 1.0.3 | 4.47 kB | 4.47 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
packages/dd-trace/test/appsec/iast/taint-tracking/plugin.express.plugin.spec.js
Outdated
Show resolved
Hide resolved
Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4781 +/- ##
===========================================
+ Coverage 68.58% 98.09% +29.51%
===========================================
Files 12 1 -11
Lines 818 105 -713
===========================================
- Hits 561 103 -458
+ Misses 257 2 -255 ☔ View full report in Codecov by Sentry. |
const nextResource = new AsyncResource('bound-anonymous-fn') | ||
arguments[2] = nextResource.bind(publishRequestBodyAndNext(req, res, next)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are we checking this multerReadCh.hasSubscribers
in line 14 and not here? imo, we should prevent creating AsyncResource when appsec/iast is not enabled.
const nextResource = new AsyncResource('bound-anonymous-fn') | |
arguments[2] = nextResource.bind(publishRequestBodyAndNext(req, res, next)) | |
if (multerReadCh.hasSubscribers) { | |
const nextResource = new AsyncResource('bound-anonymous-fn') | |
arguments[2] = nextResource.bind(publishRequestBodyAndNext(req, res, next)) | |
} | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i was inspired by body-parser instrumentation
* multer instrumentation * clean up test * unsubscribe multer * multer CI tests * multer CI plugins tests * Change multer version range * Specify the version * second try * third * multer request blocking integration test * include iast tainting multer body test * fix test * Update integration-tests/multer.spec.js Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com> * Move taint multipart body test to the integration test * delete test * Move multer tests inside appsec * Include test not using multer middleware --------- Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com>
* multer instrumentation * clean up test * unsubscribe multer * multer CI tests * multer CI plugins tests * Change multer version range * Specify the version * second try * third * multer request blocking integration test * include iast tainting multer body test * fix test * Update integration-tests/multer.spec.js Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com> * Move taint multipart body test to the integration test * delete test * Move multer tests inside appsec * Include test not using multer middleware --------- Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com>
* multer instrumentation * clean up test * unsubscribe multer * multer CI tests * multer CI plugins tests * Change multer version range * Specify the version * second try * third * multer request blocking integration test * include iast tainting multer body test * fix test * Update integration-tests/multer.spec.js Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com> * Move taint multipart body test to the integration test * delete test * Move multer tests inside appsec * Include test not using multer middleware --------- Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com>
What does this PR do?
multer instrumentation
Motivation
be able to handle multipart request bodies parsed with multer
ST: DataDog/system-tests#3249
Plugin Checklist
Additional Notes