Skip to content

Parse Lambda AppSec request bodies according to their content type - #12363

Open
claponcet wants to merge 10 commits into
masterfrom
clara.poncet/lambda-appsec-body-parsing
Open

Parse Lambda AppSec request bodies according to their content type#12363
claponcet wants to merge 10 commits into
masterfrom
clara.poncet/lambda-appsec-body-parsing

Conversation

@claponcet

@claponcet claponcet commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

What Does This Do

Dispatches Lambda AppSec request bodies on their declared Content-Type instead of handing every body to a JSON parser:

Declared type Reported to the WAF as
JSON-ish (application/json, any +json suffix, application/x-amz-json-1.1, application/javascript) or no usable type at all best-effort JSON parse
application/x-www-form-urlencoded multimap, the same shape already used for query parameters
multipart/* form fields, each part dispatched on its own Content-Type; file parts contribute their filename, not their content
text/* and everything else raw string

A body is never dropped. Any type we cannot structure, and any parse failure, degrades to the raw string, which the WAF can still match string rules against. In particular a text/plain body of 12345 must reach the WAF as a String and not as the Double a JSON parse would produce.

The same rule decides what happens to a malformed multipart body. A part whose headers run into the next delimiter is not reported part-by-part: no conforming parser accepts such a body — Commons FileUpload and Netty both reject it outright — so reporting the parts that happen to survive would show the WAF less than the application receives. The whole body degrades to the raw string instead.

Parsing is bounded by three allowances, shared across nesting levels rather than re-satisfied at each one: 1 MiB of characters read, 256 parts or parameters, and 20 levels of nesting. The depth and part limits mirror the WAF's own (WAFModule.MAX_DEPTH / MAX_ELEMENTS) — structure beyond them is discarded by ObjectIntrospection before the WAF sees it, so producing it would be wasted work. Exceeding an allowance degrades the body to a raw string rather than truncating it, since a truncated map would hide the dropped parameter from every rule.

Behavior changes worth a reviewer's eye. The JSON-ish decision is now made in one place, ContentTypeBodyParser.isJsonOrUntyped, which the response path in LambdaEventParser shares with the request path. It matches on the subtype alone, where the response path previously looked for json or javascript anywhere in the raw header. Three consequences:

  • A Content-Type holding nothing but parameters — ; charset=utf-8 — counts as declaring no type, and so gets the same best-effort JSON parse an absent header gets. Pinned by a case in dispatchesOnContentType.
  • A response body with a blank content-type now gets that best-effort JSON parse too. Previously only an absent header did, since "" does not contain json.
  • A response body whose type carries the word in a parametermultipart/form-data; boundary=--json — no longer reaches the JSON parser. This is why the gate matches on the subtype only: a client-chosen boundary must not decide how the body is read.

Motivation

Peer tracers already structure urlencoded and multipart Lambda bodies. Without it, a form-encoded or multipart request reaching a Lambda is visible to the WAF only as one opaque string, so any rule addressing server.request.body by key cannot match — the same request routed through a non-Lambda entrypoint would be inspected properly.

Additional Notes

MultipartSplitter no longer collects a HashMap of every header per part. Only Content-Disposition and Content-Type are ever read, so the others are matched and dropped as the scan passes them: a body that is nothing but header lines would otherwise allocate a map entry per line, which is why the change is here at all. The naive version of the fix — matching header names inline with a case-insensitive regionMatches — measured slower on well-formed many-part forms, so the canonical spelling is tried with a case-sensitive compare first and the matching lives in its own small method rather than inlined into split.

Two known flaky failures were seen in dd-trace-core while validating, both unrelated to this diff and both passing on re-run in isolation: PendingTraceBufferTest.testingTracerFlareDumpWithMultipleTraces and DDAgentWriterCombinedTest.statsdCommFailure. Neither is annotated @Flaky.

File parts populate server.request.body.filenames, which comes almost free once the parts are split. The other two file addresses are left empty on purpose: the Lambda event delivers the body as one UTF-8-decoded string, so a file's bytes are already corrupted by the time we see them, and files_content would report that damage as if it were the upload. files_field_names waits with it.

The repeated callback-registration boilerplate in LambdaAppSecHandler is left for its own change.

Contributor Checklist

Jira ticket: [PROJ-IDENT]

🤖 Generated with Claude Code

claponcet and others added 4 commits September 1, 2026 17:25
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@claponcet claponcet added type: feature Enhancements and improvements comp: asm waf Application Security Management (WAF) tag: serverless Serverless support tag: ai generated Largely based on code generated by an AI or LLM labels Sep 1, 2026
@claponcet

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ebad5227cd

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread dd-trace-core/src/main/java/datadog/trace/lambda/MultipartSplitter.java Outdated
@datadog-prod-us1-5

This comment has been minimized.

@dd-octo-sts

dd-octo-sts Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

🟢 Java Benchmark SLOs — All performance SLOs passed

Suite Status
Startup 🟢 pass

SLO thresholds are defined here based on automatically generated metrics. A warning is raised when results are within 5% of the threshold.

PR vs. master results
Scenario Candidate master Δ (95% CI of mean)
startup:insecure-bank:iast:Agent 13.99 s 13.90 s [-0.1%; +1.4%] (no difference)
startup:insecure-bank:tracing:Agent 12.94 s 13.01 s [-1.2%; +0.2%] (no difference)
startup:petclinic:appsec:Agent 17.00 s 16.83 s [-0.0%; +2.0%] (no difference)
startup:petclinic:iast:Agent 16.95 s 17.02 s [-1.1%; +0.3%] (no difference)
startup:petclinic:profiling:Agent 16.63 s 16.80 s [-2.2%; +0.2%] (no difference)
startup:petclinic:sca:Agent 16.95 s 16.75 s [+0.4%; +2.1%] (maybe worse)
startup:petclinic:tracing:Agent 16.18 s 16.18 s [-1.0%; +1.1%] (no difference)

Commit: 693dfe0b · CI Pipeline · Benchmarking Platform UI


Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion.

claponcet and others added 5 commits September 2, 2026 13:27
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d casts

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…the surviving parts

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@claponcet

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 58ba1ec8bc

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread dd-trace-core/src/main/java/datadog/trace/lambda/MultipartSplitter.java Outdated
Comment thread dd-trace-core/src/main/java/datadog/trace/lambda/MultipartSplitter.java Outdated
…nside part headers

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@claponcet
claponcet marked this pull request as ready for review September 2, 2026 16:02
@claponcet
claponcet requested a review from a team as a code owner September 2, 2026 16:02

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 693dfe0be2

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

}
final String delimiter = "--" + boundary;
final int length = body.length();
int position = body.startsWith(delimiter) ? 0 : nextDelimiter(body, delimiter, 0);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Validate the initial delimiter before treating it as closing

When a multipart preamble begins with a line such as --x--not-a-close, this direct startsWith selects offset 0 without calling endsLine; the close check then stops parsing and never scans the later valid --x delimiter. The body consequently falls back to an opaque string, so key-addressed WAF rules can miss fields that a conforming multipart parser extracts after ignoring the preamble. The fresh evidence beyond the prior fix is this initial-boundary path, which still bypasses the validation now performed by nextDelimiter; validate its terminator as well.

Useful? React with 👍 / 👎.

final String pair = tokenizer.nextToken();
final int equals = pair.indexOf('=');
final String name = decode(equals == -1 ? pair : pair.substring(0, equals));
if (!name.isEmpty()) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve form values with empty parameter names

For a valid urlencoded body such as =payload&ok=1, this condition silently drops the first value but still returns the nonempty map containing ok, so the caller does not fall back to the raw body. The Lambda handler still receives and can decode the original empty-name parameter, while AppSec sees no trace of payload; this also differs from the existing Netty body collector, which retains attributes under data.getName() without rejecting an empty key. Preserve the empty-name entry or treat the whole parse as unusable so an attack value cannot disappear from WAF inspection.

Useful? React with 👍 / 👎.

@datadog-prod-us1-5 datadog-prod-us1-5 Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Datadog Autotest: FAIL

The new body limit lets the WAF drop later form fields without a raw-body fallback. The multipart parser also treats a valid preamble near match as a close delimiter and misses later fields.

Open Bits AI session

🤖 Datadog Autotest · Commit 693dfe0 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest

// These bound the work done in this parser only: exceeding any of them degrades the body to a raw
// string rather than dropping content.
static final int MAX_BYTES = 1024 * 1024;
static final int MAX_PARTS = 256;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 WAF limit drops later form fields

An attacker can place a harmful value near the end and bypass key-based WAF rules.

Assertion details
  • Input: A URL-encoded body with 85 to 256 one-value parameters, or a multipart body with more than 127 scalar fields.
  • Expected: The parser should return the full raw body before the WAF can truncate any field.
  • Actual: The parser accepts the structured map. ObjectIntrospection then counts the root, keys, lists, and values against its separate 256-element limit. It drops later fields.

Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest · Open Bits AI session

}
final String delimiter = "--" + boundary;
final int length = body.length();
int position = body.startsWith(delimiter) ? 0 : nextDelimiter(body, delimiter, 0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Preamble near match stops multipart parsing

Key-based WAF rules can miss fields that the application parses.

Assertion details
  • Input: A valid multipart body starts with a preamble line such as --boundary--not-a-close, followed by a real delimiter and form fields.
  • Expected: The parser should ignore the preamble line and find the valid delimiter that follows it.
  • Actual: The initial prefix check accepts the near match. The close-delimiter check then stops parsing. AppSec receives only the raw body.
Suggested change
int position = body.startsWith(delimiter) ? 0 : nextDelimiter(body, delimiter, 0);
int position = nextDelimiter(body, delimiter, 0);

Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest · Open Bits AI session

// Unsupported trigger: AppSec skips the invocation entirely, so there is nothing to
// extract. The trigger type is carried by the caller, not by this result.
// Unsupported trigger: returning EMPTY makes the caller skip the invocation, so there is
// nothing to extract sinc ethe event is not supported.

@joeyzhao2018 joeyzhao2018 Sep 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
// nothing to extract sinc ethe event is not supported.
// nothing to extract since the event is not supported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp: asm waf Application Security Management (WAF) tag: ai generated Largely based on code generated by an AI or LLM tag: serverless Serverless support type: feature Enhancements and improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants