Skip to content

Commit

Permalink
JSON schema: skip pattern attribute compilation
Browse files Browse the repository at this point in the history
We don't use the regex "pattern" attribute for type checking,
but this still caused problems parsing schemas where the pattern
specified contained things that the Go regex dialect did not
support, such as negative lookahead. Solved for now by simply
removing the regex compilation of pattern properties.

Fixes open-policy-agent#4426

Signed-off-by: Anders Eknert <anders@eknert.com>
  • Loading branch information
anderseknert committed Mar 11, 2022
1 parent 785f742 commit f8dd7bb
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 16 deletions.
24 changes: 24 additions & 0 deletions ast/compile_test.go
Expand Up @@ -909,6 +909,30 @@ func TestCompilerCheckTypesWithSchema(t *testing.T) {
assertNotFailed(t, c)
}

func TestCompilerCheckTypesWithRegexPatternInSchema(t *testing.T) {
c := NewCompiler()
var schema interface{}
// Negative lookahead is not supported in the Go regex dialect, but this is still a valid
// JSON schema. Since we don't rely on the "pattern" attribute for type checking, ensure
// that this still works (by being ignored)
err := util.Unmarshal([]byte(`{
"properties": {
"name": {
"pattern": "^(?!testing:.*)[a-z]+$",
"type": "string"
}
}
}`), &schema)
if err != nil {
t.Fatal("Unexpected error:", err)
}
schemaSet := NewSchemaSet()
schemaSet.Put(SchemaRootRef, schema)
c.WithSchemas(schemaSet)
compileStages(c, c.checkTypes)
assertNotFailed(t, c)
}

func TestCompilerCheckTypesWithAllOfSchema(t *testing.T) {

tests := []struct {
Expand Down
15 changes: 5 additions & 10 deletions internal/gojsonschema/schema.go
Expand Up @@ -517,17 +517,12 @@ func (d *Schema) parseSchema(documentNode interface{}, currentSchema *SubSchema)
}
}

if pattern, err := getString(m, KeyPattern); err != nil {
// NOTE: Regex compilation step removed as we don't use "pattern" attribute for
// type checking, and this would cause schemas to fail if they included patterns
// that were valid ECMA regex dialect but not known to Go (i.e. the regexp.Compile
// function), such as patterns with negative lookahead
if _, err := getString(m, KeyPattern); err != nil {
return err
} else if pattern != nil {
regexpObject, err := regexp.Compile(*pattern)
if err != nil {
return errors.New(formatErrorDescription(
Locale.MustBeValidRegex(),
ErrorDetails{"key": KeyPattern},
))
}
currentSchema.pattern = regexpObject
}

if format, err := getString(m, KeyFormat); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions internal/gojsonschema/testdata/draft4/pattern.json
Expand Up @@ -9,9 +9,9 @@
"valid": true
},
{
"description": "a non-matching pattern is invalid",
"description": "a non-matching pattern is invalid (but ignored)",
"data": "abc",
"valid": false
"valid": true
},
{
"description": "ignores non-strings",
Expand Down
4 changes: 2 additions & 2 deletions internal/gojsonschema/testdata/draft6/pattern.json
Expand Up @@ -9,9 +9,9 @@
"valid": true
},
{
"description": "a non-matching pattern is invalid",
"description": "a non-matching pattern is invalid (but ignored)",
"data": "abc",
"valid": false
"valid": true
},
{
"description": "ignores non-strings",
Expand Down
4 changes: 2 additions & 2 deletions internal/gojsonschema/testdata/draft7/pattern.json
Expand Up @@ -9,9 +9,9 @@
"valid": true
},
{
"description": "a non-matching pattern is invalid",
"description": "a non-matching pattern is invalid (but ignored)",
"data": "abc",
"valid": false
"valid": true
},
{
"description": "ignores non-strings",
Expand Down

0 comments on commit f8dd7bb

Please sign in to comment.