Merge pull request #45 from CiscoM31/case-statement

Add additional $select and $expand query validation

Author: migoff875

.github/workflows/golangci-lint.yml

@@ -14,7 +14,7 @@ jobs:
         uses: golangci/golangci-lint-action@v2
         with:
           # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version
-          version: v1.53.3
+          version: v1.55.2
 
           # Optional: working directory, useful for monorepos
           # working-directory: somedir

expand_parser.go

@@ -139,6 +139,14 @@ func ParseExpandItem(ctx context.Context, input tokenQueue) (*ExpandItem, error)
 				queue = &tokenQueue{}
 			}
 		} else if token.Value == "/" && stack.Empty() {
+			if queue.Empty() {
+				// Disallow extra leading and intermediate slash, like /Product and Product//Info
+				return nil, BadRequestError("Empty path segment in expand clause.")
+			}
+			if input.Empty() {
+				// Disallow extra trailing slash, like Product/
+				return nil, BadRequestError("Empty path segment in expand clause.")
+			}
 			// at root level, slashes separate path segments
 			item.Path = append(item.Path, queue.Dequeue())
 		} else if token.Value == ";" && stack.Size == 1 {

expand_parser_test.go

@@ -123,16 +123,27 @@ func TestExpandNestedParens(t *testing.T) {
 }
 
 func TestExpandNegativeCases(t *testing.T) {
-	input := "Products," // Extraneous comma
-	ctx := context.Background()
-	output, err := ParseExpandString(ctx, input)
-
-	if err == nil {
-		t.Error("Expected parsing to return error.")
-		return
-	}
-	if output != nil {
-		t.Error("Expected parsing to return nil output.")
-		return
+	testcases := []string{
+		"Products,",        // Extraneous comma
+		"Products//Data",   // Double slash
+		"/Products",        // Extraneous leading slash
+		"Products/",        // Extraneous trailing slash
+		"Orders,/Products", // Extraneous leading slash
+		"Products/,Orders", // Extraneous trailing slash
+	}
+
+	for _, testcase := range testcases {
+		t.Logf("Expand: %s", testcase)
+		ctx := context.Background()
+		output, err := ParseExpandString(ctx, testcase)
+
+		if err == nil {
+			t.Error("Expected parsing to return error.")
+			return
+		}
+		if output != nil {
+			t.Error("Expected parsing to return nil output.")
+			return
+		}
 	}
 }

select_parser.go

@@ -11,11 +11,16 @@ type SelectItem struct {
 }
 
 func ParseSelectString(ctx context.Context, sel string) (*GoDataSelectQuery, error) {
+	return GlobalExpressionParser.ParseSelectString(ctx, sel)
+}
+
+func (p *ExpressionParser) ParseSelectString(ctx context.Context, sel string) (*GoDataSelectQuery, error) {
 	items := strings.Split(sel, ",")
 
 	result := []*SelectItem{}
 
 	for _, item := range items {
+		item = strings.TrimSpace(item)
 
 		cfg, hasComplianceConfig := ctx.Value(odataCompliance).(OdataComplianceConfig)
 		if !hasComplianceConfig {
@@ -27,11 +32,28 @@ func ParseSelectString(ctx context.Context, sel string) (*GoDataSelectQuery, err
 			return nil, BadRequestError("Extra comma in $select.")
 		}
 
-		segments := []*Token{}
-		for _, val := range strings.Split(item, "/") {
-			segments = append(segments, &Token{Value: val})
+		if _, err := p.tokenizer.Tokenize(ctx, item); err != nil {
+			switch e := err.(type) {
+			case *GoDataError:
+				return nil, &GoDataError{
+					ResponseCode: e.ResponseCode,
+					Message:      "Invalid $select value",
+					Cause:        e,
+				}
+			default:
+				return nil, &GoDataError{
+					ResponseCode: 500,
+					Message:      "Invalid $select value",
+					Cause:        e,
+				}
+			}
+		} else {
+			segments := []*Token{}
+			for _, val := range strings.Split(item, "/") {
+				segments = append(segments, &Token{Value: val})
+			}
+			result = append(result, &SelectItem{segments})
 		}
-		result = append(result, &SelectItem{segments})
 	}
 
 	return &GoDataSelectQuery{result, sel}, nil

url_parser_test.go

@@ -119,7 +119,7 @@ func TestUrlParserStrictValidation(t *testing.T) {
 		return
 	}
 
-	testUrl = "Employees(1)/Sales.Manager?$filter=Name in ('Bob','Alice')&$select=Name,Address%3B$expand=Address($select=City)"
+	testUrl = "Employees(1)/Sales.Manager?$filter=Name in ('Bob','Alice')&$select=Name,Address&$expand=Address($select=City)"
 	parsedUrl, err = url.Parse(testUrl)
 	if err != nil {
 		t.Error(err)
@@ -530,6 +530,18 @@ func TestUnescapeStringTokens(t *testing.T) {
 			expectedFilterTree: nil,
 			expectedOrderBy:    nil,
 		},
+		{
+			url:                "/Product?$select=$select=Name",
+			errRegex:           regexp.MustCompile(`Invalid \$select\ value.`),
+			expectedFilterTree: nil,
+			expectedOrderBy:    nil,
+		},
+		{
+			url:                "/Product?$select=$Name",
+			errRegex:           regexp.MustCompile(`Invalid \$select\ value.`),
+			expectedFilterTree: nil,
+			expectedOrderBy:    nil,
+		},
 		{
 			url:      "/Product?$compute=Price mul Quantity as TotalPrice",
 			errRegex: nil,