diff --git a/internal/hook/hook.go b/internal/hook/hook.go index 0e4d70f..1131eb4 100644 --- a/internal/hook/hook.go +++ b/internal/hook/hook.go @@ -649,8 +649,7 @@ func (h *Hooks) LoadFromFile(path string, asTemplate bool) error { file = buf.Bytes() } - e = yaml.Unmarshal(file, h) - return e + return yaml.Unmarshal(file, h) } // Append appends hooks unless the new hooks contain a hook with an ID that already exists diff --git a/test/hooks.json.tmpl b/test/hooks.json.tmpl index e91f663..0c7fc0c 100644 --- a/test/hooks.json.tmpl +++ b/test/hooks.json.tmpl @@ -3,7 +3,7 @@ "id": "github", "execute-command": "{{ .Hookecho }}", "command-working-directory": "/", - "http-methods": ["POST"], + "http-methods": ["Post "], "include-command-output-in-response": true, "trigger-rule-mismatch-http-response-code": 400, "pass-environment-to-command": diff --git a/test/hooks.yaml.tmpl b/test/hooks.yaml.tmpl index 79e8987..f1e1204 100644 --- a/test/hooks.yaml.tmpl +++ b/test/hooks.yaml.tmpl @@ -1,6 +1,6 @@ - id: github http-methods: - - POST + - "Post " trigger-rule: and: - match: diff --git a/webhook.go b/webhook.go index f1dc5da..5bf6f3d 100644 --- a/webhook.go +++ b/webhook.go @@ -208,6 +208,10 @@ func main() { r.HandleFunc(hooksURL, hookHandler) } else { allowed := strings.Split(*httpMethods, ",") + for i := range allowed { + allowed[i] = strings.TrimSpace(allowed[i]) + } + r.HandleFunc(hooksURL, hookHandler).Methods(allowed...) } @@ -257,7 +261,7 @@ func main() { func hookHandler(w http.ResponseWriter, r *http.Request) { rid := middleware.GetReqID(r.Context()) - log.Printf("[%s] incoming HTTP request from %s\n", rid, r.RemoteAddr) + log.Printf("[%s] incoming HTTP %s request from %s\n", rid, r.Method, r.RemoteAddr) id := mux.Vars(r)["id"] @@ -272,6 +276,10 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { if len(matchedHook.HTTPMethods) != 0 { var allowed bool for i := range matchedHook.HTTPMethods { + // TODO(moorereason): refactor config loading and reloading to + // sanitize these methods once at load time. + matchedHook.HTTPMethods[i] = strings.ToUpper(strings.TrimSpace(matchedHook.HTTPMethods[i])) + if matchedHook.HTTPMethods[i] == r.Method { allowed = true break @@ -280,6 +288,7 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { if !allowed { w.WriteHeader(http.StatusMethodNotAllowed) + log.Printf("[%s] HTTP %s method not implemented for hook %q", rid, r.Method, id) return } } diff --git a/webhook_test.go b/webhook_test.go index 7bdfab7..8b53981 100644 --- a/webhook_test.go +++ b/webhook_test.go @@ -664,9 +664,9 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 }, // test with disallowed global HTTP method - {"global disallowed method", "bitbucket", []string{"POST"}, "GET", nil, `{}`, "application/json", http.StatusMethodNotAllowed, ``, ``}, + {"global disallowed method", "bitbucket", []string{"Post "}, "GET", nil, `{}`, "application/json", http.StatusMethodNotAllowed, ``, ``}, // test with disallowed HTTP method - {"disallowed method", "github", nil, "GET", nil, `{}`, "application/json", http.StatusMethodNotAllowed, ``, ``}, + {"disallowed method", "github", nil, "Get", nil, `{}`, "application/json", http.StatusMethodNotAllowed, ``, ``}, // test with custom return code {"empty payload", "github", nil, "POST", nil, "application/json", `{}`, http.StatusBadRequest, `Hook rules were not satisfied.`, ``}, // test with custom invalid http code, should default to 200 OK