From e641bca9b574b37211fada57222cfe747b8d2a4c Mon Sep 17 00:00:00 2001 From: Nick Ufer Date: Tue, 26 Oct 2021 23:07:27 +0200 Subject: [PATCH 1/3] fix: panic on webhook with empty body --- selfservice/hook/web_hook.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/selfservice/hook/web_hook.go b/selfservice/hook/web_hook.go index 9871124f1a4c..0caa04bdf618 100644 --- a/selfservice/hook/web_hook.go +++ b/selfservice/hook/web_hook.go @@ -252,7 +252,7 @@ func (e *WebHook) execute(data *templateContext) error { return fmt.Errorf("failed to parse web hook config: %w", err) } - var body io.Reader + var body *bytes.Reader if conf.method != "TRACE" { // According to the HTTP spec any request method, but TRACE is allowed to // have a body. Even this is a really bad practice for some of them, like for @@ -263,6 +263,9 @@ func (e *WebHook) execute(data *templateContext) error { } } + if body == nil { + body = bytes.NewReader(make([]byte, 0)) + } if err = doHttpCall(conf.method, conf.url, conf.auth, body); err != nil { return fmt.Errorf("failed to call web hook %w", err) } @@ -271,7 +274,7 @@ func (e *WebHook) execute(data *templateContext) error { func createBody(l *logrusx.Logger, templateURI string, data *templateContext) (*bytes.Reader, error) { if len(templateURI) == 0 { - return nil, nil + return bytes.NewReader(make([]byte, 0)), nil } f := fetcher.NewFetcher() From 351cfffcac0ef969516dd9f1bee916628c38b52c Mon Sep 17 00:00:00 2001 From: Nick Ufer Date: Wed, 27 Oct 2021 11:15:50 +0200 Subject: [PATCH 2/3] refactor: revert bytes.Reader to io.Reader --- selfservice/hook/web_hook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/selfservice/hook/web_hook.go b/selfservice/hook/web_hook.go index 0caa04bdf618..b3b5da1b5093 100644 --- a/selfservice/hook/web_hook.go +++ b/selfservice/hook/web_hook.go @@ -252,7 +252,7 @@ func (e *WebHook) execute(data *templateContext) error { return fmt.Errorf("failed to parse web hook config: %w", err) } - var body *bytes.Reader + var body io.Reader if conf.method != "TRACE" { // According to the HTTP spec any request method, but TRACE is allowed to // have a body. Even this is a really bad practice for some of them, like for From 15cade0e2a49b14fb62c941dfda5bbe391fe4c18 Mon Sep 17 00:00:00 2001 From: Nick Ufer Date: Wed, 27 Oct 2021 20:36:22 +0200 Subject: [PATCH 3/3] test: add hook createBody test --- selfservice/hook/web_hook_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/selfservice/hook/web_hook_test.go b/selfservice/hook/web_hook_test.go index 185515a06c91..27b3166fdda6 100644 --- a/selfservice/hook/web_hook_test.go +++ b/selfservice/hook/web_hook_test.go @@ -178,6 +178,12 @@ func TestJsonNetSupport(t *testing.T) { require.Len(t, hook.Entries, 1) assert.Contains(t, hook.LastEntry().Message, "support for filepaths without a 'file://' scheme will be dropped") }) + + t.Run("case=return non nil body reader on empty templateURI", func(t *testing.T) { + body, err := createBody(l, "", nil) + assert.NotNil(t, body) + assert.Nil(t, err) + }) } func TestWebHookConfig(t *testing.T) {