diff --git a/docs/Referencing-Request-Values.md b/docs/Referencing-Request-Values.md index a76e57f..9731722 100644 --- a/docs/Referencing-Request-Values.md +++ b/docs/Referencing-Request-Values.md @@ -2,7 +2,6 @@ There are four types of request values: 1. Context values - These are the values provided by the `pre-hook-command` output. ```json diff --git a/internal/hook/hook.go b/internal/hook/hook.go index d38415a..9344691 100644 --- a/internal/hook/hook.go +++ b/internal/hook/hook.go @@ -17,9 +17,7 @@ import ( "log" "math" "net" - "net/http" "net/textproto" - "net/url" "os" "reflect" "regexp" @@ -50,33 +48,6 @@ const ( EnvNamespace string = "HOOK_" ) -// Request represents a webhook request. -type Request struct { - // The request ID set by the RequestID middleware. - ID string - - // The Content-Type of the request. - ContentType string - - // The raw request body. - Body []byte - - // Headers is a map of the parsed headers. - Headers map[string]interface{} - - // Query is a map of the parsed URL query values. - Query map[string]interface{} - - // Payload is a map of the parsed payload. - Payload map[string]interface{} - - // Context is a map of the parsed pre-hook command result - Context map[string]interface{} - - // The underlying HTTP request. - RawRequest *http.Request -} - // ParameterNodeError describes an error walking a parameter node. type ParameterNodeError struct { key string @@ -870,7 +841,9 @@ func (r OrRule) Evaluate(req *Request) (bool, error) { for _, v := range r { rv, err := v.Evaluate(req) if err != nil { - return false, err + if !IsParameterNodeError(err) { + return false, err + } } res = res || rv diff --git a/internal/hook/hook_test.go b/internal/hook/hook_test.go index 84ad248..fa912ef 100644 --- a/internal/hook/hook_test.go +++ b/internal/hook/hook_test.go @@ -661,12 +661,12 @@ var orRuleTests = []struct { }, // failures { - "invalid rule", + "missing parameter node", OrRule{ {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, }, map[string]interface{}{"Y": "Z"}, nil, nil, nil, []byte{}, - false, true, + false, false, }, } diff --git a/internal/hook/request.go b/internal/hook/request.go new file mode 100644 index 0000000..7fa9714 --- /dev/null +++ b/internal/hook/request.go @@ -0,0 +1,119 @@ +package hook + +import ( + "bytes" + "encoding/json" + "fmt" + "net/http" + "net/url" + "unicode" + + "github.com/clbanning/mxj" +) + +// Request represents a webhook request. +type Request struct { + // The request ID set by the RequestID middleware. + ID string + + // The Content-Type of the request. + ContentType string + + // The raw request body. + Body []byte + + // Headers is a map of the parsed headers. + Headers map[string]interface{} + + // Query is a map of the parsed URL query values. + Query map[string]interface{} + + // Payload is a map of the parsed payload. + Payload map[string]interface{} + + // Context is a map of the parsed pre-hook command result + Context map[string]interface{} + + // The underlying HTTP request. + RawRequest *http.Request +} + +func (r *Request) ParseJSONPayload() error { + decoder := json.NewDecoder(bytes.NewReader(r.Body)) + decoder.UseNumber() + + var firstChar byte + for i := 0; i < len(r.Body); i++ { + if unicode.IsSpace(rune(r.Body[i])) { + continue + } + firstChar = r.Body[i] + break + } + + if firstChar == byte('[') { + var arrayPayload interface{} + err := decoder.Decode(&arrayPayload) + if err != nil { + return fmt.Errorf("error parsing JSON array payload %+v", err) + } + + r.Payload = make(map[string]interface{}, 1) + r.Payload["root"] = arrayPayload + } else { + err := decoder.Decode(&r.Payload) + if err != nil { + return fmt.Errorf("error parsing JSON payload %+v", err) + } + } + + return nil +} + +func (r *Request) ParseHeaders(headers map[string][]string) { + r.Headers = make(map[string]interface{}, len(headers)) + + for k, v := range headers { + if len(v) > 0 { + r.Headers[k] = v[0] + } + } +} + +func (r *Request) ParseQuery(query map[string][]string) { + r.Query = make(map[string]interface{}, len(query)) + + for k, v := range query { + if len(v) > 0 { + r.Query[k] = v[0] + } + } +} + +func (r *Request) ParseFormPayload() error { + fd, err := url.ParseQuery(string(r.Body)) + if err != nil { + return fmt.Errorf("error parsing form payload %+v", err) + } + + r.Payload = make(map[string]interface{}, len(fd)) + + for k, v := range fd { + if len(v) > 0 { + r.Payload[k] = v[0] + } + } + + return nil +} + +func (r *Request) ParseXMLPayload() error { + var err error + + r.Payload, err = mxj.NewMapXmlReader(bytes.NewReader(r.Body)) + if err != nil { + return fmt.Errorf("error parsing XML payload: %+v", err) + } + + return nil +} diff --git a/test/hooks.json.tmpl b/test/hooks.json.tmpl index 3075762..b94ed1a 100644 --- a/test/hooks.json.tmpl +++ b/test/hooks.json.tmpl @@ -276,6 +276,76 @@ } ], }, + { + "id": "issue-471", + "execute-command": "{{ .Hookecho }}", + "response-message": "success", + "trigger-rule": + { + "or": + [ + { + "match": + { + "parameter": + { + "source": "payload", + "name": "foo" + }, + "type": "value", + "value": "bar" + } + }, + { + "match": + { + "parameter": + { + "source": "payload", + "name": "exists" + }, + "type": "value", + "value": 1 + } + } + ] + } + }, + { + "id": "issue-471-and", + "execute-command": "{{ .Hookecho }}", + "response-message": "success", + "trigger-rule": + { + "and": + [ + { + "match": + { + "parameter": + { + "source": "payload", + "name": "foo" + }, + "type": "value", + "value": "bar" + } + }, + { + "match": + { + "parameter": + { + "source": "payload", + "name": "exists" + }, + "type": "value", + "value": 1 + } + } + ] + } + }, { "id": "empty-payload-signature", "execute-command": "{{ .Hookecho }}", diff --git a/test/hooks.yaml.tmpl b/test/hooks.yaml.tmpl index 16aa8c1..325ef27 100644 --- a/test/hooks.yaml.tmpl +++ b/test/hooks.yaml.tmpl @@ -163,6 +163,42 @@ execute-command: '{{ .Hookecho }} foo' include-command-output-in-response: true +- id: issue-471 + execute-command: '{{ .Hookecho }}' + response-message: success + trigger-rule: + or: + - match: + parameter: + source: payload + name: foo + type: value + value: bar + - match: + parameter: + source: payload + name: exists + type: value + value: 1 + +- id: issue-471-and + execute-command: '{{ .Hookecho }}' + response-message: success + trigger-rule: + and: + - match: + parameter: + source: payload + name: foo + type: value + value: bar + - match: + parameter: + source: payload + name: exists + type: value + value: 1 + - id: empty-payload-signature include-command-output-in-response: true execute-command: '{{ .Hookecho }}' diff --git a/webhook.go b/webhook.go index 422e966..9949b5e 100644 --- a/webhook.go +++ b/webhook.go @@ -1,7 +1,6 @@ package main import ( - "bytes" "crypto/tls" "encoding/base64" "encoding/json" @@ -12,19 +11,16 @@ import ( "log" "net" "net/http" - "net/url" "os" "os/exec" "path/filepath" "strings" "time" - "unicode" "github.com/adnanh/webhook/internal/hook" "github.com/adnanh/webhook/internal/middleware" "github.com/adnanh/webhook/internal/pidfile" - "github.com/clbanning/mxj" chimiddleware "github.com/go-chi/chi/middleware" "github.com/gorilla/mux" fsnotify "gopkg.in/fsnotify.v1" @@ -375,55 +371,26 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { } } - // parse headers - req.Headers = valuesToMap(r.Header) + req.ParseHeaders(r.Header) + req.ParseQuery(r.URL.Query()) - // parse query variables - req.Query = valuesToMap(r.URL.Query()) - - // parse body switch { case strings.Contains(req.ContentType, "json"): - decoder := json.NewDecoder(bytes.NewReader(req.Body)) - decoder.UseNumber() - - var firstChar byte - for i := 0; i < len(req.Body); i++ { - if unicode.IsSpace(rune(req.Body[i])) { - continue - } - firstChar = req.Body[i] - break - } - - if firstChar == byte('[') { - var arrayPayload interface{} - err := decoder.Decode(&arrayPayload) - if err != nil { - log.Printf("[%s] error parsing JSON array payload %+v\n", req.ID, err) - } - - req.Payload = make(map[string]interface{}, 1) - req.Payload["root"] = arrayPayload - } else { - err := decoder.Decode(&req.Payload) - if err != nil { - log.Printf("[%s] error parsing JSON payload %+v\n", req.ID, err) - } + err = req.ParseJSONPayload() + if err != nil { + log.Printf("[%s] %s", req.ID, err) } case strings.Contains(req.ContentType, "x-www-form-urlencoded"): - fd, err := url.ParseQuery(string(req.Body)) + err = req.ParseFormPayload() if err != nil { - log.Printf("[%s] error parsing form payload %+v\n", req.ID, err) - } else { - req.Payload = valuesToMap(fd) + log.Printf("[%s] %s", req.ID, err) } case strings.Contains(req.ContentType, "xml"): - req.Payload, err = mxj.NewMapXmlReader(bytes.NewReader(req.Body)) + err = req.ParseXMLPayload() if err != nil { - log.Printf("[%s] error parsing XML payload: %+v\n", req.ID, err) + log.Printf("[%s] %s", req.ID, err) } case isMultipart: diff --git a/webhook_test.go b/webhook_test.go index c8a46cc..d12826d 100644 --- a/webhook_test.go +++ b/webhook_test.go @@ -606,6 +606,32 @@ binary data ``, }, + { + "issue-471", + "issue-471", + nil, + "POST", + nil, + "application/json", + `{"exists": 1}`, + http.StatusOK, + `success`, + ``, + }, + + { + "issue-471-and", + "issue-471-and", + nil, + "POST", + nil, + "application/json", + `{"exists": 1}`, + http.StatusOK, + `Hook rules were not satisfied.`, + `parameter node not found`, + }, + { "missing-cmd-arg", // missing head_commit.author.email "github",