mORMot Open Source

jienyuan

Member

Registered: 2017-07-22

Posts: 8

THttpRequestContext.Reset` does not clear `AcceptEncoding

THttpRequestContext.Reset` does not clear `AcceptEncoding` — compression state leaks between requests on a recycled connection.

I think I have found a small but nasty state-leak in `THttpRequestContext`.

Version: mORMot 2, commit `b082ba107` (2026-09-15), Delphi 10.3 Rio, Win64,
`TRestHttpServer` created with `useBidirAsync`, options `HTTPSERVER_DEFAULT_OPTIONS`
(i.e. `rsoCompressGZip` + `rsoCompressSynLZ`).

Symptom

A client that sends no`Accept-Encoding` header at all can receive a response
with `Content-Encoding: gzip`. It obviously cannot decode it, and since it never
asked for compression it does not even try — it just sees garbage where JSON was
expected.

This is intermittent and load dependent: it only happens shortly after *another*
client has sent `Accept-Encoding: gzip` to the same server.

Root cause

`THttpRequestContext.AcceptEncoding` (declared at `mormot.net.http.pas:440`) is
only ever assigned when the incoming request actually carries the header
(`ParseHeader`, `hhAcceptEncoding` branch, line 3813):

    hhAcceptEncoding:
      begin
         // 'ACCEPT-ENCODING:'
        GetTrimmed(P + 17, P2, PLen, AcceptEncoding);

`THttpRequestContext.Reset` (line 3621) clears the sibling header fields —
`Upgrade`, `BearerToken`, `ResponseHeaders`, `UserAgent`, `Referer`,
`ContentType`, `Headers` — and it does reset the derived bitset at line 3652:

  integer(CompressAcceptHeader) := 0;

but it never clears `AcceptEncoding` itself.

So on a recycled context, `ParseHeaderFinalize` (line 3934) re-derives the bitset
from the **previous** request's value, because the guard only tests for non-empty:

  if (CompressList <> nil) and
     (AcceptEncoding <> '') then           // <-- stale value passes this test
    CompressList^.DecodeAcceptEncoding(pointer(AcceptEncoding), CompressAcceptHeader);

`CompressAcceptHeader` is therefore repopulated for a request that never asked for
any content coding, and `CompressContentAndFinalizeHead` duly compresses the body.

With the async server this is easy to hit, because `fHttp: THttpRequestContext` is
a value field of the connection object (`mormot.net.async.pas:912`) and those
objects are pooled and reused across *different* client connections — so the leak
is not limited to keep-alive requests from the same client.

Reproduction

Start a `TRestHttpServer` with the default options (gzip + SynLZ registered), then
alternate two requests to the same endpoint, from separate connections:

1. one **with** `Accept-Encoding: gzip`
2. one with **no** `Accept-Encoding` header at all

Request (2) comes back with `Content-Encoding: gzip` roughly every second or third
iteration. On my box, using a ~24 KB JSON response:

```
baseline (clean)     24514 bytes  Content-Encoding: none
after gzip load #1   24514 bytes  none
after gzip load #2   18766 bytes  gzip     <-- never requested
after gzip load #3   24514 bytes  none
after gzip load #4   18766 bytes  gzip     <-- never requested
after gzip load #5   24514 bytes  none
```

Note that `curl` does **not** send `Accept-Encoding` unless you pass `--compressed`,
which makes curl a convenient client for step (2).

Suggested fix

Clear the field in `Reset`, alongside the other header fields it already clears
(after the `Referer` block at line 3642):

  if Referer <> '' then
    FastAssignNew(Referer);
  if AcceptEncoding <> '' then
    FastAssignNew(AcceptEncoding);
  RangeOffset := 0;

With this applied the same test gives 0 leaks out of 10 iterations, and explicitly
requested compression still works normally (`Accept-Encoding: gzip` -> 18766 bytes
`Content-Encoding: gzip`; `Accept-Encoding: synlz` -> 27078 bytes
`Content-Encoding: synlz`).

An alternative would be to make `ParseHeaderFinalize` unconditional, or to have
`ParseHeader` always assign `AcceptEncoding` (to `''` when the header is absent),
but clearing it in `Reset` keeps it consistent with how `UserAgent`, `Referer`,
`Upgrade` and `BearerToken` are already handled.

`Host` and possibly other optional fields look like they have the same shape of
problem; `Host` happens to be harmless in practice because virtually every client
sends it on every request, but you may want to audit that list.

Disclosure: I used an AI assistant to help investigate this and to draft the
  report. The diagnosis came from instrumenting our own server and client and
  reading the mORMot sources; every number quoted above is from an actual run on
  our deployment, not from the model.

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 15,599

Website

Re: THttpRequestContext.Reset` does not clear `AcceptEncoding

Good catch!

Please try with
https://github.com/synopse/mORMot2/commit/4c990d655