Skip to content

Commit

Permalink
chore: Handle invalid YAML files containing unterminated strings (#1970)
Browse files Browse the repository at this point in the history
The old parser produced an error for these whereas the new parser just
ignores them.

Also renames the testdata directory from `protoyaml` to `parser` for
consistency.

Signed-off-by: Charith Ellawala <charith@cerbos.dev>

Signed-off-by: Charith Ellawala <charith@cerbos.dev>
  • Loading branch information
charithe committed Feb 1, 2024
1 parent 90f198a commit bc84737
Show file tree
Hide file tree
Showing 44 changed files with 72 additions and 2 deletions.
9 changes: 9 additions & 0 deletions internal/parser/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package parser

import (
"bytes"
"encoding/base64"
"errors"
"fmt"
Expand Down Expand Up @@ -107,6 +108,14 @@ func UnmarshalBytes[T proto.Message](contents []byte, factory func() T, opts ...
}

if len(f.Docs) == 0 {
if len(bytes.TrimSpace(contents)) > 0 {
// Special case for unterminated strings. See test case 20.
return nil, nil, NewUnmarshalError(&sourcev1.Error{
Kind: sourcev1.Error_KIND_PARSE_ERROR,
Message: "invalid document: contents are not valid YAML or JSON",
Position: &sourcev1.Position{Line: 1, Column: 1, Path: "$"},
})
}
return nil, nil, nil
}

Expand Down
4 changes: 2 additions & 2 deletions internal/parser/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import (
)

func TestUnmarshal(t *testing.T) {
testCases := test.LoadTestCases(t, "protoyaml")
testCases := test.LoadTestCases(t, "parser")
validator, err := protovalidate.New(protovalidate.WithMessages(&policyv1.Policy{}))
require.NoError(t, err)

Expand Down Expand Up @@ -140,7 +140,7 @@ func loadTestCase(t *testing.T, tc test.Case) (*privatev1.ProtoYamlTestCase, io.

func TestFind(t *testing.T) {
rnd := rand.New(rand.NewSource(42)) //nolint:gosec
testCases := test.LoadTestCases(t, "protoyaml")
testCases := test.LoadTestCases(t, "parser")
for _, testCase := range testCases {
tc, input := loadTestCase(t, testCase)
if len(tc.Want) == 0 {
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
14 changes: 14 additions & 0 deletions internal/test/testdata/parser/case_020.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"description": "Invalid YAML file with unterminated string",
"wantErrors": [
{
"kind": "KIND_PARSE_ERROR",
"position": {
"line": 1,
"column": 1,
"path": "$"
},
"message": "invalid document: contents are not valid YAML or JSON"
}
]
}
1 change: 1 addition & 0 deletions internal/test/testdata/parser/case_020.json.input
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"This is not valid YAML
32 changes: 32 additions & 0 deletions internal/test/testdata/parser/case_021.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"description": "YAML file with unterminated string",
"wantErrors": [
{
"kind": "KIND_PARSE_ERROR",
"position": {
"line": 4,
"column": 11,
"path": "$.variables"
},
"message": "expected map got Null"
}
],
"want": [
{
"message": {
"apiVersion": "api.cerbos.dev/v1"
},
"errors": [
{
"kind": "KIND_PARSE_ERROR",
"position": {
"line": 4,
"column": 11,
"path": "$.variables"
},
"message": "expected map got Null"
}
]
}
]
}
14 changes: 14 additions & 0 deletions internal/test/testdata/parser/case_021.json.input
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# yaml-language-server: $schema=../../../../../schema/jsonschema/cerbos/policy/v1/Policy.schema.json
---
apiVersion: api.cerbos.dev/v1
variables:
"x: y
resourcePolicy:
version: default
resource: leave_request
rules:
- actions: ['*']
effect: EFFECT_ALLOW
roles:
- admin
name: wildcard

0 comments on commit bc84737

Please sign in to comment.