diff --git a/cypher/frontend/expression.go b/cypher/frontend/expression.go index 994105bc..8385d0db 100644 --- a/cypher/frontend/expression.go +++ b/cypher/frontend/expression.go @@ -424,5 +424,5 @@ func (s *NonArithmeticOperatorExpressionVisitor) EnterOC_PropertyKeyName(ctx *pa } func (s *NonArithmeticOperatorExpressionVisitor) ExitOC_PropertyKeyName(ctx *parser.OC_PropertyKeyNameContext) { - s.PropertyKeyName = s.ctx.Exit().(*SymbolicNameOrReservedWordVisitor).Name + s.PropertyKeyName = extractPropertyKeyName(s.ctx, ctx) } diff --git a/cypher/frontend/literal.go b/cypher/frontend/literal.go index 735785a9..45503a81 100644 --- a/cypher/frontend/literal.go +++ b/cypher/frontend/literal.go @@ -45,7 +45,7 @@ func (s *MapLiteralVisitor) EnterOC_PropertyKeyName(ctx *parser.OC_PropertyKeyNa } func (s *MapLiteralVisitor) ExitOC_PropertyKeyName(ctx *parser.OC_PropertyKeyNameContext) { - s.nextPropertyKey = s.ctx.Exit().(*SymbolicNameOrReservedWordVisitor).Name + s.nextPropertyKey = cypher.UnescapePropertyKeyName(s.ctx.Exit().(*SymbolicNameOrReservedWordVisitor).Name) } func (s *MapLiteralVisitor) EnterOC_Expression(ctx *parser.OC_ExpressionContext) { diff --git a/cypher/frontend/property_key.go b/cypher/frontend/property_key.go new file mode 100644 index 00000000..f6ef9ca7 --- /dev/null +++ b/cypher/frontend/property_key.go @@ -0,0 +1,20 @@ +package frontend + +import ( + "github.com/specterops/dawgs/cypher/models/cypher" + "github.com/specterops/dawgs/cypher/parser" +) + +func extractPropertyKeyName(ctx *Context, cypherCtx *parser.OC_PropertyKeyNameContext) string { + name := cypher.UnescapePropertyKeyName(ctx.Exit().(*SymbolicNameOrReservedWordVisitor).Name) + if err := cypher.ValidatePropertyKeyName(name); err != nil { + ctx.AddErrors(SyntaxError{ + Line: cypherCtx.GetStart().GetLine(), + Column: cypherCtx.GetStart().GetColumn(), + OffendingSymbol: cypherCtx.GetText(), + Message: err.Error(), + }) + } + + return name +} diff --git a/cypher/frontend/property_key_test.go b/cypher/frontend/property_key_test.go new file mode 100644 index 00000000..754be892 --- /dev/null +++ b/cypher/frontend/property_key_test.go @@ -0,0 +1,103 @@ +package frontend_test + +import ( + "testing" + + "github.com/specterops/dawgs/cypher/frontend" + "github.com/specterops/dawgs/cypher/models/cypher" + "github.com/specterops/dawgs/cypher/models/walk" + "github.com/stretchr/testify/require" +) + +func TestParsePropertyLookupStoresRawPropertyKeyNames(t *testing.T) { + regularQuery, err := frontend.ParseCypher(frontend.NewContext(), "RETURN n.match, n.`a-aaa`, n.`has``tick`, n.` `") + require.NoError(t, err) + + var symbols []string + err = walk.CypherStructural(regularQuery, walk.NewSimpleVisitor[cypher.SyntaxNode](func(node cypher.SyntaxNode, _ walk.VisitorHandler) { + if propertyLookup, typeOK := node.(*cypher.PropertyLookup); typeOK { + symbols = append(symbols, propertyLookup.Symbol) + } + })) + require.NoError(t, err) + + require.Equal(t, []string{"match", "a-aaa", "has`tick", " "}, symbols) +} + +func TestParsePropertyLookupStoresQuotePropertyKeyNames(t *testing.T) { + regularQuery, err := frontend.ParseCypher(frontend.NewContext(), "RETURN n.`'`, n.`\"`") + require.NoError(t, err) + + var symbols []string + err = walk.CypherStructural(regularQuery, walk.NewSimpleVisitor[cypher.SyntaxNode](func(node cypher.SyntaxNode, _ walk.VisitorHandler) { + if propertyLookup, typeOK := node.(*cypher.PropertyLookup); typeOK { + symbols = append(symbols, propertyLookup.Symbol) + } + })) + require.NoError(t, err) + + require.Equal(t, []string{"'", "\""}, symbols) +} + +func TestParsePropertyLookupStoresUnicodePropertyKeyNames(t *testing.T) { + regularQuery, err := frontend.ParseCypher(frontend.NewContext(), "RETURN n.\u2118, n.a\u00b7, n.a\u0301, n.a\u093e, n.a$, n.`a\u20dd`") + require.NoError(t, err) + + var symbols []string + err = walk.CypherStructural(regularQuery, walk.NewSimpleVisitor[cypher.SyntaxNode](func(node cypher.SyntaxNode, _ walk.VisitorHandler) { + if propertyLookup, typeOK := node.(*cypher.PropertyLookup); typeOK { + symbols = append(symbols, propertyLookup.Symbol) + } + })) + require.NoError(t, err) + + require.Equal(t, []string{"\u2118", "a\u00b7", "a\u0301", "a\u093e", "a$", "a\u20dd"}, symbols) +} + +func TestParseMapLiteralStoresRawPropertyKeyNames(t *testing.T) { + regularQuery, err := frontend.ParseCypher(frontend.NewContext(), "RETURN {match: 1, `a-aaa`: 2, `has``tick`: 3, ``: 4, ` `: 5}") + require.NoError(t, err) + + var keys []string + err = walk.CypherStructural(regularQuery, walk.NewSimpleVisitor[cypher.SyntaxNode](func(node cypher.SyntaxNode, _ walk.VisitorHandler) { + if mapItem, typeOK := node.(*cypher.MapItem); typeOK { + keys = append(keys, mapItem.Key) + } + })) + require.NoError(t, err) + + require.ElementsMatch(t, []string{"match", "a-aaa", "has`tick", "", " "}, keys) +} + +func TestParseMapLiteralStoresQuotePropertyKeyNames(t *testing.T) { + regularQuery, err := frontend.ParseCypher(frontend.NewContext(), "RETURN {`'`: 1, `\"`: 2}") + require.NoError(t, err) + + var keys []string + err = walk.CypherStructural(regularQuery, walk.NewSimpleVisitor[cypher.SyntaxNode](func(node cypher.SyntaxNode, _ walk.VisitorHandler) { + if mapItem, typeOK := node.(*cypher.MapItem); typeOK { + keys = append(keys, mapItem.Key) + } + })) + require.NoError(t, err) + + require.ElementsMatch(t, []string{"'", "\""}, keys) +} + +func TestParseRejectsEmptyPropertyKeyNames(t *testing.T) { + testCases := []struct { + name string + query string + }{ + {name: "property lookup", query: "RETURN n.``"}, + {name: "set property", query: "MATCH (n) SET n.`` = 'value'"}, + {name: "remove property", query: "MATCH (n) REMOVE n.``"}, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + _, err := frontend.ParseCypher(frontend.NewContext(), testCase.query) + require.ErrorContains(t, err, cypher.ErrEmptyPropertyKeyName.Error()) + }) + } +} diff --git a/cypher/frontend/query.go b/cypher/frontend/query.go index 27045fab..4207d5bb 100644 --- a/cypher/frontend/query.go +++ b/cypher/frontend/query.go @@ -707,5 +707,5 @@ func (s *PropertyExpressionVisitor) EnterOC_PropertyKeyName(ctx *parser.OC_Prope } func (s *PropertyExpressionVisitor) ExitOC_PropertyKeyName(ctx *parser.OC_PropertyKeyNameContext) { - s.PropertyLookup.SetSymbol(s.ctx.Exit().(*SymbolicNameOrReservedWordVisitor).Name) + s.PropertyLookup.SetSymbol(extractPropertyKeyName(s.ctx, ctx)) } diff --git a/cypher/models/cypher/format/format.go b/cypher/models/cypher/format/format.go index 0173915c..625087d8 100644 --- a/cypher/models/cypher/format/format.go +++ b/cypher/models/cypher/format/format.go @@ -332,7 +332,7 @@ func (s Emitter) formatMapLiteral(output io.Writer, mapLiteral cypher.MapLiteral first = false } - if _, err := io.WriteString(output, key); err != nil { + if _, err := io.WriteString(output, cypher.EscapePropertyKeyName(key)); err != nil { return err } @@ -633,7 +633,11 @@ func (s Emitter) WriteExpression(output io.Writer, expression cypher.Expression) return err } - if _, err := io.WriteString(output, typedExpression.Symbol); err != nil { + if err := cypher.ValidatePropertyKeyName(typedExpression.Symbol); err != nil { + return err + } + + if _, err := io.WriteString(output, cypher.EscapePropertyKeyName(typedExpression.Symbol)); err != nil { return err } diff --git a/cypher/models/cypher/format/format_test.go b/cypher/models/cypher/format/format_test.go index b10e8001..0a463871 100644 --- a/cypher/models/cypher/format/format_test.go +++ b/cypher/models/cypher/format/format_test.go @@ -44,6 +44,91 @@ func TestCypherEmitter_FormatsMapLiteralInKeyOrder(t *testing.T) { require.Equal(t, "{a: 1, b: 2}", buffer.String()) } +func TestCypherEmitter_FormatsMapLiteralPropertyKeys(t *testing.T) { + var ( + buffer = &bytes.Buffer{} + emitter = format.NewCypherEmitter(false) + ) + + err := emitter.WriteExpression(buffer, cypher.MapLiteral{ + "match": cypher.NewLiteral(1, false), + "a-aaa": cypher.NewLiteral(2, false), + "has`tick": cypher.NewLiteral(3, false), + "": cypher.NewLiteral(4, false), + " ": cypher.NewLiteral(5, false), + "'": cypher.NewLiteral(6, false), + }) + + require.NoError(t, err) + require.Equal(t, "{``: 4, ` `: 5, `'`: 6, `a-aaa`: 2, `has``tick`: 3, match: 1}", buffer.String()) +} + +func TestCypherEmitter_FormatsPropertyLookupKeys(t *testing.T) { + testCases := []struct { + name string + symbol string + expected string + }{ + { + name: "simple key", + symbol: "name", + expected: "n.name", + }, + { + name: "reserved word key", + symbol: "match", + expected: "n.match", + }, + { + name: "key with hyphen", + symbol: "a-aaa", + expected: "n.`a-aaa`", + }, + { + name: "key with backtick", + symbol: "has`tick", + expected: "n.`has``tick`", + }, + { + name: "key with single quote", + symbol: "'", + expected: "n.`'`", + }, + { + name: "whitespace-only key", + symbol: " ", + expected: "n.` `", + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + buffer := &bytes.Buffer{} + emitter := format.NewCypherEmitter(false) + + err := emitter.WriteExpression(buffer, &cypher.PropertyLookup{ + Atom: cypher.NewVariableWithSymbol("n"), + Symbol: testCase.symbol, + }) + + require.NoError(t, err) + require.Equal(t, testCase.expected, buffer.String()) + }) + } +} + +func TestCypherEmitter_RejectsEmptyPropertyLookupKey(t *testing.T) { + buffer := &bytes.Buffer{} + emitter := format.NewCypherEmitter(false) + + err := emitter.WriteExpression(buffer, &cypher.PropertyLookup{ + Atom: cypher.NewVariableWithSymbol("n"), + Symbol: "", + }) + + require.ErrorIs(t, err, cypher.ErrEmptyPropertyKeyName) +} + func TestCypherEmitter_MapLiteralPropagatesExpressionError(t *testing.T) { var ( buffer = &bytes.Buffer{} diff --git a/cypher/models/cypher/model.go b/cypher/models/cypher/model.go index 173919b8..514a4fe4 100644 --- a/cypher/models/cypher/model.go +++ b/cypher/models/cypher/model.go @@ -1307,7 +1307,10 @@ func (s *ProjectionItem) copy() *ProjectionItem { } type PropertyLookup struct { - Atom Expression + Atom Expression + + // Symbol is the raw property key, not an already-rendered Cypher token. + // Callers should not pre-wrap names in backticks; formatting handles that. Symbol string } diff --git a/cypher/models/cypher/property_key.go b/cypher/models/cypher/property_key.go new file mode 100644 index 00000000..e39baeab --- /dev/null +++ b/cypher/models/cypher/property_key.go @@ -0,0 +1,80 @@ +package cypher + +import ( + "errors" + "strings" + "unicode" +) + +var ErrEmptyPropertyKeyName = errors.New("property key name must not be empty") + +func isCypherIDStart(char rune) bool { + return unicode.IsLetter(char) || unicode.In(char, unicode.Nl, unicode.Other_ID_Start) +} + +func isCypherIDContinue(char rune) bool { + return isCypherIDStart(char) || unicode.In(char, unicode.Mn, unicode.Mc, unicode.Nd, unicode.Pc, unicode.Other_ID_Continue) +} + +func isCypherSymbolStart(char rune) bool { + return isCypherIDStart(char) || unicode.In(char, unicode.Pc) +} + +func isCypherSymbolPart(char rune) bool { + return isCypherIDContinue(char) || unicode.In(char, unicode.Sc) +} + +// CanEmitBarePropertyKeyName returns true when a raw property key can be emitted without backticks. +// +// This is specific to Cypher property-key position, such as n.name and {name: value}. Property keys use +// oC_PropertyKeyName -> oC_SchemaName, where reserved words are valid bare names, unlike variable or parameter +// symbols. Empty keys and keys containing characters outside the unescaped symbolic-name grammar return false; non-empty +// keys outside the bare grammar are still representable by EscapePropertyKeyName using backticks. +func CanEmitBarePropertyKeyName(name string) bool { + if name == "" { + return false + } + + for idx, char := range name { + if idx == 0 { + if !isCypherSymbolStart(char) { + return false + } + } else if !isCypherSymbolPart(char) { + return false + } + } + + return true +} + +func ValidatePropertyKeyName(name string) error { + if name == "" { + return ErrEmptyPropertyKeyName + } + + return nil +} + +// EscapePropertyKeyName formats a raw property key as a Cypher property-key token. +func EscapePropertyKeyName(name string) string { + if CanEmitBarePropertyKeyName(name) { + return name + } + + return "`" + strings.ReplaceAll(name, "`", "``") + "`" +} + +// IsEscapedPropertyKeyName returns true when name is wrapped in Cypher backtick delimiters. +func IsEscapedPropertyKeyName(name string) bool { + return len(name) >= 2 && name[0] == '`' && name[len(name)-1] == '`' +} + +// UnescapePropertyKeyName decodes a Cypher property-key token into the raw property key it names. +func UnescapePropertyKeyName(name string) string { + if !IsEscapedPropertyKeyName(name) { + return name + } + + return strings.ReplaceAll(name[1:len(name)-1], "``", "`") +} diff --git a/cypher/models/cypher/property_key_test.go b/cypher/models/cypher/property_key_test.go new file mode 100644 index 00000000..4ddfd9b2 --- /dev/null +++ b/cypher/models/cypher/property_key_test.go @@ -0,0 +1,97 @@ +package cypher_test + +import ( + "testing" + + "github.com/specterops/dawgs/cypher/models/cypher" + "github.com/stretchr/testify/require" +) + +func TestCanEmitBarePropertyKeyName(t *testing.T) { + testCases := []struct { + name string + input string + expected bool + }{ + {name: "simple", input: "name", expected: true}, + {name: "underscore", input: "object_id", expected: true}, + {name: "reserved word allowed in property key position", input: "match", expected: true}, + {name: "other id start", input: "\u2118", expected: true}, + {name: "other id continue", input: "a\u00b7", expected: true}, + {name: "nonspacing mark part", input: "a\u0301", expected: true}, + {name: "spacing mark part", input: "a\u093e", expected: true}, + {name: "currency symbol part", input: "a$", expected: true}, + {name: "empty", input: "", expected: false}, + {name: "dash", input: "a-aaa", expected: false}, + {name: "starts digit", input: "1name", expected: false}, + {name: "starts currency symbol", input: "$a", expected: false}, + {name: "literal backtick", input: "has`tick", expected: false}, + {name: "enclosing mark part", input: "a\u20dd", expected: false}, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + require.Equal(t, testCase.expected, cypher.CanEmitBarePropertyKeyName(testCase.input)) + }) + } +} + +func TestEscapePropertyKeyName(t *testing.T) { + testCases := []struct { + name string + input string + expected string + }{ + {name: "simple", input: "name", expected: "name"}, + {name: "reserved word allowed in property key position", input: "match", expected: "match"}, + {name: "other id start", input: "\u2118", expected: "\u2118"}, + {name: "other id continue", input: "a\u00b7", expected: "a\u00b7"}, + {name: "nonspacing mark part", input: "a\u0301", expected: "a\u0301"}, + {name: "spacing mark part", input: "a\u093e", expected: "a\u093e"}, + {name: "currency symbol part", input: "a$", expected: "a$"}, + {name: "enclosing mark part", input: "a\u20dd", expected: "`a\u20dd`"}, + {name: "dash", input: "a-aaa", expected: "`a-aaa`"}, + {name: "embedded backtick", input: "has`tick", expected: "`has``tick`"}, + {name: "starts backtick", input: "`starts-tick", expected: "```starts-tick`"}, + {name: "wrapped backticks", input: "`super-wrapped`", expected: "```super-wrapped```"}, + {name: "single backtick", input: "`", expected: "````"}, + {name: "single quote", input: "'", expected: "`'`"}, + {name: "double quote", input: "\"", expected: "`\"`"}, + {name: "whitespace-only", input: " ", expected: "` `"}, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + require.Equal(t, testCase.expected, cypher.EscapePropertyKeyName(testCase.input)) + }) + } +} + +func TestValidatePropertyKeyName(t *testing.T) { + require.NoError(t, cypher.ValidatePropertyKeyName(" ")) + require.ErrorIs(t, cypher.ValidatePropertyKeyName(""), cypher.ErrEmptyPropertyKeyName) +} + +func TestUnescapePropertyKeyName(t *testing.T) { + testCases := []struct { + name string + input string + expected string + }{ + {name: "simple", input: "name", expected: "name"}, + {name: "dash", input: "`a-aaa`", expected: "a-aaa"}, + {name: "embedded backtick", input: "`has``tick`", expected: "has`tick"}, + {name: "starts backtick", input: "```starts-tick`", expected: "`starts-tick"}, + {name: "wrapped backticks", input: "```super-wrapped```", expected: "`super-wrapped`"}, + {name: "single backtick", input: "````", expected: "`"}, + {name: "single quote", input: "`'`", expected: "'"}, + {name: "double quote", input: "`\"`", expected: "\""}, + {name: "empty", input: "``", expected: ""}, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + require.Equal(t, testCase.expected, cypher.UnescapePropertyKeyName(testCase.input)) + }) + } +} diff --git a/cypher/models/pgsql/test/translation_cases/nodes.sql b/cypher/models/pgsql/test/translation_cases/nodes.sql index 7a54ce40..5b10c34e 100644 --- a/cypher/models/pgsql/test/translation_cases/nodes.sql +++ b/cypher/models/pgsql/test/translation_cases/nodes.sql @@ -47,9 +47,39 @@ with s0 as (select (n0.id, n0.kind_ids, n0.properties)::nodecomposite as n0 from -- case: match (n) where n.name = '1234' return n with s0 as (select (n0.id, n0.kind_ids, n0.properties)::nodecomposite as n0 from node n0 where ((jsonb_typeof((n0.properties -> 'name')) = 'string' and (n0.properties ->> 'name') = '1234'))) select s0.n0 as n from s0; +-- case: match (n) where n.`a-aaa` = "123" return n +with s0 as (select (n0.id, n0.kind_ids, n0.properties)::nodecomposite as n0 from node n0 where ((jsonb_typeof((n0.properties -> 'a-aaa')) = 'string' and (n0.properties ->> 'a-aaa') = '123'))) select s0.n0 as n from s0; + +-- case: match (n) where n.`b_bbb` = "123" return n +with s0 as (select (n0.id, n0.kind_ids, n0.properties)::nodecomposite as n0 from node n0 where ((jsonb_typeof((n0.properties -> 'b_bbb')) = 'string' and (n0.properties ->> 'b_bbb') = '123'))) select s0.n0 as n from s0; + +-- case: match (n) where n.`has``tick` = "123" return n +with s0 as (select (n0.id, n0.kind_ids, n0.properties)::nodecomposite as n0 from node n0 where ((jsonb_typeof((n0.properties -> 'has`tick')) = 'string' and (n0.properties ->> 'has`tick') = '123'))) select s0.n0 as n from s0; + +-- case: match (n) where n.`'` = "123" return n +with s0 as (select (n0.id, n0.kind_ids, n0.properties)::nodecomposite as n0 from node n0 where ((jsonb_typeof((n0.properties -> '''')) = 'string' and (n0.properties ->> '''') = '123'))) select s0.n0 as n from s0; + +-- case: match (n) where n.```starts-tick` = "123" return n +with s0 as (select (n0.id, n0.kind_ids, n0.properties)::nodecomposite as n0 from node n0 where ((jsonb_typeof((n0.properties -> '`starts-tick')) = 'string' and (n0.properties ->> '`starts-tick') = '123'))) select s0.n0 as n from s0; + +-- case: match (n) where n.```super-wrapped``` = "123" return n +with s0 as (select (n0.id, n0.kind_ids, n0.properties)::nodecomposite as n0 from node n0 where ((jsonb_typeof((n0.properties -> '`super-wrapped`')) = 'string' and (n0.properties ->> '`super-wrapped`') = '123'))) select s0.n0 as n from s0; + +-- case: match (n) where n.```` = "123" return n +with s0 as (select (n0.id, n0.kind_ids, n0.properties)::nodecomposite as n0 from node n0 where ((jsonb_typeof((n0.properties -> '`')) = 'string' and (n0.properties ->> '`') = '123'))) select s0.n0 as n from s0; + +-- case: match (n) where (n).`a-aaa` = "123" return n +with s0 as (select (n0.id, n0.kind_ids, n0.properties)::nodecomposite as n0 from node n0) select s0.n0 as n from s0 where ((jsonb_typeof((((s0.n0)).properties -> 'a-aaa')) = 'string' and (((s0.n0)).properties ->> 'a-aaa') = '123')); + +-- case: match ()-[r]-() where startNode(r).`something` = "abc" return r +with s0 as (select (e0.id, e0.start_id, e0.end_id, e0.kind_id, e0.properties)::edgecomposite as e0 from edge e0 join node n0 on (n0.id = e0.end_id or n0.id = e0.start_id) join node n1 on (n1.id = e0.end_id or n1.id = e0.start_id) where (n0.id <> n1.id)) select s0.e0 as r from s0 where ((jsonb_typeof(((start_node(((s0.e0).id, (s0.e0).start_id, (s0.e0).end_id, (s0.e0).kind_id, (s0.e0).properties)::edgecomposite)::nodecomposite).properties -> 'something')) = 'string' and ((start_node(((s0.e0).id, (s0.e0).start_id, (s0.e0).end_id, (s0.e0).kind_id, (s0.e0).properties)::edgecomposite)::nodecomposite).properties ->> 'something') = 'abc')); + -- case: match (n:NodeKind1 {name: "SOME NAME"}) return n with s0 as (select (n0.id, n0.kind_ids, n0.properties)::nodecomposite as n0 from node n0 where n0.kind_ids operator (pg_catalog.@>) array [1]::int2[] and (jsonb_typeof((n0.properties -> 'name')) = 'string' and (n0.properties ->> 'name') = 'SOME NAME')) select s0.n0 as n from s0; +-- case: match (n:NodeKind1 {`'`: 'value'}) return n +with s0 as (select (n0.id, n0.kind_ids, n0.properties)::nodecomposite as n0 from node n0 where n0.kind_ids operator (pg_catalog.@>) array [1]::int2[] and (jsonb_typeof((n0.properties -> '''')) = 'string' and (n0.properties ->> '''') = 'value')) select s0.n0 as n from s0; + -- case: match (n) where n.objectid in $p return n -- cypher_params: {"p":["1","2","3"]} -- pgsql_params:{"pi0":["1","2","3"]} diff --git a/cypher/models/pgsql/translate/expression.go b/cypher/models/pgsql/translate/expression.go index 6123f8a5..c8c5ff70 100644 --- a/cypher/models/pgsql/translate/expression.go +++ b/cypher/models/pgsql/translate/expression.go @@ -53,8 +53,11 @@ func (s *Translator) translateCompositePropertyLookup(target pgsql.Expression, l return s.treeTranslator.CompleteBinaryExpression(s.scope, pgsql.OperatorPropertyLookup) } } - func (s *Translator) translatePropertyLookup(lookup *cypher.PropertyLookup) error { + if err := cypher.ValidatePropertyKeyName(lookup.Symbol); err != nil { + return err + } + if translatedAtom, err := s.treeTranslator.PopOperand(); err != nil { return err } else { @@ -300,7 +303,7 @@ func lookupRequiresElementType(typeHint pgsql.DataType, operator pgsql.Operator, func TypeCastExpression(expression pgsql.Expression, dataType pgsql.DataType) (pgsql.Expression, error) { if propertyLookup, isPropertyLookup := expressionToPropertyLookupBinaryExpression(expression); isPropertyLookup { - var lookupTypeHint = dataType + lookupTypeHint := dataType if lookupRequiresElementType(dataType, propertyLookup.Operator, propertyLookup.ROperand) { // Take the base type of the array type hint: in @@ -750,7 +753,6 @@ func rewriteIdentityOperands(scope *Scope, newExpression *pgsql.BinaryExpression } } } - } } diff --git a/cypher/models/pgsql/translate/semantic_drift_test.go b/cypher/models/pgsql/translate/semantic_drift_test.go index a20b7d69..1efd5a6b 100644 --- a/cypher/models/pgsql/translate/semantic_drift_test.go +++ b/cypher/models/pgsql/translate/semantic_drift_test.go @@ -5,6 +5,8 @@ import ( "testing" "github.com/specterops/dawgs/cypher/frontend" + "github.com/specterops/dawgs/cypher/models/cypher" + "github.com/specterops/dawgs/cypher/models/walk" "github.com/specterops/dawgs/drivers/pg/pgutil" "github.com/specterops/dawgs/graph" "github.com/stretchr/testify/require" @@ -40,3 +42,20 @@ func TestTranslatorRejectsUnsupportedPropertyLookupSourcesDirectly(t *testing.T) require.Error(t, err) require.Contains(t, err.Error(), "unsupported property lookup prop on expression type int8[]") } + +func TestTranslatorRejectsEmptyPropertyLookupKeys(t *testing.T) { + kindMapper := pgutil.NewInMemoryKindMapper() + + query, err := frontend.ParseCypher(frontend.NewContext(), `MATCH (n) RETURN n.name`) + require.NoError(t, err) + + err = walk.CypherStructural(query, walk.NewSimpleVisitor[cypher.SyntaxNode](func(node cypher.SyntaxNode, _ walk.VisitorHandler) { + if propertyLookup, typeOK := node.(*cypher.PropertyLookup); typeOK { + propertyLookup.Symbol = "" + } + })) + require.NoError(t, err) + + _, err = Translate(context.Background(), query, kindMapper, nil, DefaultGraphID) + require.ErrorIs(t, err, cypher.ErrEmptyPropertyKeyName) +} diff --git a/integration/testdata/bed8967.json b/integration/testdata/bed8967.json new file mode 100644 index 00000000..6d915e7b --- /dev/null +++ b/integration/testdata/bed8967.json @@ -0,0 +1,39 @@ +{ + "graph": { + "nodes": [ + { + "id": "alpha", + "kinds": ["BacktickNode"], + "properties": { + "name": "alpha", + "a-aaa": "alpha-hyphen", + "has`tick": "alpha-backtick", + " ": "alpha-whitespace", + "a\u20dd": "alpha-enclosing" + } + }, + { + "id": "beta", + "kinds": ["BacktickNode"], + "properties": { + "name": "beta", + "a-aaa": "beta-hyphen", + "has`tick": "beta-backtick", + " ": "beta-whitespace", + "a\u20dd": "beta-enclosing" + } + } + ], + "edges": [ + { + "start_id": "alpha", + "end_id": "beta", + "kind": "BacktickEdge", + "properties": { + "edge-key": "edge-hyphen", + "has`tick": "edge-backtick" + } + } + ] + } +} diff --git a/integration/testdata/cases/bed8967-backtick_property_keys.json b/integration/testdata/cases/bed8967-backtick_property_keys.json new file mode 100644 index 00000000..d2cfec05 --- /dev/null +++ b/integration/testdata/cases/bed8967-backtick_property_keys.json @@ -0,0 +1,52 @@ +{ + "dataset": "bed8967", + "cases": [ + { + "name": "BED-8967 read escaped node property keys", + "cypher": "match (n:BacktickNode) return n.`a-aaa`, n.`has``tick`, n.` `, n.`a\u20dd` order by n.name", + "assert": { + "ordered_row_values": [ + ["alpha-hyphen", "alpha-backtick", "alpha-whitespace", "alpha-enclosing"], + ["beta-hyphen", "beta-backtick", "beta-whitespace", "beta-enclosing"] + ] + } + }, + { + "name": "BED-8967 reject empty escaped property key", + "cypher": "match (n:BacktickNode) return n.``", + "assert": "query_error" + }, + { + "name": "BED-8967 filter node using escaped pattern property key", + "cypher": "match (n:BacktickNode {`a-aaa`: 'beta-hyphen'}) return n.name", + "assert": {"scalar_values": ["beta"]} + }, + { + "name": "BED-8967 read escaped relationship property keys", + "cypher": "match (:BacktickNode {name: 'alpha'})-[r:BacktickEdge]->(:BacktickNode {name: 'beta'}) return r.`edge-key`, r.`has``tick`", + "assert": {"row_values": [["edge-hyphen", "edge-backtick"]]} + }, + { + "name": "BED-8967 set escaped node property keys", + "cypher": "match (n:BacktickNode {name: 'mutable'}) set n.`set-key` = 'set-value', n.`has``tick` = 'updated-backtick' return n.`set-key`, n.`has``tick`", + "fixture": { + "nodes": [ + {"id": "mutable", "kinds": ["BacktickNode"], "properties": {"name": "mutable", "has`tick": "old-backtick"}} + ], + "edges": [] + }, + "assert": {"row_values": [["set-value", "updated-backtick"]]} + }, + { + "name": "BED-8967 remove escaped node property key", + "cypher": "match (n:BacktickNode {name: 'removable'}) remove n.`remove-key` return n.`remove-key`", + "fixture": { + "nodes": [ + {"id": "removable", "kinds": ["BacktickNode"], "properties": {"name": "removable", "remove-key": "remove-me"}} + ], + "edges": [] + }, + "assert": {"scalar_values": [null]} + } + ] +} diff --git a/query/builder_test.go b/query/builder_test.go index 0a44e20d..af2237da 100644 --- a/query/builder_test.go +++ b/query/builder_test.go @@ -78,6 +78,30 @@ func TestBuilderProjectionModifiersAreOrderIndependent(t *testing.T) { } } +func TestBuilderRendersRawPropertyKeys(t *testing.T) { + builder := query.NewBuilder(nil) + builder.Apply(query.Returning( + query.NodeProperty("a-aaa"), + query.Property(query.Node(), "has`tick"), + query.Property(query.Node(), " "), + )) + + regularQuery, err := builder.Build(false) + if err != nil { + t.Fatalf("build query: %v", err) + } + + var cypher bytes.Buffer + if err := cypherFormat.NewCypherEmitter(false).Write(regularQuery, &cypher); err != nil { + t.Fatalf("render Cypher: %v", err) + } + + expected := "match (n) return n.`a-aaa`, n.`has``tick`, n.` `" + if cypher.String() != expected { + t.Fatalf("expected %q, got %q", expected, cypher.String()) + } +} + func assertRetrieverProjection(t *testing.T, rendered string) { t.Helper() diff --git a/query/v2/query.go b/query/v2/query.go index a841c752..8faf2518 100644 --- a/query/v2/query.go +++ b/query/v2/query.go @@ -653,6 +653,14 @@ func (s *entity[T]) ID() IdentityContinuation { } func (s *entity[T]) Property(propertyName string) PropertyContinuation { + if err := cypher.ValidatePropertyKeyName(propertyName); err != nil { + return &propertyContinuation{ + comparisonContinuation: comparisonContinuation{ + qualifierExpression: invalidExpression(err), + }, + } + } + return &propertyContinuation{ comparisonContinuation: comparisonContinuation{ qualifierExpression: cypher.NewPropertyLookup(s.identifier.Symbol, propertyName), diff --git a/query/v2/query_test.go b/query/v2/query_test.go index 188530fd..18102d2f 100644 --- a/query/v2/query_test.go +++ b/query/v2/query_test.go @@ -117,6 +117,24 @@ func TestCreateRelationshipWithExplicitEndpoints(t *testing.T) { }, preparedQuery.Parameters) } +func TestRawPropertyKeysRenderEscaped(t *testing.T) { + preparedQuery, err := v2.New().Return( + v2.Node().Property("a-aaa"), + v2.Node().Property("has`tick"), + v2.Node().Property(" "), + ).Build() + require.NoError(t, err) + + require.Equal(t, "match (n) return n.`a-aaa`, n.`has``tick`, n.` `", renderPrepared(t, preparedQuery)) +} + +func TestEmptyPropertyKeyReturnsBuildError(t *testing.T) { + _, err := v2.New().Return( + v2.Node().Property(""), + ).Build() + require.ErrorIs(t, err, cypher.ErrEmptyPropertyKeyName) +} + func TestCreateSplitsDisjointNodePatterns(t *testing.T) { preparedQuery, err := v2.New().Create( v2.NodePattern(graph.Kinds{graph.StringKind("A")}, nil), diff --git a/query/v2/util.go b/query/v2/util.go index 03b5fb10..e1bdc341 100644 --- a/query/v2/util.go +++ b/query/v2/util.go @@ -248,6 +248,10 @@ func variableReference(value any) (*cypher.Variable, error) { } func propertyLookupOrError(reference any, propertyName string) cypher.Expression { + if err := cypher.ValidatePropertyKeyName(propertyName); err != nil { + return invalidExpression(err) + } + if variable, err := variableReference(reference); err != nil { return invalidExpression(err) } else {