fix(appsec): treat empty request/response bodies as absent - #1321
Open
DenzoNL wants to merge 1 commit into
Open
Conversation
Event sources commonly represent the absence of a body as an empty string (GET/OPTIONS/HEAD requests, 204 or redirect responses). The AppSec processor attempted to parse these as JSON, failing with "EOF while parsing a value at line 1 column 0" and logging "aap: unable to parse body" at INFO on every such invocation. Skip body extraction entirely when the body is empty: there is nothing to analyze, and no reason to log a parse failure.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes noisy AppSec “failed to parse body: EOF …” logs by treating empty request/response bodies as absent (not malformed), aligning behavior with common event-source conventions (e.g., API Gateway using "" for no body).
Changes:
- Update
Body::reader()to returnNonefor empty-string bodies (in addition tonull), avoiding attempted JSON parsing of empty payloads. - Update
RawPayload::response_body()to returnNonefor zero-length raw responses. - Add unit tests covering absent/empty/base64 bodies and empty-body response formats.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| bottlecap/src/lifecycle/invocation/triggers/body.rs | Treat empty string bodies as absent in Body::reader() and add focused unit tests. |
| bottlecap/src/appsec/processor/response.rs | Skip returning a reader for empty raw response payloads; add tests for empty bodies in API Gateway + raw response parsing. |
Comment on lines
+20
to
+22
| /// Returns [`None`] if there is no body, including when it is an empty | ||
| /// string, which is how many event sources represent the absence of a | ||
| /// body (e.g, `GET` requests, `204` responses, etc...). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
After enabling AppSec on our Node.js Lambdas, the extension logs this on nearly every invocation (~45k lines/day on one service):
Not all requests and responses have a body (GET/OPTIONS/HEAD, 204s, redirects), and event sources often represent that as an empty string rather than
null.Body::reader()only returnsNonefornull, so an empty body still gets parsed as JSON and fails with the EOF error above.RawPayload::response_body()has the same problem — it always returnsSome, even for zero bytes.An empty body isn't malformed and there's nothing to analyze, so skip it instead of logging a parse failure. Rather than adding a log exclusion rule on our side I figured I'd fix it upstream, since this seems like a bug.
Testing
Body::reader()(absent, empty, base64) and for empty bodies inApiGatewayResponse/RawPayload.cargo test,cargo clippy --all-targets -- -D warnings, andcargo fmt --checkpass locally.