response_body_filter not called after overriding 204/304 status in response_filter

[Erio-Harrison]

Describe the bug

When a proxy overrides a 204 or 304 status code to 200 in response_filter, response_body_filter is never called, making it impossible to inject a synthesized body.

The root cause has two parts:

1. Protocol layer: The H1 client initializes body_reader to 0 for 204/304 responses, so the header task is emitted with end=true and no HttpTask::Body is ever produced.

2. Proxy layer: In the H1 path, no_body is computed before response_filter is called, so even if the user changes the status code, chunked encoding is not added. The H2 path already has the correct ordering.

Since no HttpTask::Body task is generated, response_body_filter has no opportunity to run.

Pingora info

Pingora version: 0.8.0 (commit 1d93711)
Rust version: rustc 1.94.1
Operating system version: macOS 26

Steps to reproduce

async fn response_filter(&self, _session: &mut Session, response: &mut ResponseHeader, ctx: &mut Self::CTX) -> Result<()> {
    if response.status.as_u16() == 204 {
        response.set_status(200)?;
        ctx.patch = true;
    }
    Ok(())
}

fn response_body_filter(&self, _session: &mut Session, body: &mut Option<Bytes>, end_of_stream: bool, ctx: &mut Self::CTX) -> Result<Option<Duration>> {
    if end_of_stream && ctx.patch {
        *body = Some(Bytes::from("<synthesized body/>"));
    }
    Ok(None)
}

Expected results

After changing the status from 204/304 to 200 in response_filter, response_body_filter should be called with end_of_stream=true, allowing the user to inject body content.

Observed results

response_body_filter is never called. The downstream client receives a 200 response with no body.

Additional context

The H2 path (proxy_h2.rs) already calls response_filter before computing no_body, but still has the same issue because the upstream eos=true flag prevents any HttpTask::Body from being generated.

[Erio-Harrison]

Current workaround

It is possible to work around this by writing the response directly in response_filter:

async fn response_filter(&self, session: &mut Session, response: &mut ResponseHeader, ctx: &mut Self::CTX) -> Result<()> {
    if response.status.as_u16() == 204 {
        let mut header = ResponseHeader::build(200, None)?;
        header.insert_header("Content-Type", "application/xml")?;
        session.write_response_header(Box::new(header), false).await?;
        session.write_response_body(Some(Bytes::from("<body/>")), true).await?;
    }
    Ok(())
}

This works because both the H1 and H2 paths silently no-op when the response header has already been sent:

H1 (v1/server.rs):

if let Some(resp) = self.response_written.as_ref() {
    if !resp.status.is_informational() || self.upgraded {
        warn!("Respond header is already sent, cannot send again");
        return Ok(());
    }
}

H2 (v2/server.rs):

if self.ended {
    // TODO: error or warn?
    return Ok(());
}
// ...
if self.response_written.as_ref().is_some() {
    warn!("Response header is already sent, cannot send again");
    return Ok(());
}

The H2 path has a // TODO: error or warn? comment on the ended guard, suggesting this absorption behavior is not intended as a stable contract.

Possible solutions:

1. Fix the body pipeline (my preferred approach): move response_filter before no_body computation in H1 (already correct in H2), and inject a synthetic Body(None, true) task when the status is overridden from 204/304 — so the existing response_filter + response_body_filter API works as expected.

2. Document the double-write absorption as a stable contract, making the current workaround officially supported.

3. Provide a new explicit API (e.g. session.override_response(header, body)) for full response replacement.

CC @eaufavor — as the author of both the H1 and H2 write guards, would appreciate your input on the preferred direction.

[drcaramelsyrup]

Note that for these responses you will likely have to modify the response header anyways to change the status and possibly insert Content-Length, etc. Is there a particular reason you would like to inject the body in a separate callback vs. in the response_filter workaround as you mentioned?

[Erio-Harrison]

Note that for these responses you will likely have to modify the response header anyways to change the status and possibly insert Content-Length, etc. Is there a particular reason you would like to inject the body in a separate callback vs. in the response_filter workaround as you mentioned?

Hi, the main concern is that the response_filter workaround only works because write_response_header silently no-ops on double writes. That behavior isn't a documented contract — the H2 path even has a // TODO: error or warn? on the equivalent guard. If that's ever tightened to an error, every proxy relying on this pattern breaks.

[drcaramelsyrup]

I see, this makes sense - you need to write the header before the write body call, and then the proxy tries to write the filtered response again. Ideally you shouldn’t need either manual call yourself. We can try to validate the proposed linked PR won’t cause other unexpected side effects internally, on the surface it is reasonable.

…

On Apr 17, 2026 at 4:58 PM, <Haoren(Harrison) Hu ***@***.***)> wrote: Erio-Harrison left a comment (cloudflare/pingora#852) (#852 (comment)) > > > Note that for these responses you will likely have to modify the response header anyways to change the status and possibly insert Content-Length, etc. Is there a particular reason you would like to inject the body in a separate callback vs. in the response_filter workaround as you mentioned? > > Hi, the main concern is that the response_filter workaround only works because write_response_header silently no-ops on double writes. That behavior isn't a documented contract — the H2 path even has a // TODO: error or warn? on the equivalent guard. If that's ever tightened to an error, every proxy relying on this pattern breaks. — Reply to this email directly, view it on GitHub (#852?email_source=notifications&email_token=AAGLWY47DUWAH3PENUKDXG34WLALRA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIMRXGIYDMMBXHA32M4TFMFZW63VHMNXW23LFNZ2KKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-4272060787), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AAGLWYZRWCLD7HAI2VNMR3T4WLALRAVCNFSM6AAAAACXSLWRZWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DENZSGA3DANZYG4). You are receiving this because you commented.Message ID: ***@***.***>

[Erio-Harrison]

We can try to validate the proposed linked PR won’t cause other unexpected side effects internally, on the surface it is reasonable.

Thanks! Look forward to it.