From 1fb907198f67462aaef553b7b831c7ff72189564 Mon Sep 17 00:00:00 2001 From: Jhon Honce Date: Fri, 22 May 2020 11:43:37 -0700 Subject: V2 Fix interface nil checks Signed-off-by: Jhon Honce --- pkg/bindings/connection.go | 14 +++++--- pkg/bindings/containers/containers.go | 61 ++++++++++++++++++++++------------- pkg/bindings/containers/logs.go | 1 - 3 files changed, 48 insertions(+), 28 deletions(-) (limited to 'pkg') diff --git a/pkg/bindings/connection.go b/pkg/bindings/connection.go index d21d55beb..d30698c1d 100644 --- a/pkg/bindings/connection.go +++ b/pkg/bindings/connection.go @@ -16,7 +16,6 @@ import ( "time" "github.com/blang/semver" - "github.com/containers/libpod/pkg/api/types" jsoniter "github.com/json-iterator/go" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -28,7 +27,7 @@ var ( basePath = &url.URL{ Scheme: "http", Host: "d", - Path: "/v" + types.MinimalAPIVersion + "/libpod", + Path: "/v" + APIVersion.String() + "/libpod", } ) @@ -157,17 +156,22 @@ func pingNewConnection(ctx context.Context) error { } if response.StatusCode == http.StatusOK { - v, err := semver.ParseTolerant(response.Header.Get("Libpod-API-Version")) + versionHdr := response.Header.Get("Libpod-API-Version") + if versionHdr == "" { + logrus.Info("Service did not provide Libpod-API-Version Header") + return nil + } + versionSrv, err := semver.ParseTolerant(versionHdr) if err != nil { return err } - switch APIVersion.Compare(v) { + switch APIVersion.Compare(versionSrv) { case 1, 0: // Server's job when client version is equal or older return nil case -1: - return errors.Errorf("server API version is too old. client %q server %q", APIVersion.String(), v.String()) + return errors.Errorf("server API version is too old. client %q server %q", APIVersion.String(), versionSrv.String()) } } return errors.Errorf("ping response was %q", response.StatusCode) diff --git a/pkg/bindings/containers/containers.go b/pkg/bindings/containers/containers.go index 39a077f36..81e213d2b 100644 --- a/pkg/bindings/containers/containers.go +++ b/pkg/bindings/containers/containers.go @@ -9,6 +9,7 @@ import ( "net/url" "os" "os/signal" + "reflect" "strconv" "strings" @@ -347,6 +348,26 @@ func ContainerInit(ctx context.Context, nameOrID string) error { // Attach attaches to a running container func Attach(ctx context.Context, nameOrId string, detachKeys *string, logs, stream *bool, stdin io.Reader, stdout io.Writer, stderr io.Writer, attachReady chan bool) error { + isSet := struct { + stdin bool + stdout bool + stderr bool + }{ + stdin: !(stdin == nil || reflect.ValueOf(stdin).IsNil()), + stdout: !(stdout == nil || reflect.ValueOf(stdout).IsNil()), + stderr: !(stderr == nil || reflect.ValueOf(stderr).IsNil()), + } + // Ensure golang can determine that interfaces are "really" nil + if !isSet.stdin { + stdin = (io.Reader)(nil) + } + if !isSet.stdout { + stdout = (io.Writer)(nil) + } + if !isSet.stderr { + stderr = (io.Writer)(nil) + } + conn, err := bindings.GetClient(ctx) if err != nil { return err @@ -368,13 +389,13 @@ func Attach(ctx context.Context, nameOrId string, detachKeys *string, logs, stre if stream != nil { params.Add("stream", fmt.Sprintf("%t", *stream)) } - if stdin != nil { + if isSet.stdin { params.Add("stdin", "true") } - if stdout != nil { + if isSet.stdout { params.Add("stdout", "true") } - if stderr != nil { + if isSet.stderr { params.Add("stderr", "true") } @@ -422,32 +443,26 @@ func Attach(ctx context.Context, nameOrId string, detachKeys *string, logs, stre }() } - response, err := conn.DoRequest(nil, http.MethodPost, "/containers/%s/attach", params, nameOrId) + response, err := conn.DoRequest(stdin, http.MethodPost, "/containers/%s/attach", params, nameOrId) if err != nil { return err } - defer response.Body.Close() + if !(response.IsSuccess() || response.IsInformational()) { + return response.Process(nil) + } + // If we are attaching around a start, we need to "signal" // back that we are in fact attached so that started does // not execute before we can attach. if attachReady != nil { attachReady <- true } - if !(response.IsSuccess() || response.IsInformational()) { - return response.Process(nil) - } - - if stdin != nil { - go func() { - _, err := io.Copy(conn, stdin) - if err != nil { - logrus.Error("failed to write input to service: " + err.Error()) - } - }() - } buffer := make([]byte, 1024) if ctnr.Config.Tty { + if !isSet.stdout { + return fmt.Errorf("container %q requires stdout to be set", ctnr.ID) + } // If not multiplex'ed, read from server and write to stdout _, err := io.Copy(stdout, response.Body) if err != nil { @@ -469,25 +484,25 @@ func Attach(ctx context.Context, nameOrId string, detachKeys *string, logs, stre } switch { - case fd == 0 && stdin != nil: + case fd == 0 && isSet.stdout: _, err := stdout.Write(frame[0:l]) if err != nil { return err } - case fd == 1 && stdout != nil: + case fd == 1 && isSet.stdout: _, err := stdout.Write(frame[0:l]) if err != nil { return err } - case fd == 2 && stderr != nil: + case fd == 2 && isSet.stderr: _, err := stderr.Write(frame[0:l]) if err != nil { return err } case fd == 3: - return errors.New("error from service in stream: " + string(frame)) + return fmt.Errorf("error from service from stream: %s", frame) default: - return fmt.Errorf("unrecognized input header: %d", fd) + return fmt.Errorf("unrecognized channel in header: %d, 0-3 supported", fd) } } } @@ -520,6 +535,7 @@ func DemuxFrame(r io.Reader, buffer []byte, length int) (frame []byte, err error if len(buffer) < length { buffer = append(buffer, make([]byte, length-len(buffer)+1)...) } + n, err := io.ReadFull(r, buffer[0:length]) if err != nil { return nil, nil @@ -528,6 +544,7 @@ func DemuxFrame(r io.Reader, buffer []byte, length int) (frame []byte, err error err = io.ErrUnexpectedEOF return } + return buffer[0:length], nil } diff --git a/pkg/bindings/containers/logs.go b/pkg/bindings/containers/logs.go index 20c8b4292..7fea30003 100644 --- a/pkg/bindings/containers/logs.go +++ b/pkg/bindings/containers/logs.go @@ -50,7 +50,6 @@ func Logs(ctx context.Context, nameOrID string, opts LogOptions, stdoutChan, std if err != nil { return err } - defer response.Body.Close() buffer := make([]byte, 1024) for { -- cgit v1.2.3-54-g00ecf