Skip to content

Fix HTTP envelope substitution and body classification - #310

Merged
pinodeca merged 3 commits into
mainfrom
fix/http-and-multipart
Jul 30, 2026
Merged

Fix HTTP envelope substitution and body classification#310
pinodeca merged 3 commits into
mainfrom
fix/http-and-multipart

Conversation

@pinodeca

Copy link
Copy Markdown
Contributor

Summary

Two follow-ups to the binary response work in #304, plus a written record of
what is still wrong with the HTTP activities.

Changes

Fail loudly on missing envelope fields

$name.field on an HTTP envelope previously left the pattern in place when the
field did not exist. In SQL that is deliberate — PostgreSQL reports it with its
own diagnostics — but a URL, a header or a multipart field has no such parser,
so a typo travelled over the wire verbatim: Bearer $auth.token was sent as a
literal and the request failed somewhere far less obvious. Raw contexts now fail
and list the available fields; the SQL and null-safe paths are unchanged.

Sniff unlabelled bodies

Body classification no longer trusts Content-Type alone. A declared textual
type is still decoded by reqwest so the charset parameter is honoured, but an
unrecognised or absent type is decided by its bytes: valid UTF-8 without NUL is
text, anything else is base64. This stops an untyped binary download being
mangled by a UTF-8 decode, and stops textual types no allowlist will ever cover
(application/jwt, application/x-ndjson) being base64-encoded for no reason.
NUL is excluded because PostgreSQL's text cannot hold it, so a mostly-zero
body would otherwise fail on the way into the result row rather than at the
decision that caused it.

Also splits Content-Type on a comma before classifying, so a proxy folding
duplicate headers into text/html, application/octet-stream is handled, and
names the unit in the error-body preview rather than reporting a base64 length
as bytes.

Fix HTTP test failure diagnostic

tests/e2e/sql/06_http_and_ssrf.sql reported the wrong value on failure.

Document outstanding problems

docs/http_problems.md records 18 open issues found while reviewing the HTTP
work — unbounded payloads, unvalidated JSONB inputs, a missing upgrade grant for
df.http_multipart(), panics on malformed node config, unvalidated
Content-Disposition filenames, tests that pass without asserting, and
documentation that misdescribes 5xx/retry behaviour, among others. Each entry
has evidence and a proposed fix. No behaviour change; the document is a work
queue, not a claim that any of it is fixed.

Testing

  • cargo fmt -p pg_durable -- --check
  • cargo clippy --features pg17
  • ./scripts/test-unit.sh
  • ./scripts/test-e2e-local.sh

GitHub Copilot added 3 commits July 30, 2026 14:56
Two follow-ups to the binary response work in #304.

`$name.field` on an HTTP envelope left the pattern in place when the field
was missing. In SQL that is deliberate -- PostgreSQL reports it with its own
diagnostics -- but a URL, a header or a multipart field has no such parser,
so a typo travelled over the wire verbatim: `Bearer $auth.token` would be
sent as a literal and the request would fail somewhere far less obvious.
Raw contexts now fail and list the available fields; the SQL and null-safe
paths are unchanged.

Body classification no longer trusts `Content-Type` alone. A declared
textual type is still decoded by reqwest so the `charset` parameter is
honoured, but an unrecognised or absent type is decided by its bytes: valid
UTF-8 without NUL is text, anything else is base64. This stops an untyped
binary download being mangled by a UTF-8 decode, and stops textual types no
allowlist will ever cover (`application/jwt`, `application/x-ndjson`) being
base64-encoded for no reason. NUL is excluded because PostgreSQL's `text`
cannot hold it, so a mostly-zero body would otherwise fail on the way into
the result row rather than at the decision that caused it.

Also split `Content-Type` on a comma before classifying, so a proxy folding
duplicate headers into `text/html, application/octet-stream` is handled, and
name the unit in the error-body preview rather than reporting a base64
length as bytes.
@pinodeca
pinodeca merged commit ad9f5d2 into main Jul 30, 2026
5 checks passed
@pinodeca
pinodeca deleted the fix/http-and-multipart branch July 30, 2026 16:55
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