Skip to content

fix: isolate logger response body buffers#13450

Open
nic-6443 wants to merge 1 commit into
apache:masterfrom
nic-6443:fix-logger-body-buffer-isolation
Open

fix: isolate logger response body buffers#13450
nic-6443 wants to merge 1 commit into
apache:masterfrom
nic-6443:fix-logger-body-buffer-isolation

Conversation

@nic-6443
Copy link
Copy Markdown
Member

@nic-6443 nic-6443 commented May 28, 2026

This fixes response body collection when loggers are configured in multiple scopes for the same request. Each logger configuration now keeps an isolated body buffer, so a route-level logger cannot overwrite the response body collected for a global logger.

The change keeps existing plugin configuration unchanged and preserves the current response body size limit behavior.

Tests:

  • prove -I. -Itest-nginx/lib -r t/core/response.t t/plugin/http-logger.t

Copilot AI review requested due to automatic review settings May 28, 2026 11:37
@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels May 28, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a bug where multiple logger plugins (e.g., a global http-logger and a route-level file-logger) collecting the response body would share ctx._resp_body_bytes and ctx._body_buffer[ctx._plugin_name], allowing one logger's accumulation/truncation state to break another logger's collection. The fix isolates the body buffer per caller by accepting an optional body_buffer_key (the logger's conf table) and moves byte counting onto the buffer itself, while also introducing a done flag so truncated buffers stop accumulating without dropping subsequent loggers' data.

Changes:

  • core.response.hold_body_chunk now accepts a body_buffer_key and stores per-key bytes/done instead of a request-wide ctx._resp_body_bytes.
  • log-util.collect_body passes the plugin conf as the buffer key and removes the now-redundant pre-call byte check.
  • New t/plugin/http-logger.t tests 28–29 cover global http-logger + global file-logger + route-level file-logger all collecting bodies simultaneously.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
apisix/core/response.lua Adds body_buffer_key param, per-buffer byte/done tracking.
apisix/utils/log-util.lua Uses plugin conf as the buffer key and drops the shared-counter guard.
t/plugin/http-logger.t Adds regression test for global+route loggers collecting response body.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

Labels

bug Something isn't working size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants