Skip to content

Commit

Permalink
Fix webhook (TykTechnologies#2372)
Browse files Browse the repository at this point in the history
1. Added Timeout to HTTP client used for sending a request to webhook
2. Added new flag `useDefaultTemplate` in WebHookHandler
3. Set Content-Type header to "application/json" if default template is used.
4. Handle unsuccessful response code

 Fixes TykTechnologies#2371
  • Loading branch information
komalsukhani authored and buger committed Jul 10, 2019
1 parent c7f74f6 commit 36848bf
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 9 deletions.
36 changes: 27 additions & 9 deletions gateway/event_handler_webhooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"net/url"
"path/filepath"
"strings"
"time"

"github.com/Sirupsen/logrus"

Expand Down Expand Up @@ -38,7 +39,8 @@ type WebHookHandler struct {
template *template.Template // non-nil if Init is run without error
store storage.Handler

dashboardService DashboardServiceSender
useDefaultTemplate bool
dashboardService DashboardServiceSender
}

// createConfigObject by default tyk will provide a map[string]interface{} type as a conf, converting it
Expand Down Expand Up @@ -97,6 +99,7 @@ func (w *WebHookHandler) Init(handlerConf interface{}) error {
}).Error("Could not load the default template: ", err)
return err
}
w.useDefaultTemplate = true
}

log.WithFields(logrus.Fields{
Expand Down Expand Up @@ -187,6 +190,10 @@ func (w *WebHookHandler) BuildRequest(reqBody string) (*http.Request, error) {
req.Header.Set(key, val)
}

if req.Header.Get("Content-Type") == "" && w.useDefaultTemplate {
req.Header.Set("Content-Type", "application/json")
}

return req, nil
}

Expand Down Expand Up @@ -217,22 +224,33 @@ func (w *WebHookHandler) HandleEvent(em config.EventMessage) {
return
}

resp, err := http.DefaultClient.Do(req)
cli := &http.Client{Timeout: 30 * time.Second}

resp, err := cli.Do(req)
if err != nil {
log.WithFields(logrus.Fields{
"prefix": "webhooks",
}).Error("Webhook request failed: ", err)
} else {
defer resp.Body.Close()
content, err := ioutil.ReadAll(resp.Body)
if err == nil {
log.WithFields(logrus.Fields{
"prefix": "webhooks",
}).Debug(string(content))
if resp.StatusCode >= 200 && resp.StatusCode < 300 {
content, err := ioutil.ReadAll(resp.Body)
if err == nil {
log.WithFields(logrus.Fields{
"prefix": "webhooks",
"responseCode": resp.StatusCode,
}).Debug(string(content))
} else {
log.WithFields(logrus.Fields{
"prefix": "webhooks",
}).Error(err)
}

} else {
log.WithFields(logrus.Fields{
"prefix": "webhooks",
}).Error(err)
"prefix": "webhooks",
"responseCode": resp.StatusCode,
}).Error("Request to webhook failed")
}
}

Expand Down
48 changes: 48 additions & 0 deletions gateway/event_handler_webhooks_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package gateway

import (
"path/filepath"
"strings"
"testing"

Expand Down Expand Up @@ -96,6 +97,10 @@ func TestBuildRequest(t *testing.T) {
if got := req.Header.Get("User-Agent"); got != "Tyk-Hookshot" {
t.Error("Header User Agent is not correct!")
}

if got := req.Header.Get("Content-Type"); got != "application/json" {
t.Error("Header Content-Type is not correct!")
}
}

func TestCreateBody(t *testing.T) {
Expand Down Expand Up @@ -215,3 +220,46 @@ func TestNewCustomTemplate(t *testing.T) {
})
}
}

func TestWebhookContentTypeHeader(t *testing.T) {
globalConf := config.Global()
templatePath := globalConf.TemplatePath

tests := []struct {
Name string
TemplatePath string
InputHeaders map[string]string
ExpectedContentType string
}{
{"MissingTemplatePath", "", nil, "application/json"},
{"MissingTemplatePath/CustomHeaders", "", map[string]string{"Content-Type": "application/xml"}, "application/xml"},
{"InvalidTemplatePath", "randomPath", nil, "application/json"},
{"InvalidTemplatePath/CustomHeaders", "randomPath", map[string]string{"Content-Type": "application/xml"}, "application/xml"},
{"CustomTemplate", filepath.Join(templatePath, "breaker_webhook.json"), nil, ""},
{"CustomTemplate/CustomHeaders", filepath.Join(templatePath, "breaker_webhook.json"), map[string]string{"Content-Type": "application/json"}, "application/json"},
}

for _, ts := range tests {
t.Run(ts.Name, func(t *testing.T) {
conf := config.WebHookHandlerConf{
TemplatePath: ts.TemplatePath,
HeaderList: ts.InputHeaders,
}

hook := &WebHookHandler{}
if err := hook.Init(conf); err != nil {
t.Fatal("Webhook Init failed with err ", err)
}

req, err := hook.BuildRequest("")
if err != nil {
t.Fatal("Failed to build request with error ", err)
}

if req.Header.Get("Content-Type") != ts.ExpectedContentType {
t.Fatalf("Expect Content-Type %s. Got %s", ts.ExpectedContentType, req.Header.Get("Content-Type"))
}
})
}

}

0 comments on commit 36848bf

Please sign in to comment.