From 389ce0aae6a303660e591ef80272322ac82854e2 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 --- api/server/httputils/form.go | 3 -- api/server/router/local/container.go | 56 ---------------------------- api/server/router/local/copy.go | 4 -- api/server/router/local/exec.go | 8 ---- api/server/router/local/image.go | 22 ----------- api/server/router/local/inspect.go | 4 -- api/server/server.go | 7 +++- 7 files changed, 6 insertions(+), 98 deletions(-) diff --git a/api/server/httputils/form.go b/api/server/httputils/form.go index 5180b99f1..20188c12d 100644 --- a/api/server/httputils/form.go +++ b/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/api/server/router/local/container.go b/api/server/router/local/container.go index d1e352dd3..9cf14367d 100644 --- a/api/server/router/local/container.go +++ b/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/api/server/router/local/copy.go b/api/server/router/local/copy.go index 72ceb0a44..ff749a02e 100644 --- a/api/server/router/local/copy.go +++ b/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/api/server/router/local/exec.go b/api/server/router/local/exec.go index aaafcd3e2..7d3c883e1 100644 --- a/api/server/router/local/exec.go +++ b/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/api/server/router/local/image.go b/api/server/router/local/image.go index dbaae1652..3b453c981 100644 --- a/api/server/router/local/image.go +++ b/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/api/server/router/local/inspect.go b/api/server/router/local/inspect.go index b9bf69708..8b2a058e1 100644 --- a/api/server/router/local/inspect.go +++ b/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/api/server/server.go b/api/server/server.go index a4b9fb4c4..598fcfb2b 100644 --- a/api/server/server.go +++ b/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) }