Merge pull request #41 from CiscoM31/strict-parsing

Disallow extraneous commas in $expand $select $orderby query options

Author: migoff875

expand_parser.go

@@ -162,6 +162,16 @@ func ParseExpandItem(ctx context.Context, input tokenQueue) (*ExpandItem, error)
 		item.Path = append(item.Path, queue.Dequeue())
 	}
 
+	cfg, hasComplianceConfig := ctx.Value(odataCompliance).(OdataComplianceConfig)
+	if !hasComplianceConfig {
+		// Strict ODATA compliance by default.
+		cfg = ComplianceStrict
+	}
+
+	if len(item.Path) == 0 && cfg&ComplianceIgnoreInvalidComma == 0 {
+		return nil, BadRequestError("Extra comma in $expand.")
+	}
+
 	return item, nil
 }
 

expand_parser_test.go

@@ -121,3 +121,18 @@ func TestExpandNestedParens(t *testing.T) {
 		return
 	}
 }
+
+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
+	}
+}

orderby_parser.go

@@ -31,6 +31,17 @@ func (p *ExpressionParser) ParseOrderByString(ctx context.Context, orderby strin
 
 	for _, v := range items {
 		v = strings.TrimSpace(v)
+
+		cfg, hasComplianceConfig := ctx.Value(odataCompliance).(OdataComplianceConfig)
+		if !hasComplianceConfig {
+			// Strict ODATA compliance by default.
+			cfg = ComplianceStrict
+		}
+
+		if len(v) == 0 && cfg&ComplianceIgnoreInvalidComma == 0 {
+			return nil, BadRequestError("Extra comma in $orderby.")
+		}
+
 		var order string
 		vLower := strings.ToLower(v)
 		if strings.HasSuffix(vLower, " "+ASC) {

select_parser.go

@@ -16,6 +16,17 @@ func ParseSelectString(ctx context.Context, sel string) (*GoDataSelectQuery, err
 	result := []*SelectItem{}
 
 	for _, item := range items {
+
+		cfg, hasComplianceConfig := ctx.Value(odataCompliance).(OdataComplianceConfig)
+		if !hasComplianceConfig {
+			// Strict ODATA compliance by default.
+			cfg = ComplianceStrict
+		}
+
+		if len(strings.TrimSpace(item)) == 0 && cfg&ComplianceIgnoreInvalidComma == 0 {
+			return nil, BadRequestError("Extra comma in $select.")
+		}
+
 		segments := []*Token{}
 		for _, val := range strings.Split(item, "/") {
 			segments = append(segments, &Token{Value: val})

url_parser_test.go

@@ -494,6 +494,42 @@ func TestUnescapeStringTokens(t *testing.T) {
 			expectedFilterTree: nil,
 			expectedOrderBy:    nil,
 		},
+		{
+			url:                "/Product?$orderby=Name,",
+			errRegex:           regexp.MustCompile(`Extra comma in \$orderby\.`),
+			expectedFilterTree: nil,
+			expectedOrderBy:    nil,
+		},
+		{
+			url:                "/Product?$orderby=Name,,Count",
+			errRegex:           regexp.MustCompile(`Extra comma in \$orderby\.`),
+			expectedFilterTree: nil,
+			expectedOrderBy:    nil,
+		},
+		{
+			url:                "/Product?$orderby=,Name",
+			errRegex:           regexp.MustCompile(`Extra comma in \$orderby\.`),
+			expectedFilterTree: nil,
+			expectedOrderBy:    nil,
+		},
+		{
+			url:                "/Product?$select=Name,",
+			errRegex:           regexp.MustCompile(`Extra comma in \$select\.`),
+			expectedFilterTree: nil,
+			expectedOrderBy:    nil,
+		},
+		{
+			url:                "/Product?$select=Name,,Count",
+			errRegex:           regexp.MustCompile(`Extra comma in \$select\.`),
+			expectedFilterTree: nil,
+			expectedOrderBy:    nil,
+		},
+		{
+			url:                "/Product?$select=,Name",
+			errRegex:           regexp.MustCompile(`Extra comma in \$select\.`),
+			expectedFilterTree: nil,
+			expectedOrderBy:    nil,
+		},
 		{
 			url:      "/Product?$compute=Price mul Quantity as TotalPrice",
 			errRegex: nil,