Merge branch 'development' into feature/context-provider-command

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

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

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,
 	},
 }
 

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
+}

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 }}",

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 }}'

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.ParseHeaders(r.Header)
-	req.Headers = valuesToMap(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))
+		err = req.ParseJSONPayload()
-		decoder.UseNumber()
+		if err != nil {
-
+			log.Printf("[%s] %s", req.ID, err)
-		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)
-			}
 		}
 
 	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)
+			log.Printf("[%s] %s", req.ID, err)
-		} else {
-			req.Payload = valuesToMap(fd)
 		}
 
 	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:

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",