From 5fb1ad70113d3272232c266b4ff58e9f7f646594 Mon Sep 17 00:00:00 2001
From: techknowlogick <matti@mdranta.net>
Date: Wed, 15 May 2019 08:01:53 -0400
Subject: [PATCH] Webhook Logs show proper HTTP Method, and allow change HTTP
 method in form (#6953)

* Fix #6951 - logs show proper HTTP Method, and allow change HTTP method
in form

* enforce POST method for webhook

* set default if method is empty
---
 models/webhook.go                            | 25 ++++++++++++--------
 routers/api/v1/utils/hook.go                 |  1 +
 routers/repo/webhook.go                      |  1 +
 templates/repo/settings/webhook/history.tmpl |  2 +-
 4 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/models/webhook.go b/models/webhook.go
index 51b91879ee..48c4de2ea3 100644
--- a/models/webhook.go
+++ b/models/webhook.go
@@ -755,17 +755,15 @@ func prepareWebhooks(e Engine, repo *Repository, event HookEventType, p api.Payl
 
 func (t *HookTask) deliver() {
 	t.IsDelivered = true
-	t.RequestInfo = &HookRequest{
-		Headers: map[string]string{},
-	}
-	t.ResponseInfo = &HookResponse{
-		Headers: map[string]string{},
-	}
 
 	timeout := time.Duration(setting.Webhook.DeliverTimeout) * time.Second
 
 	var req *httplib.Request
-	if t.HTTPMethod == http.MethodPost {
+	switch t.HTTPMethod {
+	case "":
+		log.Info("HTTP Method for webhook %d empty, setting to POST as default", t.ID)
+		fallthrough
+	case http.MethodPost:
 		req = httplib.Post(t.URL)
 		switch t.ContentType {
 		case ContentTypeJSON:
@@ -773,10 +771,10 @@ func (t *HookTask) deliver() {
 		case ContentTypeForm:
 			req.Param("payload", t.PayloadContent)
 		}
-	} else if t.HTTPMethod == http.MethodGet {
+	case http.MethodGet:
 		req = httplib.Get(t.URL).Param("payload", t.PayloadContent)
-	} else {
-		t.ResponseInfo.Body = fmt.Sprintf("Invalid http method: %v", t.HTTPMethod)
+	default:
+		log.Error("Invalid http method for webhook: [%d] %v", t.ID, t.HTTPMethod)
 		return
 	}
 
@@ -792,10 +790,17 @@ func (t *HookTask) deliver() {
 		SetTLSClientConfig(&tls.Config{InsecureSkipVerify: setting.Webhook.SkipTLSVerify})
 
 	// Record delivery information.
+	t.RequestInfo = &HookRequest{
+		Headers: map[string]string{},
+	}
 	for k, vals := range req.Headers() {
 		t.RequestInfo.Headers[k] = strings.Join(vals, ",")
 	}
 
+	t.ResponseInfo = &HookResponse{
+		Headers: map[string]string{},
+	}
+
 	defer func() {
 		t.Delivered = time.Now().UnixNano()
 		if t.IsSucceed {
diff --git a/routers/api/v1/utils/hook.go b/routers/api/v1/utils/hook.go
index da15850ec5..92846c5f2a 100644
--- a/routers/api/v1/utils/hook.go
+++ b/routers/api/v1/utils/hook.go
@@ -98,6 +98,7 @@ func addHook(ctx *context.APIContext, form *api.CreateHookOption, orgID, repoID
 		URL:         form.Config["url"],
 		ContentType: models.ToHookContentType(form.Config["content_type"]),
 		Secret:      form.Config["secret"],
+		HTTPMethod:  "POST",
 		HookEvent: &models.HookEvent{
 			ChooseEvents: true,
 			HookEvents: models.HookEvents{
diff --git a/routers/repo/webhook.go b/routers/repo/webhook.go
index 7695f208ce..8daf721b50 100644
--- a/routers/repo/webhook.go
+++ b/routers/repo/webhook.go
@@ -564,6 +564,7 @@ func WebHooksEditPost(ctx *context.Context, form auth.NewWebhookForm) {
 	w.Secret = form.Secret
 	w.HookEvent = ParseHookEvent(form.WebhookForm)
 	w.IsActive = form.Active
+	w.HTTPMethod = form.HTTPMethod
 	if err := w.UpdateEvent(); err != nil {
 		ctx.ServerError("UpdateEvent", err)
 		return
diff --git a/templates/repo/settings/webhook/history.tmpl b/templates/repo/settings/webhook/history.tmpl
index 7f85c702b5..577f357720 100644
--- a/templates/repo/settings/webhook/history.tmpl
+++ b/templates/repo/settings/webhook/history.tmpl
@@ -45,7 +45,7 @@
 							{{if .RequestInfo}}
 								<h5>{{$.i18n.Tr "repo.settings.webhook.headers"}}</h5>
 								<pre class="raw"><strong>Request URL:</strong> {{.URL}}
-<strong>Request method:</strong> POST
+<strong>Request method:</strong> {{if .HTTPMethod}}{{.HTTPMethod}}{{else}}POST{{end}}
 {{ range $key, $val := .RequestInfo.Headers }}<strong>{{$key}}:</strong> {{$val}}
 {{end}}</pre>
 								<h5>{{$.i18n.Tr "repo.settings.webhook.payload"}}</h5>