Skip to content

Fix wrong transfer-encoding headers being sent #274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Julow
Copy link
Contributor

@Julow Julow commented Jul 18, 2025

Ocsigen_response carries a Cohttp.Response.t and a body and both of them carry a transfer-encoding.
The problem is that the Cohttp.Response.t is often constructed with a default value for the transfer encoding that takes precedence over the encoding of the body.

This changes Ocsigen_response.make to remove the header if it is equal to the default value. The correct way to specify the encoding is while constructing the Body.t value. Setting the header with Ocsigen_response.add_header is also supported.

This bug could materialize with pages that never finish loading or with trimmed content.
Especially in Eliom, where Cohttp.Response.make is often called without the ~encoding argument.

This bug was introduced in #260 while introducing the Body.t type.

`Ocsigen_response` carries a `Cohttp.Response.t` and a `body` and both
of them carry a `transfer-encoding`.
The problem is that the `Cohttp.Response.t` is often constructed with a
default value for the transfer encoding that takes precedence over the
encoding of the body.

This changes `Ocsigen_response.make` to remove the header if it is equal
to the default value. The correct way to specify the encoding is while
constructing the `Body.t` value. Setting the header with
`Ocsigen_response.add_header` is also supported.

This bug could materialize with pages that never finish loading or with
trimmed content.
Especially in Eliom, where `Cohttp.Response.make` is often called
without the `~encoding` argument.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant