Skip to content

Commit 66a4b98

Browse files
EmyrkEmyrk
committed
feat: Error on excessive invalid search keys
1 parent 8850ce0 commit 66a4b98

File tree

4 files changed

+158
-13
lines changed

4 files changed

+158
-13
lines changed

‎coderd/httpapi/queryparams.go

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,38 @@ type QueryParamParser struct {
2020
// Errors is the set of errors to return via the API. If the length
2121
// of this set is 0, there are no errors!.
2222
Errors []codersdk.ValidationError
23+
// Parsed is a map of all query params that were parsed. This is useful
24+
// for checking if extra query params were passed in.
25+
Parsed map[string]bool
2326
}
2427

2528
func NewQueryParamParser() *QueryParamParser {
2629
return &QueryParamParser{
2730
Errors: []codersdk.ValidationError{},
31+
Parsed: map[string]bool{},
2832
}
2933
}
3034

35+
// ErrorExcessParams checks if any query params were passed in that were not
36+
// parsed. If so, it adds an error to the parser as these values are not valid
37+
// query parameters.
38+
func (p *QueryParamParser) ErrorExcessParams(values url.Values) {
39+
for k := range values {
40+
if _, ok := p.Parsed[k]; !ok {
41+
p.Errors = append(p.Errors, codersdk.ValidationError{
42+
Field: k,
43+
Detail: fmt.Sprintf("Query param %q is not a valid query param", k),
44+
})
45+
}
46+
}
47+
}
48+
49+
func (p *QueryParamParser) addParsed(key string) {
50+
p.Parsed[key] = true
51+
}
52+
3153
func (p *QueryParamParser) Int(vals url.Values, def int, queryParam string) int {
32-
v, err := parseQueryParam(vals, strconv.Atoi, def, queryParam)
54+
v, err := parseQueryParam(p, vals, strconv.Atoi, def, queryParam)
3355
if err != nil {
3456
p.Errors = append(p.Errors, codersdk.ValidationError{
3557
Field: queryParam,
@@ -40,14 +62,16 @@ func (p *QueryParamParser) Int(vals url.Values, def int, queryParam string) int
4062
}
4163

4264
func (p *QueryParamParser) UUIDorMe(vals url.Values, def uuid.UUID, me uuid.UUID, queryParam string) uuid.UUID {
43-
if vals.Get(queryParam) == "me" {
44-
return me
45-
}
46-
return p.UUID(vals, def, queryParam)
65+
return ParseCustom(p, vals, def, queryParam, func(v string) (uuid.UUID, error) {
66+
if v == "me" {
67+
return me, nil
68+
}
69+
return uuid.Parse(v)
70+
})
4771
}
4872

4973
func (p *QueryParamParser) UUID(vals url.Values, def uuid.UUID, queryParam string) uuid.UUID {
50-
v, err := parseQueryParam(vals, uuid.Parse, def, queryParam)
74+
v, err := parseQueryParam(p, vals, uuid.Parse, def, queryParam)
5175
if err != nil {
5276
p.Errors = append(p.Errors, codersdk.ValidationError{
5377
Field: queryParam,
@@ -58,7 +82,7 @@ func (p *QueryParamParser) UUID(vals url.Values, def uuid.UUID, queryParam strin
5882
}
5983

6084
func (p *QueryParamParser) UUIDs(vals url.Values, def []uuid.UUID, queryParam string) []uuid.UUID {
61-
v, err := parseQueryParam(vals, func(v string) ([]uuid.UUID, error) {
85+
v, err := parseQueryParam(p, vals, func(v string) ([]uuid.UUID, error) {
6286
var badValues []string
6387
strs := strings.Split(v, ",")
6488
ids := make([]uuid.UUID, 0, len(strs))
@@ -85,15 +109,15 @@ func (p *QueryParamParser) UUIDs(vals url.Values, def []uuid.UUID, queryParam st
85109
return v
86110
}
87111

88-
func (*QueryParamParser) String(vals url.Values, def string, queryParam string) string {
89-
v, _ := parseQueryParam(vals, func(v string) (string, error) {
112+
func (p *QueryParamParser) String(vals url.Values, def string, queryParam string) string {
113+
v, _ := parseQueryParam(p, vals, func(v string) (string, error) {
90114
return v, nil
91115
}, def, queryParam)
92116
return v
93117
}
94118

95-
func (*QueryParamParser) Strings(vals url.Values, def []string, queryParam string) []string {
96-
v, _ := parseQueryParam(vals, func(v string) ([]string, error) {
119+
func (p *QueryParamParser) Strings(vals url.Values, def []string, queryParam string) []string {
120+
v, _ := parseQueryParam(p, vals, func(v string) ([]string, error) {
97121
if v == "" {
98122
return []string{}, nil
99123
}
@@ -105,7 +129,7 @@ func (*QueryParamParser) Strings(vals url.Values, def []string, queryParam strin
105129
// ParseCustom has to be a function, not a method on QueryParamParser because generics
106130
// cannot be used on struct methods.
107131
func ParseCustom[T any](parser *QueryParamParser, vals url.Values, def T, queryParam string, parseFunc func(v string) (T, error)) T {
108-
v, err := parseQueryParam(vals, parseFunc, def, queryParam)
132+
v, err := parseQueryParam(parser, vals, parseFunc, def, queryParam)
109133
if err != nil {
110134
parser.Errors = append(parser.Errors, codersdk.ValidationError{
111135
Field: queryParam,
@@ -115,7 +139,8 @@ func ParseCustom[T any](parser *QueryParamParser, vals url.Values, def T, queryP
115139
return v
116140
}
117141

118-
func parseQueryParam[T any](vals url.Values, parse func(v string) (T, error), def T, queryParam string) (T, error) {
142+
func parseQueryParam[T any](parser *QueryParamParser, vals url.Values, parse func(v string) (T, error), def T, queryParam string) (T, error) {
143+
parser.addParsed(queryParam)
119144
if !vals.Has(queryParam) || vals.Get(queryParam) == "" {
120145
return def, nil
121146
}

‎coderd/searchquery/search.go

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
package searchquery
2+
3+
import (
4+
"fmt"
5+
"net/url"
6+
"strings"
7+
"time"
8+
9+
"github.com/coder/coder/coderd/httpapi"
10+
"golang.org/x/xerrors"
11+
12+
"github.com/coder/coder/coderd/database"
13+
14+
"github.com/coder/coder/codersdk"
15+
)
16+
17+
func Workspace(query string, page codersdk.Pagination, agentInactiveDisconnectTimeout time.Duration) (database.GetWorkspacesParams, []codersdk.ValidationError) {
18+
filter := database.GetWorkspacesParams{
19+
AgentInactiveDisconnectTimeoutSeconds: int64(agentInactiveDisconnectTimeout.Seconds()),
20+
21+
Offset: int32(page.Offset),
22+
Limit: int32(page.Limit),
23+
}
24+
25+
if query == "" {
26+
return filter, nil
27+
}
28+
29+
// Always lowercase for all searches.
30+
query = strings.ToLower(query)
31+
values, errors := searchTerms(query, func(term string, values url.Values) error {
32+
// It is a workspace name, and maybe includes an owner
33+
parts := splitQueryParameterByDelimiter(term, '/', false)
34+
switch len(parts) {
35+
case 1:
36+
values.Set("name", parts[0])
37+
case 2:
38+
values.Set("owner", parts[0])
39+
values.Set("name", parts[1])
40+
default:
41+
return xerrors.Errorf("Query element %q can only contain 1 '/'", term)
42+
}
43+
return nil
44+
})
45+
if errors != nil {
46+
return filter, errors
47+
}
48+
49+
parser := httpapi.NewQueryParamParser()
50+
filter.OwnerUsername = parser.String(values, "", "owner")
51+
filter.TemplateName = parser.String(values, "", "template")
52+
filter.Name = parser.String(values, "", "name")
53+
filter.Status = parser.String(values, "", "status")
54+
filter.HasAgent = parser.String(values, "", "has-agent")
55+
parser.ErrorExcessParams(values)
56+
return filter, parser.Errors
57+
}
58+
59+
func searchTerms(query string, defaultKey func(term string, values url.Values) error) (url.Values, []codersdk.ValidationError) {
60+
searchValues := make(url.Values)
61+
62+
// Because we do this in 2 passes, we want to maintain quotes on the first
63+
// pass. Further splitting occurs on the second pass and quotes will be
64+
// dropped.
65+
elements := splitQueryParameterByDelimiter(query, ' ', true)
66+
for _, element := range elements {
67+
parts := splitQueryParameterByDelimiter(element, ':', false)
68+
switch len(parts) {
69+
case 1:
70+
// No key:value pair. Use default behavior.
71+
err := defaultKey(element, searchValues)
72+
if err != nil {
73+
return nil, []codersdk.ValidationError{
74+
{Field: "q", Detail: err.Error()},
75+
}
76+
}
77+
case 2:
78+
searchValues.Set(strings.ToLower(parts[0]), parts[1])
79+
default:
80+
return nil, []codersdk.ValidationError{
81+
{
82+
Field: "q",
83+
Detail: fmt.Sprintf("Query element %q can only contain 1 ':'", element),
84+
},
85+
}
86+
}
87+
}
88+
return searchValues, nil
89+
}

‎coderd/searchquery/split.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package searchquery
2+
3+
import "strings"
4+
5+
// splitQueryParameterByDelimiter takes a query string and splits it into the individual elements
6+
// of the query. Each element is separated by a delimiter. All quoted strings are
7+
// kept as a single element.
8+
//
9+
// Although all our names cannot have spaces, that is a validation error.
10+
// We should still parse the quoted string as a single value so that validation
11+
// can properly fail on the space. If we do not, a value of `template:"my name"`
12+
// will search `template:"my name:name"`, which produces an empty list instead of
13+
// an error.
14+
// nolint:revive
15+
func splitQueryParameterByDelimiter(query string, delimiter rune, maintainQuotes bool) []string {
16+
quoted := false
17+
parts := strings.FieldsFunc(query, func(r rune) bool {
18+
if r == '"' {
19+
quoted = !quoted
20+
}
21+
return !quoted && r == delimiter
22+
})
23+
if !maintainQuotes {
24+
for i, part := range parts {
25+
parts[i] = strings.Trim(part, "\"")
26+
}
27+
}
28+
29+
return parts
30+
}

‎coderd/workspaces.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1296,6 +1296,7 @@ func workspaceSearchQuery(query string, page codersdk.Pagination, agentInactiveD
12961296
return filter, parser.Errors
12971297
}
12981298

1299+
12991300
// splitQueryParameterByDelimiter takes a query string and splits it into the individual elements
13001301
// of the query. Each element is separated by a delimiter. All quoted strings are
13011302
// kept as a single element.

0 commit comments

Comments
 (0)