Skip to content

Commit

Permalink
Extend redirect URI validation with protocol check
Browse files Browse the repository at this point in the history
  • Loading branch information
Raul Marrero authored and Rulox committed Feb 21, 2020
1 parent 5f3ff50 commit ae85531
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -992,7 +992,7 @@ In the example below, NGINX responds with a redirect when a response from an ups
codes: [404]
redirect:
code: 301
url: ${scheme}://cafe.example.com/error_${status}.html
url: ${scheme}://cafe.example.com/error.html
```

```eval_rst
Expand All @@ -1008,7 +1008,7 @@ redirect:
- ``int``
- No
* - ``url``
- The URL to redirect the request to. Supported NGINX variables: ``$scheme``\ , ``$status``\ and ``$http_x_forwarded_proto``\. Variables must be inclosed in curly braces. For example: ``${scheme}``.
- The URL to redirect the request to. Supported NGINX variables: ``$scheme``\ and ``$http_x_forwarded_proto``\. Variables must be inclosed in curly braces. For example: ``${scheme}``.
- ``string``
- Yes
```
Expand All @@ -1026,8 +1026,8 @@ return:
type: application/json
body: "{\"msg\": \"You don't have permission to do this\"}"
headers:
- name: x-debug-original-status
value: ${status}
- name: x-debug-original-statuses
value: ${upstream_status}
```

```eval_rst
Expand All @@ -1047,7 +1047,7 @@ return:
- ``string``
- No
* - ``body``
- The body of the response. Supported NGINX variable: ``$status`` \ . Variables must be inclosed in curly braces. For example: ``${status}``.
- The body of the response. Supported NGINX variable: ``$upstream_status`` \ . Variables must be inclosed in curly braces. For example: ``${upstream_status}``.
- ``string``
- Yes
* - ``headers``
Expand All @@ -1061,8 +1061,8 @@ return:
The header defines an HTTP Header for a canned response in an errorPage:
```yaml
name: x-debug-original-status
value: ${status}
name: x-debug-original-statuses
value: ${upstream_status}
```
```eval_rst
Expand All @@ -1078,7 +1078,7 @@ value: ${status}
- ``string``
- Yes
* - ``value``
- The value of the header. Supported NGINX variables: ``$status`` \ . Variables must be inclosed in curly braces. For example: ``${status}``.
- The value of the header. Supported NGINX variable: ``$upstream_status`` \ . Variables must be inclosed in curly braces. For example: ``${upstream_status}``.
- ``string``
- No
```
Expand Down
10 changes: 7 additions & 3 deletions pkg/apis/configuration/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ func validateErrorPage(errorPage v1.ErrorPage, fieldPath *field.Path) field.Erro
return allErrs
}

var errorPageReturnBodyVariable = map[string]bool{"status": true}
var errorPageReturnBodyVariable = map[string]bool{"upstream_status": true}

func validateErrorPageReturn(r *v1.ErrorPageReturn, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
Expand All @@ -659,7 +659,7 @@ func validateErrorPageReturn(r *v1.ErrorPageReturn, fieldPath *field.Path) field
return allErrs
}

var errorPageHeaderValueVariables = map[string]bool{"status": true}
var errorPageHeaderValueVariables = map[string]bool{"upstream_status": true}

func validateErrorPageHeader(h v1.Header, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
Expand All @@ -682,7 +682,7 @@ func validateErrorPageHeader(h v1.Header, fieldPath *field.Path) field.ErrorList
return allErrs
}

var validErrorPageRedirectVariables = map[string]bool{"scheme": true, "status": true, "http_x_forwarded_proto": true}
var validErrorPageRedirectVariables = map[string]bool{"scheme": true, "http_x_forwarded_proto": true}

func validateErrorPageRedirect(r *v1.ErrorPageRedirect, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
Expand Down Expand Up @@ -801,6 +801,10 @@ func validateRedirectURL(redirectURL string, fieldPath *field.Path, validVars ma
return append(allErrs, field.Required(fieldPath, "must specify a url"))
}

if !strings.Contains(redirectURL, "://") {
return append(allErrs, field.Invalid(fieldPath, redirectURL, "must contain the protocol with '://', for example http://, https:// or ${scheme}://"))
}

if !escapedStringsFmtRegexp.MatchString(redirectURL) {
msg := validation.RegexError(escapedStringsErrMsg, escapedStringsFmt, "http://www.nginx.com", "${scheme}://${host}/green/", `\"http://www.nginx.com\"`)
return append(allErrs, field.Invalid(fieldPath, redirectURL, msg))
Expand Down
14 changes: 9 additions & 5 deletions pkg/apis/configuration/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -946,7 +946,7 @@ func TestValidateRedirectURL(t *testing.T) {
msg: "x-forwarded-proto redirect url use case",
},
{
redirectURL: "${host}${request_uri}",
redirectURL: "http://${host}${request_uri}",
msg: "use multi variables, no scheme set",
},
{
Expand All @@ -962,7 +962,7 @@ func TestValidateRedirectURL(t *testing.T) {
msg: "url with escaped quotes",
},
{
redirectURL: "{abc}",
redirectURL: "http://{abc}",
msg: "url with curly braces with no $ prefix",
},
}
Expand All @@ -985,6 +985,10 @@ func TestValidateRedirectURLFails(t *testing.T) {
redirectURL: "",
msg: "url is blank",
},
{
redirectURL: "/uri",
msg: "url does not start with http://, https:// or ${scheme}://",
},
{
redirectURL: "$scheme://www.$host.com",
msg: "usage of nginx variable in url without ${}",
Expand Down Expand Up @@ -3041,7 +3045,7 @@ func TestValidateErrorPageReturn(t *testing.T) {
ActionReturn: v1.ActionReturn{
Code: 0,
Type: "",
Body: "Could not process request, try again. Status ${status}",
Body: "Could not process request, try again. Upstream status ${upstream_status}",
},
Headers: []v1.Header{
{
Expand All @@ -3054,7 +3058,7 @@ func TestValidateErrorPageReturn(t *testing.T) {
ActionReturn: v1.ActionReturn{
Code: 200,
Type: "application/json",
Body: `{\"message\": \"Could not process request, try again\", \"status\": \"${status}\"}`,
Body: `{\"message\": \"Could not process request, try again\", \"upstream_status\": \"${upstream_status}\"}`,
},
Headers: nil,
},
Expand Down Expand Up @@ -3207,7 +3211,7 @@ func TestValidateErrorPageHeader(t *testing.T) {
},
{
Name: "Header-Name",
Value: "${status}",
Value: "${upstream_status}",
},
}

Expand Down

0 comments on commit ae85531

Please sign in to comment.