Skip to content

Commit

Permalink
fix: overzealous url validation
Browse files Browse the repository at this point in the history
Closes #930
  • Loading branch information
aeneasr committed Apr 11, 2022
1 parent 2610d2c commit 9fc8b28
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 5 deletions.
9 changes: 5 additions & 4 deletions rule/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
package rule

import (
"github.com/asaskevich/govalidator"
"net/url"

"github.com/pkg/errors"

"github.com/ory/herodot"
Expand Down Expand Up @@ -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 {
Expand Down
12 changes: 11 additions & 1 deletion rule/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down

0 comments on commit 9fc8b28

Please sign in to comment.