From 9fc8b286c2067bc9905d6629cc3c9b52a9a0721c Mon Sep 17 00:00:00 2001 From: aeneasr <3372410+aeneasr@users.noreply.github.com> Date: Mon, 11 Apr 2022 21:09:47 +0200 Subject: [PATCH] fix: overzealous url validation Closes #930 --- rule/validator.go | 9 +++++---- rule/validator_test.go | 12 +++++++++++- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/rule/validator.go b/rule/validator.go index 46bc25038a..f67657dba1 100644 --- a/rule/validator.go +++ b/rule/validator.go @@ -21,7 +21,8 @@ package rule import ( - "github.com/asaskevich/govalidator" + "net/url" + "github.com/pkg/errors" "github.com/ory/herodot" @@ -126,13 +127,13 @@ func (v *ValidatorDefault) Validate(r *Rule) error { } if r.Match.URL == "" { - return errors.WithStack(herodot.ErrInternalServerError.WithReasonf(`Value "%s" of "match.url" field is not a valid url.`, r.Match.URL)) + return errors.WithStack(herodot.ErrInternalServerError.WithReasonf(`Value "%s" of "match.url" field must not be empty.`, r.Match.URL)) } if r.Upstream.URL == "" { // Having no upstream URL is fine here because the judge does not need an upstream! - } else if !govalidator.IsURL(r.Upstream.URL) { - return errors.WithStack(herodot.ErrInternalServerError.WithReasonf(`Value "%s" of "upstream.url" is not a valid url.`, r.Upstream.URL)) + } else if _, err := url.ParseRequestURI(r.Upstream.URL); err != nil { + return errors.WithStack(herodot.ErrInternalServerError.WithReasonf(`Value "%s" of "upstream.url" is not a valid url: %s`, r.Upstream.URL, err)) } if err := v.validateAuthenticators(r); err != nil { diff --git a/rule/validator_test.go b/rule/validator_test.go index 794964c107..20fe9740e0 100644 --- a/rule/validator_test.go +++ b/rule/validator_test.go @@ -58,7 +58,7 @@ func TestValidateRule(t *testing.T) { }, { r: &Rule{Match: &Match{}}, - expectErr: `Value "" of "match.url" field is not a valid url.`, + expectErr: `Value "" of "match.url" field must not be empty.`, }, { r: &Rule{ @@ -145,6 +145,16 @@ func TestValidateRule(t *testing.T) { Mutators: []Handler{{Handler: "noop"}}, }, }, + { + setup: prep(true, true, true), + r: &Rule{ + Match: &Match{URL: "https://www.ory.sh", Methods: []string{"MKCOL"}}, + Upstream: Upstream{URL: "http://tasks.foo-bar_baz"}, + Authenticators: []Handler{{Handler: "noop"}}, + Authorizer: Handler{Handler: "allow"}, + Mutators: []Handler{{Handler: "noop"}}, + }, + }, { setup: prep(true, true, false), r: &Rule{