Skip to content

Commit e218ce7

Browse files
committed
fix(exec): honour empty --detach-keys in remote exec
The compat exec-create handler decoded the request body into `ExecCreateConfig` which embeds `dockerContainer.ExecCreateRequest`. That struct has `DetachKeys string`, whose JSON zero value (`""`) is indistinguishable from the field being absent entirely, so an explicit `--detach-keys ""` (disable detach) was silently ignored on the remote client path. Fix: shadow the embedded field with `DetachKeys *string` in `ExecCreateConfig`. The JSON decoder now sets the pointer to nil when the field is absent and to a non-nil `*""` when it is explicitly empty. Update the handler and the tunnel client accordingly. Add a unit test verifying the JSON decoding semantics (absent, empty body, empty string, and a real value) and API tests in `test/apiv2/20-containers.at` covering exec create and inspect for the no-DetachKeys (default preserved), empty, and non-empty cases. Fixes: #28400 Signed-off-by: Devesh B <98201065+DeveshB-1@users.noreply.github.com>
1 parent 00012e3 commit e218ce7

5 files changed

Lines changed: 107 additions & 3 deletions

File tree

pkg/api/handlers/compat/exec.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,9 @@ func ExecCreateHandler(w http.ResponseWriter, r *http.Request) {
4646
libpodConfig.AttachStdin = input.AttachStdin
4747
libpodConfig.AttachStderr = input.AttachStderr
4848
libpodConfig.AttachStdout = input.AttachStdout
49-
if input.DetachKeys != "" {
50-
libpodConfig.DetachKeys = &input.DetachKeys
49+
// DetachKeys is a *string so nil means "not provided" and "" means "disable detach".
50+
if input.DetachKeys != nil {
51+
libpodConfig.DetachKeys = input.DetachKeys
5152
}
5253
libpodConfig.Environment = make(map[string]string)
5354
for _, envStr := range input.Env {
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
package handlers_test
2+
3+
import (
4+
"encoding/json"
5+
"testing"
6+
7+
"github.com/containers/podman/v6/pkg/api/handlers"
8+
)
9+
10+
// TestExecCreateConfigDetachKeys verifies that DetachKeys *string correctly
11+
// distinguishes "field absent" (nil) from "explicitly set to empty string" (*"").
12+
// This is the regression test for the bug where DetachKeys:"" was silently
13+
// ignored because the zero value of string is indistinguishable from absent.
14+
func TestExecCreateConfigDetachKeys(t *testing.T) {
15+
tests := []struct {
16+
name string
17+
body string
18+
wantNil bool
19+
wantVal string
20+
}{
21+
{
22+
name: "empty body - DetachKeys should be nil",
23+
body: `{}`,
24+
wantNil: true,
25+
},
26+
{
27+
name: "field absent - DetachKeys should be nil",
28+
body: `{"AttachStdout":true}`,
29+
wantNil: true,
30+
},
31+
{
32+
name: "field set to empty string - DetachKeys should be non-nil empty",
33+
body: `{"DetachKeys":""}`,
34+
wantNil: false,
35+
wantVal: "",
36+
},
37+
{
38+
name: "field set to value - DetachKeys should be non-nil with value",
39+
body: `{"DetachKeys":"ctrl-p,ctrl-q"}`,
40+
wantNil: false,
41+
wantVal: "ctrl-p,ctrl-q",
42+
},
43+
}
44+
45+
for _, tt := range tests {
46+
t.Run(tt.name, func(t *testing.T) {
47+
var cfg handlers.ExecCreateConfig
48+
if err := json.Unmarshal([]byte(tt.body), &cfg); err != nil {
49+
t.Fatalf("unmarshal: %v", err)
50+
}
51+
if tt.wantNil {
52+
if cfg.DetachKeys != nil {
53+
t.Errorf("expected DetachKeys to be nil, got %q", *cfg.DetachKeys)
54+
}
55+
} else {
56+
if cfg.DetachKeys == nil {
57+
t.Fatal("expected DetachKeys to be non-nil")
58+
}
59+
if *cfg.DetachKeys != tt.wantVal {
60+
t.Errorf("DetachKeys = %q, want %q", *cfg.DetachKeys, tt.wantVal)
61+
}
62+
}
63+
})
64+
}
65+
}

pkg/api/handlers/types.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,9 @@ type HistoryResponse struct {
243243

244244
type ExecCreateConfig struct {
245245
dockerContainer.ExecCreateRequest
246+
// DetachKeys shadows the embedded string field so the JSON decoder
247+
// can distinguish "field absent" (nil) from "explicitly set to empty" (*"").
248+
DetachKeys *string `json:"DetachKeys"`
246249
}
247250

248251
type ExecStartConfig struct {

pkg/domain/infra/tunnel/containers.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,7 @@ func makeExecConfig(options entities.ExecOptions) *handlers.ExecCreateConfig {
614614
createConfig.AttachStdin = options.Interactive
615615
createConfig.AttachStdout = true
616616
createConfig.AttachStderr = true
617-
createConfig.DetachKeys = options.DetachKeys
617+
createConfig.DetachKeys = &options.DetachKeys
618618
createConfig.Env = env
619619
createConfig.WorkingDir = options.WorkDir
620620
createConfig.Cmd = options.Cmd

test/apiv2/20-containers.at

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -913,4 +913,39 @@ t POST containers/create?platform=linux/amd64 \
913913
t POST containers/create?platform=linux/aarch64 \
914914
Image=$IMAGE \
915915
404
916+
917+
# 28405: exec create must honour DetachKeys:"" (explicitly disable detach).
918+
# Previously the JSON decoder could not distinguish an absent field from an
919+
# explicitly-empty one, so DetachKeys:"" was silently ignored.
920+
podman run -d --name exec-detach-keys-test $IMAGE top
921+
922+
# Without DetachKeys the system default must be preserved (non-empty).
923+
t POST containers/exec-detach-keys-test/exec \
924+
Cmd='["true"]' \
925+
201 \
926+
.Id~[0-9a-f]\\{64\\}
927+
eid=$(jq -r '.Id' <<<"$output")
928+
t GET exec/$eid/json 200 .DetachKeys~.+
929+
930+
# Explicitly empty DetachKeys disables detach - exec create must accept it.
931+
t POST containers/exec-detach-keys-test/exec \
932+
Cmd='["true"]' \
933+
DetachKeys='' \
934+
201 \
935+
.Id~[0-9a-f]\\{64\\}
936+
eid=$(jq -r '.Id' <<<"$output")
937+
t GET exec/$eid/json 200 .DetachKeys=''
938+
939+
# A non-empty DetachKeys must be stored and returned by inspect.
940+
t POST containers/exec-detach-keys-test/exec \
941+
Cmd='["true"]' \
942+
DetachKeys='ctrl-c' \
943+
201 \
944+
.Id~[0-9a-f]\\{64\\}
945+
eid=$(jq -r '.Id' <<<"$output")
946+
t GET exec/$eid/json 200 .DetachKeys='ctrl-c'
947+
948+
podman rm -f exec-detach-keys-test
949+
950+
916951
podman rmi -f $IMAGE

0 commit comments

Comments
 (0)