[PR #103] [MERGED] fix(attach,logs): correct hijack status code and multiplexed Content-Type #103

New issue

Closed

opened 2026-10-15 12:52:03 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-15 12:52:03 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/games-on-whales/LXC2Docker/pull/103
Author: @JBailes
Created: 7/1/2026
Status: ✅ Merged
Merged: 7/1/2026
Merged by: @JBailes

Base: main ← Head: fix/attach-hijack-response-parity

📝 Commits (1)

279c42e fix(attach,logs): correct hijack status code and multiplexed Content-Type

📊 Changes

2 files changed (+116 additions, -14 deletions)

View changed files

➕ internal/api/attach_response_test.go (+68 -0)
📝 internal/api/containers.go (+48 -14)

📄 Description

Two Docker response-parity fixes surfaced by the roundtable review of #101, in the attach/logs stream handlers.

1. Attach: 200 vs 101

POST /containers/{id}/attach wrote HTTP/1.1 101 UPGRADED unconditionally. Real Docker only sends 101 when the client requested a protocol upgrade (an Upgrade request header); every other client gets the attached stream over a plain 200 OK. Returning 101 to a non-upgrading client breaks it — the exact failure the exec-start handler already fixed in #98 (e.g. Wolf's fake-udev exec). This mirrors that logic onto attach.

2. Attach + logs: multiplexed Content-Type

Both handlers advertised Content-Type: application/vnd.docker.raw-stream unconditionally. Since Docker API v1.42 a non-TTY (stdcopy-multiplexed) stream is application/vnd.docker.multiplexed-stream; only a TTY stream is raw-stream. The stream body was already framed correctly per-TTY (#101) — this makes the advertised media type match, so a client can tell whether to strip the 8-byte frame headers.

Implementation

Extract writeHijackPreamble(w, upgrade, tty) and streamContentType(tty) helpers.
Attach uses both; the two logs Content-Type sites use streamContentType.
rec/tty moved above the hijack so the preamble can depend on it.

Tests

internal/api/attach_response_test.go — streamContentType for both TTY states, and writeHijackPreamble across all upgrade×tty combinations (status line, Content-Type, and presence/absence of Connection/Upgrade headers).

Full go test ./..., go vet, gofmt clean locally.

Follow-up (not in this PR): exec-start still advertises raw-stream for non-TTY exec — same helper can be applied there.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/games-on-whales/LXC2Docker/pull/103 **Author:** [@JBailes](https://github.com/JBailes) **Created:** 7/1/2026 **Status:** ✅ Merged **Merged:** 7/1/2026 **Merged by:** [@JBailes](https://github.com/JBailes) **Base:** `main` ← **Head:** `fix/attach-hijack-response-parity` --- ### 📝 Commits (1) - [`279c42e`](https://github.com/games-on-whales/LXC2Docker/commit/279c42e34266cd0268c6cfb76689b6fca763e372) fix(attach,logs): correct hijack status code and multiplexed Content-Type ### 📊 Changes **2 files changed** (+116 additions, -14 deletions) <details> <summary>View changed files</summary> ➕ `internal/api/attach_response_test.go` (+68 -0) 📝 `internal/api/containers.go` (+48 -14) </details> ### 📄 Description Two Docker response-parity fixes surfaced by the roundtable review of #101, in the attach/logs stream handlers. ## 1. Attach: 200 vs 101 `POST /containers/{id}/attach` wrote `HTTP/1.1 101 UPGRADED` **unconditionally**. Real Docker only sends `101` when the client requested a protocol upgrade (an `Upgrade` request header); every other client gets the attached stream over a plain `200 OK`. Returning `101` to a non-upgrading client breaks it — the exact failure the exec-start handler already fixed in #98 (e.g. Wolf's fake-udev exec). This mirrors that logic onto attach. ## 2. Attach + logs: multiplexed Content-Type Both handlers advertised `Content-Type: application/vnd.docker.raw-stream` unconditionally. Since **Docker API v1.42** a non-TTY (stdcopy-multiplexed) stream is `application/vnd.docker.multiplexed-stream`; only a TTY stream is `raw-stream`. The stream body was already framed correctly per-TTY (#101) — this makes the advertised media type match, so a client can tell whether to strip the 8-byte frame headers. ## Implementation - Extract `writeHijackPreamble(w, upgrade, tty)` and `streamContentType(tty)` helpers. - Attach uses both; the two `logs` Content-Type sites use `streamContentType`. - `rec`/`tty` moved above the hijack so the preamble can depend on it. ## Tests `internal/api/attach_response_test.go` — `streamContentType` for both TTY states, and `writeHijackPreamble` across all upgrade×tty combinations (status line, Content-Type, and presence/absence of Connection/Upgrade headers). Full `go test ./...`, `go vet`, `gofmt` clean locally. Follow-up (not in this PR): exec-start still advertises `raw-stream` for non-TTY exec — same helper can be applied there. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-15 12:52:03 +00:00

bsakaguchi referenced this issue

2026-10-15 12:52:03 +00:00

[PR #104] [MERGED] fix(stream): version-gate multiplexed Content-Type and apply to exec-start #104

bsakaguchi closed this issue

2026-10-15 12:52:03 +00:00