Validate that one of streams choosen in logs on api side

Fixes #6506

There is the bug, that very hard to fix: When we return job.Errorf in
"logs" job it writes to job.Stderr, to which connected ResponseWriter and on
this write w.WriteHeader(http.StatusOK) is called. So, we get 200 on error
from "logs" job.

Docker-DCO-1.1-Signed-off-by: Alexandr Morozov <lk4d4math@gmail.com> (github: LK4D4)

api/server/server.go

@@ -370,13 +370,23 @@ func getContainersLogs(eng *engine.Engine, version version.Version, w http.Respo
 	}
 
 	var (
-		job    = eng.Job("container_inspect", vars["name"])
-		c, err = job.Stdout.AddEnv()
+		inspectJob = eng.Job("container_inspect", vars["name"])
+		logsJob    = eng.Job("logs", vars["name"])
+		c, err     = inspectJob.Stdout.AddEnv()
 	)
 	if err != nil {
 		return err
 	}
-	if err = job.Run(); err != nil {
+	logsJob.Setenv("follow", r.Form.Get("follow"))
+	logsJob.Setenv("stdout", r.Form.Get("stdout"))
+	logsJob.Setenv("stderr", r.Form.Get("stderr"))
+	logsJob.Setenv("timestamps", r.Form.Get("timestamps"))
+	// Validate args here, because we can't return not StatusOK after job.Run() call
+	stdout, stderr := logsJob.GetenvBool("stdout"), logsJob.GetenvBool("stderr")
+	if !(stdout || stderr) {
+		return fmt.Errorf("Bad parameters: you must choose at least one stream")
+	}
+	if err = inspectJob.Run(); err != nil {
 		return err
 	}
 
@@ -390,14 +400,9 @@ func getContainersLogs(eng *engine.Engine, version version.Version, w http.Respo
 		errStream = outStream
 	}
 
-	job = eng.Job("logs", vars["name"])
-	job.Setenv("follow", r.Form.Get("follow"))
-	job.Setenv("stdout", r.Form.Get("stdout"))
-	job.Setenv("stderr", r.Form.Get("stderr"))
-	job.Setenv("timestamps", r.Form.Get("timestamps"))
-	job.Stdout.Add(outStream)
-	job.Stderr.Set(errStream)
-	if err := job.Run(); err != nil {
+	logsJob.Stdout.Add(outStream)
+	logsJob.Stderr.Set(errStream)
+	if err := logsJob.Run(); err != nil {
 		fmt.Fprintf(outStream, "Error running logs job: %s\n", err)
 	}
 	return nil

api/server/server_unit_test.go

@@ -4,12 +4,14 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
-	"github.com/dotcloud/docker/api"
-	"github.com/dotcloud/docker/engine"
 	"io"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
+
+	"github.com/dotcloud/docker/api"
+	"github.com/dotcloud/docker/engine"
 )
 
 func TestGetBoolParam(t *testing.T) {
@@ -151,6 +153,99 @@ func TestGetContainersByName(t *testing.T) {
 	}
 }
 
+func TestLogs(t *testing.T) {
+	eng := engine.New()
+	var inspect bool
+	var logs bool
+	eng.Register("container_inspect", func(job *engine.Job) engine.Status {
+		inspect = true
+		if len(job.Args) == 0 {
+			t.Fatal("Job arguments is empty")
+		}
+		if job.Args[0] != "test" {
+			t.Fatalf("Container name %s, must be test", job.Args[0])
+		}
+		return engine.StatusOK
+	})
+	expected := "logs"
+	eng.Register("logs", func(job *engine.Job) engine.Status {
+		logs = true
+		if len(job.Args) == 0 {
+			t.Fatal("Job arguments is empty")
+		}
+		if job.Args[0] != "test" {
+			t.Fatalf("Container name %s, must be test", job.Args[0])
+		}
+		follow := job.Getenv("follow")
+		if follow != "1" {
+			t.Fatalf("follow: %s, must be 1", follow)
+		}
+		stdout := job.Getenv("stdout")
+		if stdout != "1" {
+			t.Fatalf("stdout %s, must be 1", stdout)
+		}
+		stderr := job.Getenv("stderr")
+		if stderr != "" {
+			t.Fatalf("stderr %s, must be empty", stderr)
+		}
+		timestamps := job.Getenv("timestamps")
+		if timestamps != "1" {
+			t.Fatalf("timestamps %s, must be 1", timestamps)
+		}
+		job.Stdout.Write([]byte(expected))
+		return engine.StatusOK
+	})
+	r := serveRequest("GET", "/containers/test/logs?follow=1&stdout=1&timestamps=1", nil, eng, t)
+	if r.Code != http.StatusOK {
+		t.Fatalf("Got status %d, expected %d", r.Code, http.StatusOK)
+	}
+	if !inspect {
+		t.Fatal("container_inspect job was not called")
+	}
+	if !logs {
+		t.Fatal("logs job was not called")
+	}
+	res := r.Body.String()
+	if res != expected {
+		t.Fatalf("Output %s, expected %s", res, expected)
+	}
+}
+
+func TestLogsNoStreams(t *testing.T) {
+	eng := engine.New()
+	var inspect bool
+	var logs bool
+	eng.Register("container_inspect", func(job *engine.Job) engine.Status {
+		inspect = true
+		if len(job.Args) == 0 {
+			t.Fatal("Job arguments is empty")
+		}
+		if job.Args[0] != "test" {
+			t.Fatalf("Container name %s, must be test", job.Args[0])
+		}
+		return engine.StatusOK
+	})
+	eng.Register("logs", func(job *engine.Job) engine.Status {
+		logs = true
+		return engine.StatusOK
+	})
+	r := serveRequest("GET", "/containers/test/logs", nil, eng, t)
+	if r.Code != http.StatusBadRequest {
+		t.Fatalf("Got status %d, expected %d", r.Code, http.StatusBadRequest)
+	}
+	if inspect {
+		t.Fatal("container_inspect job was called, but it shouldn't")
+	}
+	if logs {
+		t.Fatal("logs job was called, but it shouldn't")
+	}
+	res := strings.TrimSpace(r.Body.String())
+	expected := "Bad parameters: you must choose at least one stream"
+	if !strings.Contains(res, expected) {
+		t.Fatalf("Output %s, expected %s in it", res, expected)
+	}
+}
+
 func serveRequest(method, target string, body io.Reader, eng *engine.Engine, t *testing.T) *httptest.ResponseRecorder {
 	r := httptest.NewRecorder()
 	req, err := http.NewRequest(method, target, body)