From f38f3109c20be879425f4077be0903eb45f90c6c Mon Sep 17 00:00:00 2001 From: David Calavera Date: Wed, 14 Oct 2015 10:31:09 -0700 Subject: [PATCH] Remove defensive check of mux vars handling. We should not check if the mux framework internals work as expected in every handler. The missing parameter error doesn't make sense from the user point of view. This change initializes a proper vars context if the mux fails to do so and delegates specific parameter error checks to the handlers. Signed-off-by: David Calavera Upstream-commit: 389ce0aae6a303660e591ef80272322ac82854e2 Component: engine --- .../engine/api/server/httputils/form.go | 3 - .../api/server/router/local/container.go | 56 ------------------- .../engine/api/server/router/local/copy.go | 4 -- .../engine/api/server/router/local/exec.go | 8 --- .../engine/api/server/router/local/image.go | 22 -------- .../engine/api/server/router/local/inspect.go | 4 -- components/engine/api/server/server.go | 7 ++- 7 files changed, 6 insertions(+), 98 deletions(-) diff --git a/components/engine/api/server/httputils/form.go b/components/engine/api/server/httputils/form.go index 5180b99f16..20188c12d8 100644 --- a/components/engine/api/server/httputils/form.go +++ b/components/engine/api/server/httputils/form.go @@ -55,9 +55,6 @@ type ArchiveOptions struct { // ArchiveFormValues parses form values and turns them into ArchiveOptions. // It fails if the archive name and path are not in the request. func ArchiveFormValues(r *http.Request, vars map[string]string) (ArchiveOptions, error) { - if vars == nil { - return ArchiveOptions{}, fmt.Errorf("Missing parameter") - } if err := ParseForm(r); err != nil { return ArchiveOptions{}, err } diff --git a/components/engine/api/server/router/local/container.go b/components/engine/api/server/router/local/container.go index d1e352dd32..9cf14367db 100644 --- a/components/engine/api/server/router/local/container.go +++ b/components/engine/api/server/router/local/container.go @@ -56,9 +56,6 @@ func (s *router) getContainersStats(ctx context.Context, w http.ResponseWriter, if err := httputils.ParseForm(r); err != nil { return err } - if vars == nil { - return fmt.Errorf("Missing parameter") - } stream := httputils.BoolValueOrDefault(r, "stream", true) var out io.Writer @@ -88,9 +85,6 @@ func (s *router) getContainersLogs(ctx context.Context, w http.ResponseWriter, r if err := httputils.ParseForm(r); err != nil { return err } - if vars == nil { - return fmt.Errorf("Missing parameter") - } // Args are validated before the stream starts because when it starts we're // sending HTTP 200 by writing an empty chunk of data to tell the client that @@ -150,18 +144,10 @@ func (s *router) getContainersLogs(ctx context.Context, w http.ResponseWriter, r } func (s *router) getContainersExport(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - if vars == nil { - return fmt.Errorf("Missing parameter") - } - return s.daemon.ContainerExport(vars["name"], w) } func (s *router) postContainersStart(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - if vars == nil { - return fmt.Errorf("Missing parameter") - } - // If contentLength is -1, we can assumed chunked encoding // or more technically that the length is unknown // https://golang.org/src/pkg/net/http/request.go#L139 @@ -193,9 +179,6 @@ func (s *router) postContainersStop(ctx context.Context, w http.ResponseWriter, if err := httputils.ParseForm(r); err != nil { return err } - if vars == nil { - return fmt.Errorf("Missing parameter") - } seconds, _ := strconv.Atoi(r.Form.Get("t")) @@ -208,9 +191,6 @@ func (s *router) postContainersStop(ctx context.Context, w http.ResponseWriter, } func (s *router) postContainersKill(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - if vars == nil { - return fmt.Errorf("Missing parameter") - } if err := httputils.ParseForm(r); err != nil { return err } @@ -247,9 +227,6 @@ func (s *router) postContainersRestart(ctx context.Context, w http.ResponseWrite if err := httputils.ParseForm(r); err != nil { return err } - if vars == nil { - return fmt.Errorf("Missing parameter") - } timeout, _ := strconv.Atoi(r.Form.Get("t")) @@ -263,9 +240,6 @@ func (s *router) postContainersRestart(ctx context.Context, w http.ResponseWrite } func (s *router) postContainersPause(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - if vars == nil { - return fmt.Errorf("Missing parameter") - } if err := httputils.ParseForm(r); err != nil { return err } @@ -280,9 +254,6 @@ func (s *router) postContainersPause(ctx context.Context, w http.ResponseWriter, } func (s *router) postContainersUnpause(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - if vars == nil { - return fmt.Errorf("Missing parameter") - } if err := httputils.ParseForm(r); err != nil { return err } @@ -297,10 +268,6 @@ func (s *router) postContainersUnpause(ctx context.Context, w http.ResponseWrite } func (s *router) postContainersWait(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - if vars == nil { - return fmt.Errorf("Missing parameter") - } - status, err := s.daemon.ContainerWait(vars["name"], -1*time.Second) if err != nil { return err @@ -312,10 +279,6 @@ func (s *router) postContainersWait(ctx context.Context, w http.ResponseWriter, } func (s *router) getContainersChanges(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - if vars == nil { - return fmt.Errorf("Missing parameter") - } - changes, err := s.daemon.ContainerChanges(vars["name"]) if err != nil { return err @@ -325,10 +288,6 @@ func (s *router) getContainersChanges(ctx context.Context, w http.ResponseWriter } func (s *router) getContainersTop(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - if vars == nil { - return fmt.Errorf("Missing parameter") - } - if err := httputils.ParseForm(r); err != nil { return err } @@ -345,9 +304,6 @@ func (s *router) postContainerRename(ctx context.Context, w http.ResponseWriter, if err := httputils.ParseForm(r); err != nil { return err } - if vars == nil { - return fmt.Errorf("Missing parameter") - } name := vars["name"] newName := r.Form.Get("name") @@ -387,9 +343,6 @@ func (s *router) deleteContainers(ctx context.Context, w http.ResponseWriter, r if err := httputils.ParseForm(r); err != nil { return err } - if vars == nil { - return fmt.Errorf("Missing parameter") - } name := vars["name"] config := &daemon.ContainerRmConfig{ @@ -415,9 +368,6 @@ func (s *router) postContainersResize(ctx context.Context, w http.ResponseWriter if err := httputils.ParseForm(r); err != nil { return err } - if vars == nil { - return fmt.Errorf("Missing parameter") - } height, err := strconv.Atoi(r.Form.Get("h")) if err != nil { @@ -435,9 +385,6 @@ func (s *router) postContainersAttach(ctx context.Context, w http.ResponseWriter if err := httputils.ParseForm(r); err != nil { return err } - if vars == nil { - return fmt.Errorf("Missing parameter") - } containerName := vars["name"] if !s.daemon.Exists(containerName) { @@ -477,9 +424,6 @@ func (s *router) wsContainersAttach(ctx context.Context, w http.ResponseWriter, if err := httputils.ParseForm(r); err != nil { return err } - if vars == nil { - return fmt.Errorf("Missing parameter") - } containerName := vars["name"] if !s.daemon.Exists(containerName) { diff --git a/components/engine/api/server/router/local/copy.go b/components/engine/api/server/router/local/copy.go index 72ceb0a44e..ff749a02eb 100644 --- a/components/engine/api/server/router/local/copy.go +++ b/components/engine/api/server/router/local/copy.go @@ -16,10 +16,6 @@ import ( // postContainersCopy is deprecated in favor of getContainersArchive. func (s *router) postContainersCopy(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - if vars == nil { - return fmt.Errorf("Missing parameter") - } - if err := httputils.CheckForJSON(r); err != nil { return err } diff --git a/components/engine/api/server/router/local/exec.go b/components/engine/api/server/router/local/exec.go index aaafcd3e29..7d3c883e19 100644 --- a/components/engine/api/server/router/local/exec.go +++ b/components/engine/api/server/router/local/exec.go @@ -16,10 +16,6 @@ import ( ) func (s *router) getExecByID(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - if vars == nil { - return fmt.Errorf("Missing parameter 'id'") - } - eConfig, err := s.daemon.ContainerExecInspect(vars["id"]) if err != nil { return err @@ -118,10 +114,6 @@ func (s *router) postContainerExecResize(ctx context.Context, w http.ResponseWri if err := httputils.ParseForm(r); err != nil { return err } - if vars == nil { - return fmt.Errorf("Missing parameter") - } - height, err := strconv.Atoi(r.Form.Get("h")) if err != nil { return err diff --git a/components/engine/api/server/router/local/image.go b/components/engine/api/server/router/local/image.go index dbaae16524..3b453c9818 100644 --- a/components/engine/api/server/router/local/image.go +++ b/components/engine/api/server/router/local/image.go @@ -156,10 +156,6 @@ func (s *router) postImagesCreate(ctx context.Context, w http.ResponseWriter, r } func (s *router) postImagesPush(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - if vars == nil { - return fmt.Errorf("Missing parameter") - } - metaHeaders := map[string][]string{} for k, v := range r.Header { if strings.HasPrefix(k, "X-Meta-") { @@ -208,9 +204,6 @@ func (s *router) postImagesPush(ctx context.Context, w http.ResponseWriter, r *h } func (s *router) getImagesGet(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - if vars == nil { - return fmt.Errorf("Missing parameter") - } if err := httputils.ParseForm(r); err != nil { return err } @@ -243,9 +236,6 @@ func (s *router) deleteImages(ctx context.Context, w http.ResponseWriter, r *htt if err := httputils.ParseForm(r); err != nil { return err } - if vars == nil { - return fmt.Errorf("Missing parameter") - } name := vars["name"] @@ -265,10 +255,6 @@ func (s *router) deleteImages(ctx context.Context, w http.ResponseWriter, r *htt } func (s *router) getImagesByName(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - if vars == nil { - return fmt.Errorf("Missing parameter") - } - imageInspect, err := s.daemon.LookupImage(vars["name"]) if err != nil { return err @@ -456,10 +442,6 @@ func (s *router) getImagesJSON(ctx context.Context, w http.ResponseWriter, r *ht } func (s *router) getImagesHistory(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - if vars == nil { - return fmt.Errorf("Missing parameter") - } - name := vars["name"] history, err := s.daemon.ImageHistory(name) if err != nil { @@ -473,10 +455,6 @@ func (s *router) postImagesTag(ctx context.Context, w http.ResponseWriter, r *ht if err := httputils.ParseForm(r); err != nil { return err } - if vars == nil { - return fmt.Errorf("Missing parameter") - } - repo := r.Form.Get("repo") tag := r.Form.Get("tag") name := vars["name"] diff --git a/components/engine/api/server/router/local/inspect.go b/components/engine/api/server/router/local/inspect.go index b9bf697083..8b2a058e1a 100644 --- a/components/engine/api/server/router/local/inspect.go +++ b/components/engine/api/server/router/local/inspect.go @@ -1,7 +1,6 @@ package local import ( - "fmt" "net/http" "github.com/docker/docker/api/server/httputils" @@ -11,9 +10,6 @@ import ( // getContainersByName inspects containers configuration and serializes it as json. func (s *router) getContainersByName(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { displaySize := httputils.BoolValue(r, "size") - if vars == nil { - return fmt.Errorf("Missing parameter") - } var json interface{} var err error diff --git a/components/engine/api/server/server.go b/components/engine/api/server/server.go index a4b9fb4c42..598fcfb2bb 100644 --- a/components/engine/api/server/server.go +++ b/components/engine/api/server/server.go @@ -153,7 +153,12 @@ func (s *Server) makeHTTPHandler(handler httputils.APIFunc) http.HandlerFunc { ctx := context.Background() handlerFunc := s.handleWithGlobalMiddlewares(handler) - if err := handlerFunc(ctx, w, r, mux.Vars(r)); err != nil { + vars := mux.Vars(r) + if vars == nil { + vars = make(map[string]string) + } + + if err := handlerFunc(ctx, w, r, vars); err != nil { logrus.Errorf("Handler for %s %s returned error: %s", r.Method, r.URL.Path, utils.GetErrorMessage(err)) httputils.WriteError(w, err) }